Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/busy-masks-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

CounterLabel: Deprecated scheme prop in favor of variant prop
Comment thread
lukasoppermann marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ export default {
component: CounterLabel,
} as Meta<typeof CounterLabel>

export const PrimaryTheme: StoryFn<typeof CounterLabel> = () => <CounterLabel scheme="primary">12</CounterLabel>
export const PrimaryTheme: StoryFn<typeof CounterLabel> = () => <CounterLabel variant="primary">12</CounterLabel>

export const SecondaryTheme: StoryFn<typeof CounterLabel> = () => <CounterLabel scheme="secondary">12</CounterLabel>
export const SecondaryTheme: StoryFn<typeof CounterLabel> = () => <CounterLabel variant="secondary">12</CounterLabel>
2 changes: 1 addition & 1 deletion packages/react/src/CounterLabel/CounterLabel.figma.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ figma.connect(
}),
count: figma.textContent('text'),
},
example: ({variant, count}) => <CounterLabel scheme={variant}>{count}</CounterLabel>,
example: ({variant, count}) => <CounterLabel variant={variant}>{count}</CounterLabel>,
},
)
4 changes: 2 additions & 2 deletions packages/react/src/CounterLabel/CounterLabel.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@
/* stylelint-disable-next-line primer/borders */
border-radius: 20px;

&:where([data-scheme='primary']) {
&:where([data-variant='primary']) {
color: var(--fgColor-onEmphasis);
background-color: var(--bgColor-neutral-emphasis);
}

&:where([data-scheme='secondary']) {
&:where([data-variant='secondary']) {
color: var(--fgColor-default);
background-color: var(--bgColor-neutral-muted);
}
Expand Down
6 changes: 3 additions & 3 deletions packages/react/src/CounterLabel/CounterLabel.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ export const Default: StoryFn<typeof CounterLabel> = () => <CounterLabel>12</Cou
export const Playground: StoryObj<typeof CounterLabel> = {
render: args => <CounterLabel {...args}>12</CounterLabel>,
args: {
scheme: 'primary',
variant: 'primary',
},
argTypes: {
scheme: {
control: 'select',
variant: {
control: 'radio',
options: ['primary', 'secondary'],
},
},
Expand Down
14 changes: 13 additions & 1 deletion packages/react/src/CounterLabel/CounterLabel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,25 @@ describe('CounterLabel', () => {
expect(container.firstChild).toHaveAttribute('aria-hidden', 'true')
})

it('respects the primary "variant" prop', () => {
const {container} = HTMLRender(<CounterLabel variant="primary">1234</CounterLabel>)
expect(container.firstChild).toBeInTheDocument()
expect(container.firstChild).toHaveTextContent('1234')
})

it('respects the secondary "variant" prop', () => {
const {container} = HTMLRender(<CounterLabel variant="secondary">1234</CounterLabel>)
expect(container.firstChild).toBeInTheDocument()
expect(container.firstChild).toHaveTextContent('1234')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of the added tests check for the same thing 🤔

What are we testing here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I somehow just copied the old tests and didn't get that they don't quite make sense.

})

it('respects the primary "scheme" prop', () => {
const {container} = HTMLRender(<CounterLabel scheme="primary">1234</CounterLabel>)
expect(container.firstChild).toBeInTheDocument()
expect(container.firstChild).toHaveTextContent('1234')
})

it('renders with secondary scheme when no "scheme" prop is provided', () => {
it('renders with secondary variant when no "scheme" or "variant" prop is provided', () => {
const {container} = HTMLRender(<CounterLabel>1234</CounterLabel>)
expect(container.firstChild).toBeInTheDocument()
expect(container.firstChild).toHaveTextContent('1234')
Expand Down
9 changes: 7 additions & 2 deletions packages/react/src/CounterLabel/CounterLabel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,23 @@ import classes from './CounterLabel.module.css'

export type CounterLabelProps = React.PropsWithChildren<
HTMLAttributes<HTMLSpanElement> & {
/** @deprecated use variant instead */
scheme?: 'primary' | 'secondary'
variant?: 'primary' | 'secondary'
className?: string
}
>

const CounterLabel = forwardRef<HTMLSpanElement, CounterLabelProps>(
({scheme = 'secondary', className, children, ...rest}, forwardedRef) => {
({variant, scheme, className, children, ...rest}, forwardedRef) => {
const label = <VisuallyHidden>&nbsp;({children})</VisuallyHidden>

const inferredVariant = variant ? variant : scheme ? scheme : 'secondary'
Comment thread
lukasoppermann marked this conversation as resolved.
Outdated

const counterProps = {
ref: forwardedRef,
['aria-hidden']: 'true' as const,
['data-scheme']: scheme,
['data-variant']: inferredVariant,
...rest,
}

Expand Down
Loading