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.
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
Disabled IconButton states #521
Conversation
background: transparent; | ||
} | ||
|
||
.enabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tiny nit: I'm not sure how the rest of Gestalt does it, but in past projects, I tended to include the default case (enabled, in this case) along with the base styles, and then use the alternate (disabled) state as overrides. Otherwise the component isn't truly "styled" until you add either an enabled or disabled style, and the code doesn't communicate this well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried pulling the pointer back into the base styles and only adding the disabled one when the prop is set but it doesnt work. The base styles always override this one. Im going to keep it as is since thats what Button.js does
packages/gestalt/src/IconButton.js
Outdated
@@ -93,10 +101,10 @@ export default class IconButton extends React.Component<Props, State> { | |||
type="button" | |||
> | |||
<Pog | |||
active={active} | |||
active={disabled ? false : active} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
active && !disabled
?
Stop gap solution to introduce
disabled
prop to<IconButton />
until we get a more cohesive unified solution to buttons.