fix(NotificationBadge): add Button.variant to allowed NotificationBadgeVariants
#12069
Conversation
|
Preview: https://pf-react-pr-12069.surge.sh A11y report: https://pf-react-pr-12069-a11y.surge.sh |
There was a problem hiding this comment.
Change request below if this is something that is still approved by design. cc @andrew-ronaldson wdyt? Should NotificationBadge support plain styling (and if so, only plain or stateful?), or would this implementation be better suited to setting up a custom Button (after all all NotificationBadge is is a Button wwith only the necessary props exposed)?
| isExpanded?: boolean; | ||
| /** Determines the variant of the notification badge. */ | ||
| variant?: NotificationBadgeVariant | 'read' | 'unread' | 'attention'; | ||
| variant?: NotificationBadgeVariant | ButtonProps['variant'] | 'read' | 'unread' | 'attention'; |
There was a problem hiding this comment.
We would want to add a different prop, maybe buttonVariant, to pass into the Button variant prop. This prop is a bit confusingly named and should only be for passing to the Button state prop.
We would also need to only pass in anything to the Button state prop only if the button variant is stateful, otherwise the icon coloring on hover/focus gets messed up if the state modifier classes are applied with pf-m-plain.
3 participants
To be honest I don't know if these changes should be made to PF.. but
variant="plain"looks the most "in place" in the context of OCP's masthead and we should definitely at least have that variant supported in PFWhat: Closes #12066
Additional issues: openshift/console#15555