feat(Notification Badge): Adds the notification badge component - #2342
Conversation
Sorry, something went wrong.
Added the notificatoin badge component. Currently adding the typescript integreation tests. Will update the PR when they are done. "fix #2021"
|
PatternFly-React preview: https://patternfly-react-pr-2342.surge.sh |
Sorry, something went wrong.
| } | ||
| onClick = () => { | ||
| this.setState({ | ||
| isRead: true |
There was a problem hiding this comment.
Could go with isRead: !this.state.isRead to allow toggling.
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks @redallen I figured once you set it you wouldn't want to show unread again, but if we want I can update the example to have it toggle. Just let me know I'm fine either way.
Sorry, something went wrong.
| }; | ||
| this.onClick = () => { | ||
| this.setState({ | ||
| isRead: true |
There was a problem hiding this comment.
Same with the example.
Sorry, something went wrong.
mcarrano
left a comment
There was a problem hiding this comment.
@dlabaj Visually this looks good, but I do have a question about behavior. This is intended to work as a toogle for opening and closing the notification drawer. So when the drawer is open it would display a selected state until the user clicks it again to close the drawer. It's hard to tell from the preview of the icon only, but it looks like this is coded to behave link a standard action button.
Here is the latest visual spec: https://marvelapp.com/project/4090736/
Sorry, something went wrong.
|
@mcarrano I don't have access to that visual spec above |
Sorry, something went wrong.
|
@mcarrano Right now this is an external component that can also be used outside of the notification drawer. Is the circle around it when it's toggled on suppose to be shown when it's not used with the notification drawer? @christiemolloy @mcoker I don't see the toggled on circle in the core example. https://pf4.patternfly.org/components/NotificationBadge/examples/ Is this being added in a modifier that we will get later? |
Sorry, something went wrong.
|
@mcarrano Spoke with Mary and she's going to add that selected state when the drawer is open to this issue patternfly/patternfly#1832. When we get this in core and it is time to add it to react we will add that to the notification badge in the masthead at that time. Does that sound good? |
Sorry, something went wrong.
|
This looks great @dlabaj and we hadn't addressed the circle background behind the icon yet because we still need to discuss how to best implement it in Core. I'm going to create a separate issue for it because patternfly/patternfly#1832 is large and addresses different concepts. Issue here: patternfly/patternfly#1998 ... @rachael-phillips making you aware. |
Sorry, something went wrong.
|
@dlabaj @christiemolloy this all makes sense and thanks for identifying the approach here. I that case I will go ahead and approve of this PR. |
Sorry, something went wrong.
mcarrano
left a comment
There was a problem hiding this comment.
I'm good to merge this when ready.
Sorry, something went wrong.
tlabaj
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
What:
Adds the notification badge component to react core.
Additional issues:
#2021