feat(NotificationBadge): added expanded state
What: Closes #7379, closes #6142
Notification badge examples Notification drawer demos
- Added an
isExpandedprop to the Notification Badge component - Updated the examples so that they match Core (showing read, unread, and attention variants)
- Updated the examples so that the onClick handler updates the expanded state, rather than the variant
- Updated Notification Drawer demos to use this new expanded state
- Reordered the props alphabetically
Additional issues: N/A
Preview: https://patternfly-react-pr-8010.surge.sh
A11y report: https://patternfly-react-pr-8010-a11y.surge.sh
@thatblindgeye and @mcoker shouldn't there be a new "expanded". example, Or at least a way to switch the basic? example?
Actually I see the difference now. You built it into each example. I admit I was confused about the use case for 'isExpanded` in the in context of just the notification badge example. That could just me though. @mcarrano Maybe some descriptive text would help.
@tlabaj @mcarrano it might depend whether we intend for the Notification Badge to ever be used without the Notification Drawer (maybe instead of a drawer, the badge links to a "notifications" page or something)? I could either add in something like:
The following example.... The isExpanded property is also passed in to visually indicate that the badge is expanded, for use-cases as seen in our [notification drawer](...) demos.
Or if we intend for the badge to be used independently of the drawer, I could revert the current examples a bit and add a new one for this new prop (with similar verbiage as above).
@thatblindgeye I think that the text you proposed would be helpful to convey what's going on here.
It's a good question whether this might ever be used for something other than the drawer. To that extent I wonder if calling this an "expanded" state is a bit misleading. It's really more like a selected or active state that can be more generally used to reflect state. @mcoker what do you think of that? I think "expanded" came from our discussion of the need to visually reflect state of masthead items, but is this the right semantic?
@mcarrano good question... I don't see why it would necessarily need to be tied to the notification drawer. It's basically just a plain variant button with unique background/text color. It could be used as tabs or something, too, or maybe some sort of filter.
It's worth noting that besides the naming being more flexible between selected and expanded, there is a bit of a code difference between the two, in that at a minimum, an expanded toggle would have aria-expanded="true" and a selected toggle, at least in some circumstances, would not. So it may be useful to have both - or maybe that's confusing? I'm curious @thatblindgeye's thoughts on that.
I suppose in the case of the badge being selected in a tab-like scenario, another prop could be introduced to apply the same styling (or apply "selected" styling that is default the same), but apply possibly different aria attributes. That would I assume require a Core PR first to add styling for a selected (or whichever, there may be a better name for that sort of badge) modifier.
I think expanded works for this particular use-case of a badge causing a drawer to open or close. If we anticipate (or if we just want to offer the option for) a notification badge being used outside of a drawer, I can update this PR so that:
- The current examples are somewhat reverted, basically just removing the
isExpandedprop - A new example gets added, describing when to use the isExpanded prop and linking to notification drawer demos
I could see the notification badge being used in non-drawer scenarios, since any props passed into it are going to be applied to the Button that is used under the hood. So a consumer could pass in href and component="a" props to turn the notification badge into a link. I guess it just depends if we anticipate that being a significant situation?
OK. Let's stick with what we've got for now and just add the text to clarify what the example is demonstrating. Sound good?
@tlabaj @mcarrano Updated the example descriptions to mention the isExpanded prop. Also updated the prop description to mention the expanded styling as well as it setting the aria-expanded attribute.
Would it be more correct to say "indicate the drawer is expanded?"
@mcarrano It's definitely a little trickier here since the badge does get the aria-expanded attribute set based on this prop, but it also isn't technically the badge that is expanding.
Maybe, "...set the aria-expanded attribute on the notification badge and to indicate that a notification drawer is expanded"?
How about, "...set the aria-expanded attribute and apply visual styling on the notification badge and to indicate that a notification drawer is expanded"?
@mcarrano Updated the description! Thanks for that last suggestion