Skip to content
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

Notification usage updates #1081

Merged

Conversation

jillianhowarth
Copy link
Contributor

Updates the notification component usage guidance to follow the new template.

@jillianhowarth jillianhowarth requested review from a team April 2, 2020 14:02
@jillianhowarth jillianhowarth requested a review from a team as a code owner April 2, 2020 14:02
@jillianhowarth jillianhowarth requested review from janchild, vpicone and andreancardona and removed request for a team April 2, 2020 14:02
@vercel
Copy link

vercel bot commented Apr 2, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/carbon-design-system/carbon-website/muwf8f0wj
✅ Preview: https://carbon-website-git-fork-jillianhowarth-notification-usag-5d89b8.carbon-design-system.now.sh

@ghost ghost requested review from abbeyhrt and emyarod and removed request for a team April 2, 2020 14:03
@netlify
Copy link

netlify bot commented Apr 2, 2020

Deploy preview for carbon-website ready!

Built with commit 84ec602

https://deploy-preview-1081--carbon-website.netlify.app

@jillianhowarth jillianhowarth requested review from aagonzales and jeanservaas and removed request for vpicone, andreancardona, abbeyhrt and emyarod April 2, 2020 14:03

### Dismissal

We recommend that toast notifications automatically disappear after five seconds. Inline notifications are persistent until the user dismisses them. All notifications have at least one method of dismissal which is typically a small “x” in the upper right hand corner.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Template question: Why are we sometimes making h4s for the variants like in placement but then sometimes just talking about them together in one paragraph like here. Is that going to always be the recommendation for this sections?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Behavior comment: The 'x' can actually be removed/hidden on inline notifications and is part of the component code that I think we'll need to account for and explain here about when removing the "close" feature is allowed.


Carbon supports high and low constrast style notifications. High-contrast notifications are best for critical messaginging while low-contrast notifications are less visually disruptive for users.

It's' up to the product team to decide which notification style to use in their product. Inline and toast notifications can use different styles but you should never mix styles within each notification type. When in doubt, use low-contrast notifications.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: It's' too many apostrophes?

@alisonjoseph alisonjoseph requested a review from a team April 2, 2020 18:20
@ghost ghost requested review from joshblack and tw15egan April 2, 2020 18:20
@vercel vercel bot temporarily deployed to Preview April 8, 2020 20:28 Inactive
Copy link
Contributor

@janchild janchild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stellar job on this, Jillian! I really like the way you handled the Placement section for Inline notifications. Great work! 🎉 🎉 🎉

I added a couple of copyedits for you but I'm ready to approve. Thank you!

src/pages/components/notification/usage.mdx Outdated Show resolved Hide resolved
src/pages/components/notification/usage.mdx Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview April 9, 2020 14:11 Inactive

We recommend that toast notifications automatically disappear after five seconds. Inline notifications are persistent until the user dismisses them. All notifications have at least one method of dismissal (typically, it is a small “x” in the upper right hand corner).
- Keep labels concise and clearly indicate the action the user can take.
- Limit action labels to one or two words. For a list of approved action labels, see Carbon's [content guidelines](https://www.carbondesignsystem.com/guidelines/content/action-labels).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was surprised to see approved here, is there a process through which teams get labels approved? Or are these recommendations by our team?

Copy link
Contributor

@janchild janchild Apr 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These labels have been developed through multiple rounds of review with Cloud Data & AI, IoT, and Watson designers and content designers. If there were any changes requested, it would need to go back to this group for discussion and approval or not. The list has been stable for a period of time and so yes, "approved" is the correct word as opposed to "recommended".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion - should read; "For a list of recommended labels...". I'm not sure "approved" is correct here, in that it implies anything else is non-approved (but we don't have any real control over the action labels different teams might use).

Process for choosing which labels are recommended is outside the scope of this PR, but we can discuss if there is something we want to document/clarify.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's implying that labels have to be approved before using, but merely saying the action labels on that content page are approved to use. Maybe it should say "For a list of suggested action labels, ..."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be stronger than just suggested. If people have concerns with "approved" then I'd like to go with "recommended".

Copy link
Member

@aagonzales aagonzales Apr 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops sorry y'all my internet is terrible and for some reason no one else comments loaded until now so I wasn't purposefully ignoring any earlier comments.


Toast notifications can include a link at the end of their body content. This link should be short and navigate users to a page or modal where they can take action to address the notification or find further information. Because toast notifications automatically dismiss, there should be alternative routes to navigate to the link destination.

If you include links within the notification body that redirect the user to steps, be sure keyboard and screenreader users have enough time to access the notification or can easily find the notification after it disappears.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it seems like we would want to recommend that notifications don't have any interactive content within them. When using a live region, the text of the area is what is communicated to the end user:

demo

As a result, there is no way for them to know if the content is interactive or not nor is there an easy way for them to navigate to it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More specifically. Is the feedback here that this guidance should change because we don't/can't include links in notifications?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically speaking something that we currently allow and show in the storybook. She is only commenting on what we're currently allowing. If we don't want to allow this then we need to also remove this from the component code.

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aagonzales definitely agreed, I think this needs to be a conversation for us all to better understand what is/is not applicable here. It feels odd given that so many sites use this pattern that this is how a screen reader behaves, maybe I'm misunderstanding something? Was surprised when I was testing it out in that recording above that it announced the way that it did.

@dakahn do you have any insights here? Clearly this is a common pattern, including notifications at the OS-level, is there anything we can do to accommodate interactive content either through a link/action button/close button?

Copy link
Contributor

@dakahn dakahn Apr 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At IBM we have the requirement that any mouse interaction have a keyboard only equivalent. With notifications that popup and disappear (toasts or whatever) and have interactable child elements this rule becomes problematic.

How do we move keyboard focus to that notification? If this were a modal dialog we'd simply programmatically steal the users focus and move it to the dialog when the modal was opened and then trap focus in the dialog until an action was taken. When the modal is closed we then programmatically return focus to the element used to originally trigger the modal.

Toasts or notifications are non-modal dialogs and come and go perhaps on a timer as the user works and are intended to be unobtrusive so the above described interaction pattern doesn't really work.

At the application level there are ways to solve around this problem by having notifications collected somewhere that is keyboard accessible like this:

image

but its always highly inadvisable to use operating system level interactions as guidance for building websites since -- in short -- they are very different contexts and only superficially comparable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dakahn it seems like the guidance from WAI-ARIA gets distilled into two categories: alerts and alert dialogs:

An alert is an element that displays a brief, important message in a way that attracts the user's attention without interrupting the user's task
Because alerts are intended to provide important and potentially time-sensitive information without interfering with the user's ability to continue working, it is crucial they do not affect keyboard focus.

The alert dialog is designed for situations where interrupting work flow is necessary.


Alerts are what we can use for time-based notifications, but if there is anything interactive then we need to use the alert dialog?

What happens if we want to use a timed alert, but also want to give someone the ability to dismiss it early? This seems like it'd directly contradict the alert guidance as it says that it shouldn't steal focus from the user.

Links & resources

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if we want to use a timed alert, but also want to give someone the ability to dismiss it early?

They're highly problematic for sure. You can minimize a lot of these gnarly UX frustrations for keyboard/screen reader/magnification users, but some of those solutions cause frustrations for other (dialogs with interactable elements becoming modal for example solves one problem but creates another).


#### Placement

Toast notifications slide in and out from the top right of the screen. They align to the edge of the last column and can stack with `$spacing-03` in-between. New toast notifications should appear at the top of the list, with older notifications being pushed down until they are dismissed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the height of the stacked notifications is greater than the browser height?

Notifications are typically absolutely positioned, as a result, how would one implement this guidance:

They align to the edge of the last column

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification on @joshblack's last point; "absolute positioning" means that an element doesn't reference other elements, so the "align to the edge of the last column" is difficult/impossible to apply from a code perspective.

Do we need to either address the design guidance, or address the development implementation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point that we don't currently have a solution for. This will need to be opened as a issue to figure out. I also think if they're disappearing after x seconds the chances of there being a long stack of them seems very unlikely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshblack do you have a suggestion for how to provide the positioning guidance? I was trying to address this issue #76

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jillianhowarth it seems like we could re-word it to say that the notifications are aligned vertically and use the spacer in between and not mention the edge alignment? Just a thought 👀

src/pages/components/notification/usage.mdx Show resolved Hide resolved

#### Dismissal

Toasts dismiss automatically after five seconds on the screen. They can also include a close button so users can dismiss them sooner. Toasts cover content on the screen so they should always be easily dismissed. Because toast notifications dismiss automatically, users should be able to access them elsewhere after the toast disappears. This allows them to be accessible for users who need more time reading the notification or who may want to refer back to the notification.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because toast notifications dismiss automatically, users should be able to access them elsewhere after the toast disappears

Should we word this more strongly to say that there must be an alternate location if the toast is automatically dismissed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It definitely seems like it should be a requirement rather than a preference, right? (so "users must be able to access..." rather than "users should...")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would simply changing "should be" to "must be" be enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so.

Copy link
Contributor Author

@jillianhowarth jillianhowarth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshblack all of your comments above have been addressed in the updates with the solutions discussed above. Like I mentioned in slack, we’re calling out the current accessibility gaps with interactive content, but addressing those gaps in not within the scope of this PR

@joshblack
Copy link
Contributor

@jillianhowarth definitely didn't mean to suggest that we should solve all of that in this PR, sorry if it came across that way!

Finally caught up on that one thread in Slack and left a comment, let me know if you want to chat there or if review comments here would be preferred 👍

@janchild janchild merged commit ac820ab into carbon-design-system:master Apr 16, 2020
natashadecoste pushed a commit to natashadecoste/carbon-website that referenced this pull request May 19, 2021
* Docs: notification guidance updates

* docs: guidance updates

* Docs: final updates

* Docs: image fix

* Docs: structure updates

* Docs: update content

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Docs: image and content chagnes

* docs: minor content change

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Docs: image and content updates

* Docs: typo fix

* Docs: anatomy updates

* Docs: a11y update

* Docs: a11y updates

Co-authored-by: TJ Egan <[email protected]>
Co-authored-by: Jan Child <[email protected]>
natashadecoste pushed a commit to natashadecoste/carbon-website that referenced this pull request May 19, 2021
* Docs: notification guidance updates

* docs: guidance updates

* Docs: final updates

* Docs: image fix

* Docs: structure updates

* Docs: update content

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Docs: image and content chagnes

* docs: minor content change

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Update src/pages/components/notification/usage.mdx

Co-Authored-By: Jan Child <[email protected]>

* Docs: image and content updates

* Docs: typo fix

* Docs: anatomy updates

* Docs: a11y update

* Docs: a11y updates

Co-authored-by: TJ Egan <[email protected]>
Co-authored-by: Jan Child <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants