-
Notifications
You must be signed in to change notification settings - Fork 793
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
docs (notifications): usage and style guidance for Callout #4218
docs (notifications): usage and style guidance for Callout #4218
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
DCO Assistant Lite bot All contributors have signed the DCO. |
I have read the DCO document and I hereby sign the DCO. |
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.
The docs are looking good, just a few small tweaks
- The anatomy image should be 12 columns instead 8.
- See my other inline comments
And then one question:
Do we know when the name change will be made in code? Are we going to wait for that to merge these changes in? If not, then we need to add some information about Callout currently being called StaticNotification
in experimental.
Hi @aagonzales , I have fixed the issues that you mentioned. Can you please review the changes again. Thank you. |
Hi @aagonzales I have updated the anatomy image as requested. Please share your review. Thank you. |
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 good to go! Good job!
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.
Made a few minor edits just for clarity. Otherwise looks good!
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.
Thanks for making the updates @benjamin-kurien ! LGTM
PR ready to merge. |
Closes #3913
Usage & style document for Notification component has been updated to include guidance for a 4th variant named Callout.
Changelog
New
Changed
Removed