-
-
Notifications
You must be signed in to change notification settings - Fork 565
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
feat: enhancements to profile account notifications #1127
feat: enhancements to profile account notifications #1127
Conversation
Run & review this pull request in StackBlitz Codeflow. |
✅ Deploy Preview for elk-docs canceled.
|
Thanks @jeffsikes to work with others on this feature. We're going to be able to move a lot faster if we start to do cross-reviews like this. @rshigg could you add a review to this PR before we merge? |
✅ Deploy Preview for elk-zone ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
Great work on this!
This PR adds a tooltip to the Account Header's new notify button. The tooltip text changes as the notify button is toggled from on to off.
Additionally, the icon has changed from a "jingle bell" to a "notification bell" icon which is consistent with the concept of "notifications" in Elk (the same type of icon is used for global notifications as well). This was a discussion that occurred on Jan 15th in the UI Discord channel.
Fixes #1124
Huge thanks to @rshigg for working on these changes with me, as we both worked on similar parts of the code in separate PRs! 🙌🏼
The videos below show all 4 states of the notification button:
Before
Screen.Recording.2023-01-14.at.10.36.58.PM.mov
After
Screen.Recording.2023-01-15.at.3.50.50.PM.mov
Tooltip Screenshots