docs(alert-dialog): updated docs #5371
Conversation
|
nikkimk
changed the title
Branch previewReview the following VRT differencesWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
If the changes are expected, update the |
Tachometer resultsCurrently, no packages are changed by this PR... |
packages/alert-dialog/README.md
Outdated
| - <kbd>Space</kbd>/<kbd>Enter</kbd>: Activate the focused button | ||
| - <kbd>Esc</kbd>: Close the dialog | ||
| - Maintains proper focus management when opened and closed | ||
| - Uses ARIA roles and attributes appropriately: |
There was a problem hiding this comment.
Should our examples have role defined based on this guidance?
There was a problem hiding this comment.
@caseyisonit IMHO this should be part of what the component does for the consumer, rather than the onus be on the consumer. Also, I'm interested in having a11y auditors confirm that modal alerts are alertdialog because they interrupt user action where alert roles do not.
| This is the default variant for alert dialogs. Use a confirmation variant for asking a user to confirm a choice. | ||
| - A heading, using `slot="heading"`, that describes the purpose of the dialog | ||
| - Content, using the default slot, that provides additional context | ||
| - Action buttons, using `slot="button"`, that allow users to respond |
There was a problem hiding this comment.
| - Action buttons, using `slot="button"`, that allow users to respond | |
| - **Action Button(s):** | |
| - Use `slot="button"` to render your action button(s) that allow users to respond | |
| - An alert dialog must have one primary action button with the option to include a secondary action and/or a cancel action. | |
| - Non-primary action buttons should be `variant="secondary"` and `treatment: "outline"`. | |
| - The three buttons should be rendered in the DOM in the following order: | |
| - **Cancel action:** Offers an option to go back and cancel the action. | |
| - **Secondary action:** Offers a secondary action. e.g. "Remind me later" | |
| - **Primary action:** The first (right-most) button communicates what the button will do if selected, or to acknowledge and dismiss the dialog. Check [variants](#variants) for the correct primary button styling. |
There was a problem hiding this comment.
Hey, I'm wondering if having 3 buttons in the dialog is the right approach. Would you mind checking in with Nate Baldwin about this? I believe he's working on a more comprehensive dialog RFC that addresses a lot of the related concerns. It might offer some useful guidance and help us align better with the direction he's setting.
| </sp-tab-panel> | ||
| </sp-tabs> | ||
|
|
||
| ### Behaviors |
There was a problem hiding this comment.
Lets add a behavior section that should the alert dialog in an overlay with the correct accessibility set up. we can make a follow up ticket to add it to storybook and clean up the button spacing issue
packages/alert-dialog/README.md
Outdated
| @@ -154,7 +199,6 @@ Destructive alert dialogs are for when a user needs to confirm an action that wi | |||
| </sp-button> | |||
| <sp-button | |||
| slot="button" | |||
| id="confirmButton" | |||
| variant="negative" | |||
| treatment="fill" | |||
There was a problem hiding this comment.
| treatment="fill" | |
| treatment="outline" |
aligning to the design specs
Co-authored-by: Casey Eickhoff <[email protected]>
Co-authored-by: Casey Eickhoff <[email protected]>
Co-authored-by: Casey Eickhoff <[email protected]>
| This is the default variant for alert dialogs. Use a confirmation variant for asking a user to confirm a choice. | ||
| - A heading, using `slot="heading"`, that describes the purpose of the dialog | ||
| - Content, using the default slot, that provides additional context | ||
| - Action buttons, using `slot="button"`, that allow users to respond |
There was a problem hiding this comment.
Hey, I'm wondering if having 3 buttons in the dialog is the right approach. Would you mind checking in with Nate Baldwin about this? I believe he's working on a more comprehensive dialog RFC that addresses a lot of the related concerns. It might offer some useful guidance and help us align better with the direction he's setting.
@Rajdeepc We (@caseyisonit and me) pulled it directly from the design guidelines. |
Co-authored-by: Rajdeep Chandra <[email protected]>
Co-authored-by: Rajdeep Chandra <[email protected]>
|
@Rajdeepc and @caseyisonit PTAL I added a link to reference design docs on variants and button options, to address Rajdeep's feedback. |
I have a follow up question. If there are 3 buttons in a frame how the focus trap should work? Should the immediate focus goes to the primary call to action or to the first button element in the DOM? According to me it should go to the first primary call to action but we have a different implementation in our storybook. |
I can definitely ask when this component gets audited. As a reminder, the scope of this PR is to document the component as-is, so that the auditors can review our current implementation and submit issues, so this shouldn't hold back the docs. |
packages/alert-dialog/README.md
Outdated
| Information alert dialogs communicate important information that a user needs to acknowledge. Before using this kind of alert dialog, make sure it’s the appropriate communication channel for the message instead of a toast or a more lightweight messaging option. | ||
| Use `slot="button"` to render your action button(s) that allow users to respond | ||
|
|
||
| - An alert dialog must have one primary action button (with `variant="secondary"`) with the option to include a secondary action and/or a cancel action. |
There was a problem hiding this comment.
Is this correct for the primary button to have variant="secondary"?
Non-primary action buttons should be
variant="secondary"andtreatment: "outline".
There was a problem hiding this comment.
Nice catch, @cdransf! Pushing a correction.
packages/alert-dialog/README.md
Outdated
| Use `slot="button"` to render your action button(s) that allow users to respond | ||
|
|
||
| - An alert dialog must have one primary action button (with `variant="primary"`) with the option to include a secondary action and/or a cancel action. | ||
| - Non-primary action buttons should be `variant="secondary"` and `treatment: "outline"`. |
There was a problem hiding this comment.
Should this be treatment="outline"?
packages/alert-dialog/README.md
Outdated
| - The three buttons should be rendered in the DOM in the following order: | ||
| - **Cancel action:** Offers an option to go back and cancel the action. | ||
| - **Secondary action:** Offers a secondary action. e.g. "Remind me later" | ||
| - **Primary action:** The first (right-most) button communicates what the button will do if selected, or to acknowledge and dismiss the dialog. Check [variants](#variants) for the correct primary button styling. See also the (Alert Dialog design options)[https://spectrum.adobe.com/page/alert-dialog/#Options]. |
There was a problem hiding this comment.
I think this link syntax should be swapped.
[Alert Dialog design options](https://spectrum.adobe.com/page/alert-dialog/)
packages/alert-dialog/README.md
Outdated
|
|
||
| #### Context | ||
|
|
||
| An alert dialog should be placed inside a model overaly: |
packages/alert-dialog/README.md
Outdated
| ### Accessibility | ||
|
|
||
| When implementing an alert dialog: | ||
|
|
||
| - Use concise, meaningful dialog title that clearly states the purpose | ||
| - Use semantic heading elements (`<h2>`) for the dialog title | ||
| - Ensure button labels clearly indicate the action they will perform | ||
| - Provide clear, concise content that explains the situation and required actions | ||
| - Consider the appropriate variant based on the message's importance and urgency |
There was a problem hiding this comment.
I love this section here. Should we add any suggestions on using aria-labelledby or aria-describedby for the heading and description of the AlertDialog and maybe show some of those in the code highlights above?
There was a problem hiding this comment.
Thanks @aramos-adobe!
Description
Improving the accessibility documentation of components.
Related issue(s)
Motivation and context
Documentation should provide more information and examples that demonstrate how to use the components accessibly.
How has this been tested?
Review alert dialog docs
Do the docs examples give enough context to test for accessibility?
Do the docs examples and text provide information on how to use the component accessibly?
If the component is to be used in the context of another component, do the examples include how that component is used accessibly in that context?
Are the docs headings logical and consistent across these components? You can use the WAVE browser extension's Structure tab to review heading structure.
Did it pass in Desktop?
Did it pass in Mobile?
Did it pass in iPad?
Screenshots (if appropriate)
Types of changes
Checklist
Best practices
This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against
main.