-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Snackbar: Update design documentation and add accessibility documentation. #16038
Conversation
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.
overall, following design guidelines depends on the team, while I'm onboard about the important of design, I'm not sure if we should:
- remove code for the sake of following design, therefore not allowing anything multiple buttons or custom buttons
- leaving the code as is and adding linter warning about it
|
||
![](https://wordpress.org/gutenberg/files/2019/06/snackbar-action-dont2.png) | ||
|
||
**Don’t:** Do not use more than one action button in a snackbar. |
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 line conflicts with the code documentation on line 146
actions
: (array) an array of action objects. Each member object should contain alabel
and either aurl
link string oronClick
callback function. AclassName
property can be used to add custom classes to the button styles.
I kinda agree more with the design one, snackbar are meant to be informative and low priority, having multiple action would be confusing taking into account the 10s choice period.
while the design docs tells you what you should probably do, the code docs describe what exist and makes no assumption, so I think the actual code should be updated? @youknowriad @kjellr what do you think?
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.
I think the code is as is to be consistent with the Notice
component as with the core/notices
package, you can just change the "type" of the dispatched action to switch between regular notices and snackbars.
So, I'm on the fence because consistency in APIs is a good thing as well. So I may be leaning towards keeping this as a guideline and not something strict in the API.
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 is a great point — thanks for raising it. From a design perspective, I think I'd prefer we were a little more strict about this sort of thing. Mostly because we know that 3rd parties will most certainly abuse the available options and create really distracting/confusing snackbars if we don't.
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.
@senadir I think you can attempt a strict refactoring of the props. This would have impact on the notices package though :)
|
||
![](https://wordpress.org/gutenberg/files/2019/06/snackbar-action-dont1.png) | ||
|
||
**Don’t:** Do not use buttons inside of snackbars. |
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 also falls in the same area as the other comment, class names should not be available to customization because the button should not be customized
A
className
property can be used to add custom classes to the button styles.
@youknowriad, I can refactor the snackbar component if we're following the strict approach |
The PR was brought up during a Gutenberg Design Triage. @annezazu |
No major comments on my end except that I want to be sure proper accessibility feedback has been incorporated and everything is up to date especially since this is a year old. Otherwise, looks good to proceed with and can be iterated on in the future :) |
I am wondering if there should be a "undo" operation option? For instance: |
@critterverse thoughts on whether to just close this out or push forward and merge? I'm assuming these updates are still in line with current design thinking :) |
@annezazu Merging this makes sense to me! This documentation aligns nicely with the guidelines around snackbar usage in the WordPress Design Library. |
This was never moved forward and, two years on, I worry how out of date it is. I'm going to close this out as a result and encourage folks who want to continue working on it to open a new PR. |
This PR adds design and accessibility guidelines for the Snackbar component. This is in addition to the development documentation (and very light design documentation) that existed previously. These guidelines are 'best practices' for the usage of the component, and they describe the component in more detail.
You can preview the updated document here:
https://github.com/WordPress/gutenberg/blob/fa16fa5f777af29958201b320185dc7601c349da/packages/components/src/snackbar/README.md
Thanks @drw158 and @sarahmonster for help drafting this. Looking forward to feedback from everyone! We'd especially love some feedback from folks on the accessibility team, on whether this covers the most important guidelines.