jpmorganchase / salt-ds

React UI components built with a focus on accessibility, customization and ease-of-use
https://www.saltdesignsystem.com
Apache License 2.0
132 stars 90 forks source link

Banner ADA review #1602

Closed maj-stella closed 11 months ago

maj-stella commented 1 year ago

Hi @dplsek and @alycrys

Thanks for showing me the banner work. Here are few points to consider:

@dplsek

@alycrys

maj-stella commented 1 year ago

Figma file: https://www.figma.com/file/8oJlx8xG3HQNjMyTwwzQvX/Salt-Banner?type=design&node-id=2234%3A12605&t=xXfan2kfYXICLmKn-1

Storybook: https://9bcce88e.saltdesignsystem-storybook.pages.dev/?path=/story/core-banner-ada--form https://9bcce88e.saltdesignsystem-storybook.pages.dev/?path=/story/core-banner-ada--text

Other examples (note: these are only examples for a design inspiration not for a build as they might not be accessible): Vibe https://60b3813c9e93660049cec98b-lmuvkivqmh.chromatic.com/?path=/story/feedback-alertbanner--overview

GitLab https://gitlab-org.gitlab.io/gitlab-ui/?path=/story/base-banner--default

D2iQ https://d2iq.github.io/ui-kit/?path=/story/feedback-promobanner--default

AnyVision http://storybook.anyvision.co/?path=/story/user-feedback-banner--default

AppNexus https://appnexus.github.io/lucid/?path=/story/communication-banner--basic

Kiwi com https://kiwicom.github.io/orbit/?path=/story/calloutbanner--default

GovUK https://5ad61aab7f00090020dfdba8-bakpheneuz.chromatic.com/?path=/story/phase-banner--default

US Department of Veteran's Affairs https://design.va.gov/storybook/?path=/story/components-va-banner--dismissible

Shopify https://5d559397bae39100201eedc1-jgtvpgocvh.chromatic.com/?path=/story/all-components-banner--default

Phork https://6195b518b76f57003aa69b4c-dzkgzotbvz.chromatic.com/?path=/story/feedback-banners-banner--default

Skyscanner https://backpack.github.io/storybook/?path=/story/bpk-component-banner-alert--docs-default

LaunchDarkly https://626696a2018c1f004a1cde86-ofodaazvvv.chromatic.com/?path=/story/components-banner--error

maj-stella commented 1 year ago

Hi @dplsek
We have tree different scenarios for a banner.

  1. Content does not have any interactive elements and it’s always on the page
  2. Content has interactive elements such as a link, close button, etc
  3. Banner appears when a user performs an actions

Please could you add these scenarios to the Spec?

Also I have a question about the icon at the start. Is this icon always visible? I’m asking so I can decide how a banner is going to be identified.

Thanks

alycrys commented 1 year ago

@maj-stella It has been decided that the icon is always visible

dplsek commented 1 year ago

Thanks @maj-stella ... I've added the scenarios here: https://www.figma.com/file/8oJlx8xG3HQNjMyTwwzQvX/Salt-Banner?type=design&node-id=2284%3A4504&t=ryq2gUowpLeIoKnl-1

but I'm not sure how #2 is different from #3 or what to call it if there is a difference? Let me know if you have an idea of how to capture it or what we should label the scenario... I've just called it 'Post user action' for now. Appreciate your help!

maj-stella commented 1 year ago

Hi @dplsek,

Regarding your question. Please see the 3rd point below. This examples shows what I meant in the 3rd point described previously.

Here are some suggestions for defining a banner:

1) I would suggest to take away the example ‘general’ information as it is static banner on a site. Please see screenshot below: The reason is that that we are mixing two different cases, the first banner is more like informational, and all other banners are dynamic, they use aria to alert/notify a user when they appear on the site.

I have checked banner in UITK, they have only banners which use aria, not a static banner.

Image

2) Could a spec document specify what kind of banner we are building? Please see consideration which Carbon included in their documentation:

Patterns - Notification: https://carbondesignsystem.com/patterns/notification-pattern/ Components - Notification: https://carbondesignsystem.com/components/notification/usage/

We don’t need to cover all their considerations but some basics. They are also other DS which have an alert/notification/banner component which are worth checking.

3) When we are clear what banner is, it would be great to have/build some simple usable examples. Please see one example (screenshot) below. I have encountered that example when I submitted HR question today.

Image

dplsek commented 1 year ago

Hi @maj-stella thanks for the follow-up. With you and Alina OOO today, Josh and I worked together to better capture your points and updated the interactive spec and accessibility spec

In a nutshell, we have broken banner down like this:

Types:

  1. Static
  2. Interactive

Note: other than the interactive elements, these banners are the same.

Usage:

  1. System generated
  2. User generated

Status options:

  1. Info
  2. Warning
  3. Error
  4. Success

I will be OOO next week so feel free to edit the Figma document as you feel necessary. I've sent an invite to everyone with access @bhoppers2008 @alycrys @joshwooding @parsakhan21