-
Notifications
You must be signed in to change notification settings - Fork 103
Notification Drawer Updates #297
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
Conversation
@yih-wang @serenamarie125 @mcarrano @LHinson @lizsurette Please take a look at the notification drawer updates when you get a chance. Thanks! |
nice job @beanh66 no comments here. +1 from my end. |
@beanh66 For the mockup of Responsive State, it seems like there's an extra layer below "Label 3". Can you remove it? Thanks. |
1. **Icon:** Displays visual differentiator when new or unread notifications are present. Clicking on the icon will slide out a drawer and dismiss the toast notification. Clicking on the icon again will close the drawer. | ||
1. **Icon:** | ||
- The fa-bell icon should be used to represent the notification drawer. | ||
- Clicking on the icon will slide out a drawer and dismiss the toast notification. |
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.
Nit picky, but I'm wondering if this should be "Drop down" rather than "Slide out". Or maybe I'm mistaken and it does slide out! Also, maybe this should say "If there is a toast notification, it will be dismissed on click."
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.
You're right, it drops! See: https://rawgit.com/patternfly/patternfly/master-dist/dist/tests/notification-drawer-vertical-nav.html
Regarding the toast notification, looks like it obstructs the bell and requires the user to dismiss first. Is that what you mean by the latter @lizsurette ?
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.
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.
@LHinson @lizsurette This actually wasn't something I added or changed, but I agree with your comments! I'll update to go with "drop down." Per the toast notifications, unfortunately they do block the bell, which originally I thought was a bug. @jeff-phillips-18 @serenamarie125 didn't this come up with MIQ and they ended up moving the notifications down below the icon or did I make that up?
@beanh66 - I added one nitpicky comment, but overall I think this looks awesome. |
into notificationDrawer * 'master' of https://github.com/patternfly/patternfly-design: Remove line break causing formatting error in Markdown Corrected capitalization Minor sentence structure changes. Login page images update
@lizsurette @LHinson @yih-wang Thanks for reviewing 👍 I made the updates discussed. |
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.
This looks great @beanh66. I only had one question. I see that the badge on the bell icon does not have a number inside. Is that by intent? Wasn't sure it you wanted to report the # for new events or if this is only to reflect the presence of any new events.
@mcarrano having no number in the badge is intentional. Github does the same thing. Currently many of our products are adding the notification drawer to an existing product which is overloaded with inline and/or toast notifications. Most products that i have seen are first pushing everything to the notification drawer, and then taking further steps in fixing the back end and/or thinkin of a notification management system to allow users to turn things off, etc. For this reason ( scalability ), we chose to use this design in CloudForms/ManageIQ, RHV, OpenShift & Satellite. |
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.
Looks great!
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Implementation of patternfly/patternfly-design#297
Description
Changes