Skip to content

Button.counter does not have disabled styles #1955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 16, 2022
Merged

Button.counter does not have disabled styles #1955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a6388eb
Add disabled styles to counter and add storybook
pksjce Mar 14, 2022
5ce7323
Update with PR review comments
pksjce Mar 15, 2022
eb83a0b
Create yellow-planes-decide.md
pksjce Mar 15, 2022
4c71d4e
Merge branch 'main' into pk/button-counter
pksjce Mar 15, 2022
b9a29d9
Merge branch 'main' into pk/button-counter
pksjce Mar 16, 2022
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/yellow-planes-decide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Add disabled color and backgroundColor to Button.Counter
15 changes: 14 additions & 1 deletion src/Button/Button2.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ export default {
type: 'radio',
options: ['small', 'medium', 'large']
}
},
disabled: {
control: {
type: 'boolean',
default: false
}
}
}
} as Meta
Expand Down Expand Up @@ -151,9 +157,16 @@ export const blockButton = ({...args}: ButtonProps) => {
)
}

export const disabledButton = ({...args}: ButtonProps) => {
export const DisabledButton = ({...args}: ButtonProps) => {
const [count, setCount] = useState(0)
return (
<>
<Box mb={2}>
<Button disabled onClick={() => setCount(count + 1)} {...args}>
Watch
<Button.Counter>{count}</Button.Counter>
</Button>
</Box>
<Box mb={2}>
<Button disabled {...args}>
Disabled
Expand Down
36 changes: 23 additions & 13 deletions src/Button/styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
},
'&:disabled': {
color: 'primer.fg.disabled',
backgroundColor: 'btn.disabledBg'
Copy link
Member

@siddharthkp siddharthkp Mar 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

btn.disabledBg doesn't exist 🤔 , it falls back to btn.bg which is the right background anyways, so we can remove this line :)

image

'[data-component=ButtonCounter]': {
color: 'inherit'
}
},
'&[aria-expanded=true]': {
backgroundColor: 'btn.activeBg',
Expand All @@ -48,9 +50,12 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
},
'&:disabled': {
color: 'btn.primary.disabledText',
backgroundColor: 'btn.primary.disabledBg'
backgroundColor: 'btn.primary.disabledBg',
'[data-component=ButtonCounter]': {
color: 'inherit'
}
},
'[data-component="ButtonCounter"]': {
'[data-component=ButtonCounter]': {
backgroundColor: 'btn.primary.counterBg',
color: 'btn.primary.text'
},
Expand All @@ -68,7 +73,7 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
backgroundColor: 'btn.danger.hoverBg',
borderColor: 'btn.danger.hoverBorder',
boxShadow: `${theme?.shadows.btn.danger.hoverShadow}`,
'[data-component="ButtonCounter"]': {
'[data-component=ButtonCounter]': {
backgroundColor: 'btn.danger.hoverCounterBg',
color: 'btn.danger.hoverText'
}
Expand All @@ -88,11 +93,12 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
color: 'btn.danger.disabledText',
backgroundColor: 'btn.danger.disabledBg',
borderColor: 'btn.danger.disabledBorder',
'[data-component="ButtonCounter"]': {
'[data-component=ButtonCounter]': {
color: 'inherit',
backgroundColor: 'btn.danger.disabledCounterBg'
}
},
'[data-component="ButtonCounter"]': {
'[data-component=ButtonCounter]': {
color: 'btn.danger.text',
backgroundColor: 'btn.danger.counterBg'
},
Expand All @@ -119,7 +125,10 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
backgroundColor: 'btn.selectedBg'
},
'&:disabled': {
color: 'primer.fg.disabled'
color: 'primer.fg.disabled',
'[data-component=ButtonCounter]': {
color: 'inherit'
}
},
'&[aria-expanded=true]': {
backgroundColor: 'btn.selectedBg'
Expand All @@ -136,9 +145,9 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
backgroundColor: 'btn.outline.hoverBg',
borderColor: 'outline.hoverBorder',
boxShadow: `${theme?.shadows.btn.outline.hoverShadow}`,
'[data-component="ButtonCounter"]': {
'[data-component=ButtonCounter]': {
backgroundColor: 'btn.outline.hoverCounterBg',
color: 'btn.outline.hoverText'
color: 'inherit'
}
},
// focus must come before :active so that the active box shadow overrides
Expand All @@ -158,11 +167,12 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
color: 'btn.outline.disabledText',
backgroundColor: 'btn.outline.disabledBg',
borderColor: 'btn.border',
'[data-component="ButtonCounter"]': {
backgroundColor: 'btn.outline.disabledCounterBg'
'[data-component=ButtonCounter]': {
backgroundColor: 'btn.outline.disabledCounterBg',
color: 'inherit'
}
},
'[data-component="ButtonCounter"]': {
'[data-component=ButtonCounter]': {
backgroundColor: 'btn.outline.counterBg',
color: 'btn.outline.text'
},
Expand Down Expand Up @@ -212,7 +222,7 @@ export const getSizeStyles = (size = 'medium', variant: VariantType = 'default',
paddingY: `${paddingY}px`,
paddingX: `${paddingX}px`,
fontSize,
'[data-component="ButtonCounter"]': {
'[data-component=ButtonCounter]': {
fontSize
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/__tests__/__snapshots__/ActionMenu.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ exports[`ActionMenu renders consistently 1`] = `
.c1:disabled {
cursor: default;
color: #8c959f;
background-color: btn.disabledBg;
}

.c1:disabled [data-component=ButtonCounter] {
color: inherit;
}

.c1:disabled svg {
Expand All @@ -74,7 +77,7 @@ exports[`ActionMenu renders consistently 1`] = `
grid-area: trailingIcon;
}

.c1 [data-component="ButtonCounter"] {
.c1 [data-component=ButtonCounter] {
font-size: 14px;
}

Expand Down
35 changes: 24 additions & 11 deletions src/__tests__/__snapshots__/Button.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ exports[`Button renders consistently 1`] = `
.c0:disabled {
cursor: default;
color: #8c959f;
background-color: btn.disabledBg;
}

.c0:disabled [data-component=ButtonCounter] {
color: inherit;
}

.c0:disabled svg {
Expand All @@ -63,7 +66,7 @@ exports[`Button renders consistently 1`] = `
grid-area: trailingIcon;
}

.c0 [data-component="ButtonCounter"] {
.c0 [data-component=ButtonCounter] {
font-size: 14px;
}

Expand Down Expand Up @@ -133,7 +136,8 @@ exports[`Button styles danger button appropriately 1`] = `
border-color: btn.danger.disabledBorder;
}

.c0:disabled [data-component="ButtonCounter"] {
.c0:disabled [data-component=ButtonCounter] {
color: inherit;
background-color: btn.danger.disabledCounterBg;
}

Expand All @@ -157,7 +161,7 @@ exports[`Button styles danger button appropriately 1`] = `
grid-area: trailingIcon;
}

.c0 [data-component="ButtonCounter"] {
.c0 [data-component=ButtonCounter] {
font-size: 14px;
color: btn.danger.text;
background-color: btn.danger.counterBg;
Expand All @@ -170,7 +174,7 @@ exports[`Button styles danger button appropriately 1`] = `
box-shadow: undefined;
}

.c0:hover:not([disabled]) [data-component="ButtonCounter"] {
.c0:hover:not([disabled]) [data-component=ButtonCounter] {
background-color: btn.danger.hoverCounterBg;
color: btn.danger.hoverText;
}
Expand Down Expand Up @@ -246,6 +250,10 @@ exports[`Button styles invisible button appropriately 1`] = `
color: primer.fg.disabled;
}

.c0:disabled [data-component=ButtonCounter] {
color: inherit;
}

.c0:disabled svg {
opacity: 0.6;
}
Expand All @@ -266,7 +274,7 @@ exports[`Button styles invisible button appropriately 1`] = `
grid-area: trailingIcon;
}

.c0 [data-component="ButtonCounter"] {
.c0 [data-component=ButtonCounter] {
font-size: 14px;
}

Expand Down Expand Up @@ -341,8 +349,9 @@ exports[`Button styles outline button appropriately 1`] = `
border-color: btn.border;
}

.c0:disabled [data-component="ButtonCounter"] {
.c0:disabled [data-component=ButtonCounter] {
background-color: btn.outline.disabledCounterBg;
color: inherit;
}

.c0:disabled svg {
Expand All @@ -365,7 +374,7 @@ exports[`Button styles outline button appropriately 1`] = `
grid-area: trailingIcon;
}

.c0 [data-component="ButtonCounter"] {
.c0 [data-component=ButtonCounter] {
font-size: 14px;
background-color: btn.outline.counterBg;
color: btn.outline.text;
Expand All @@ -378,9 +387,9 @@ exports[`Button styles outline button appropriately 1`] = `
box-shadow: undefined;
}

.c0:hover:not([disabled]) [data-component="ButtonCounter"] {
.c0:hover:not([disabled]) [data-component=ButtonCounter] {
background-color: btn.outline.hoverCounterBg;
color: btn.outline.hoverText;
color: inherit;
}

.c0:focus:not([disabled]) {
Expand Down Expand Up @@ -456,6 +465,10 @@ exports[`Button styles primary button appropriately 1`] = `
background-color: btn.primary.disabledBg;
}

.c0:disabled [data-component=ButtonCounter] {
color: inherit;
}

.c0:disabled svg {
opacity: 0.6;
}
Expand All @@ -476,7 +489,7 @@ exports[`Button styles primary button appropriately 1`] = `
grid-area: trailingIcon;
}

.c0 [data-component="ButtonCounter"] {
.c0 [data-component=ButtonCounter] {
font-size: 14px;
background-color: btn.primary.counterBg;
color: btn.primary.text;
Expand Down