feat(accessible-names): require accessible names in non-text wrapping components #732

nataliadelmar · 2024-12-12T19:52:53Z

Adding type requirement or console.warning to components that need an accessible name:

Required type:

<AriaToolbar /> SPARK-583000
<List /> SPARK-582998
<NavigationTab />
<Reaction />
<Slider />
<Tree /> SPARK-582999
<ToastNotification /> (require close button label)

Console.warn (just because they could trigger a Tooltip type="label"):

<AddReactionButton /> SPARK-582980
<ButtonCircle /> SPARK-582980
<ButtonCircleLink /> SPARK-582980
<ButtonCircleToggle /> SPARK-582980
<ReactionBadge /> SPARK-582980
<ReactionButton /> SPARK-582980

… components

nataliadelmar · 2024-12-13T11:01:39Z

src/components/AddReactionButton/AddReactionButton.tsx


 const AddReactionButton = forwardRef((props: Props, ref: ForwardedRef<HTMLButtonElement>) => {
  const { className, id, style, ...otherProps } = props;
  delete otherProps.size;
+
+  useCheckAriaLabel('AddReactionButton', props);


we can't do type requirement because some (most) times, ButtonCircles should be labelled by its Tooltip. So ButtonCircle would not have aria-label or aria-labelledby directly and TS will complain

I have added a story to Tooltip to prove that we don't get a console warning only when we trigger a label tooltip from the ButtonCircle

nataliadelmar · 2024-12-13T11:02:47Z

src/components/AriaToolbar/AriaToolbar.tsx

    className,
    id,
    style,
    children,
    orientation = DEFAULTS.ORIENTATION,
    shouldRenderAsButtonGroup = DEFAULTS.SHOULD_RENDER_AS_BUTTON_GROUP,
    onTabPress,
-    ariaControls,


we have decided not to use camelCase anymore on aria properties, to keep them consistent. I know this would cause a breaking change, so Im preparing the Cantina PR for it.

nataliadelmar · 2024-12-13T11:07:44Z

src/components/AriaToolbar/AriaToolbar.types.ts

@@ -67,6 +59,8 @@ export interface Props {
  ariaToolbarItemsSize: number;
 }

+export type Props = AriaToolbarProps & AriaLabelRequired;


SPARK-583000 aria-toolbar gives the wrapper a role of 'toolbar'. The SR will read toolbar with an empty label if the developer doesn't pass any aria-label(lledby) to the Toolbar.

nataliadelmar · 2024-12-13T11:21:32Z

src/components/ButtonCircle/ButtonCircle.tsx

@@ -24,6 +25,8 @@ const ButtonCircle = forwardRef((props: Props, providedRef: RefObject<HTMLButton
    console.warn('MRV2: Momentum does not support a ghost inverted ButtonCircle.');
  }

+  useCheckAriaLabel('ButtonCircle', props);


ButtonCircle can be labelled by a tooltip, so we won't require aria-label(ledby) to it. I have added a story to Tooltip that we don't get a console warning only when we trigger a label tooltip from the ButtonCircle

nataliadelmar · 2024-12-13T11:22:15Z

src/components/ButtonCircleLink/ButtonCircleLink.tsx

@@ -26,6 +27,8 @@ const ButtonCircleLink = forwardRef((props: Props, providedRef: RefObject<HTMLAn
    ...otherProps
  } = props;

+  useCheckAriaLabel('ButtonCircleLink', props);


same as ButtonCircle. ButtonCircle can be labelled by a tooltip, so we won't require aria-label(ledby) to it. I have added a story to Tooltip that we don't get a console warning only when we trigger a label tooltip from the ButtonCircle

nataliadelmar · 2024-12-13T11:22:22Z

src/components/ButtonCircleToggle/ButtonCircleToggle.tsx

@@ -23,6 +24,8 @@ const ButtonCircleToggle = forwardRef((props: Props, providedRef: RefObject<HTML
    ...otherProps
  } = props;

+  useCheckAriaLabel('ButtonCircleToggle', props);


Same as ButtonCircle. ButtonCircle can be labelled by a tooltip, so we won't require aria-label(ledby) to it. I have added a story to Tooltip that we don't get a console warning only when we trigger a label tooltip from the ButtonCircle

nataliadelmar · 2024-12-13T11:33:16Z

src/components/ButtonControl/ButtonControl.tsx


 /**
 * `<ControlButtons />` are used to control [close, maximize, minimize, etc] components [usually panels] they are assigned to.
 */
 const ButtonControl = forwardRef((props: Props, providedRef: RefObject<HTMLButtonElement>) => {
  const { className, control, isCircular, ...otherProps } = props;

+  useCheckAriaLabel('ButtonControl', props);


Same as ButtonCircle. ButtonCircle can be labelled by a tooltip, so we won't require aria-label(ledby) to it. I have added a story to Tooltip that we don't get a console warning only when we trigger a label tooltip from the ButtonCircle

nataliadelmar · 2024-12-13T11:34:34Z

src/components/List/List.types.ts

@@ -67,6 +69,8 @@ export interface Props {
  initialFocus?: number;
 }

+export type Props = ListProps & AriaLabelRequired;


All lists should have an aria-label(lledby) SPARK-582998
This will cause a breaking change so Im working on the Cantina PR for it.

nataliadelmar · 2024-12-13T11:35:03Z

src/components/NavigationTab/NavigationTab.stories.tsx

@@ -15,6 +15,9 @@ export default {
      page: DocumentationPage(Documentation, StyleDocs),
    },
  },
+  args: {
+    icon: 'chat',


This is something I saw we missed and was causing the story to look empty

nataliadelmar · 2024-12-13T11:53:21Z

src/components/Reaction/Reaction.types.ts

+  /**
+   * Translated label of the reaction
+   */
+  'aria-label': string;


lottie only accepts a string to label the svg. If we don't pass any string, the svg is left unlabelled and you end up with an unlabelled image. SR reads "image". Nothing else.

For this reason, we are requiring aria-label to be passed as a reaction inside a meeting or even a chat will always be meaningful and worth reading to the unsighted user.

nataliadelmar · 2024-12-13T11:54:18Z

src/components/ReactionBadge/ReactionBadge.tsx


 const ReactionBadge = forwardRef((props: Props, providedRef: RefObject<HTMLButtonElement>) => {
  const { className, count, reacted, reaction, ...otherProps } = props;

+  useCheckAriaLabel('ReactionBadge', props);


Same as ButtonCircle. ButtonCircle can be labelled by a tooltip, so we won't require aria-label(ledby) to it. I have added a story to Tooltip that we don't get a console warning only when we trigger a label tooltip from the ButtonCircle

nataliadelmar · 2024-12-13T11:55:11Z

src/components/ReactionPicker/ReactionPicker.tsx


  return (
    <AriaToolbar
-      ariaLabel={ariaLabel}


we have decided to keep aria props as non-camelCase to keep consistency

nataliadelmar · 2024-12-13T11:56:07Z

src/components/ReactionPicker/ReactionPicker.tsx

@@ -7,11 +7,10 @@ import './ReactionPicker.style.scss';
 import AriaToolbar from '../AriaToolbar';

 const ReactionPicker: FC<Props> = (props: Props) => {


these props type already require aria-label(ledby)

nataliadelmar · 2024-12-13T11:57:16Z

src/components/Tooltip/Tooltip.stories.tsx

@@ -63,9 +79,7 @@ Common.parameters = {
      children: <p>Label tooltip, without label overwriteTERTIARY color, variant medium</p>,
      type: 'none',
      triggerComponent: (
-        <ButtonSimple style={{ margin: '10rem auto', display: 'flex' }}>
-          Hover me for label!


this text was wrong. This is a Tooltip of type: 'none', so it does not label the button. That is why my change

nataliadelmar · 2024-12-13T11:58:31Z

src/components/Tree/Tree.types.ts

@@ -198,6 +201,8 @@ export interface Props
  onToggleNode?: (id: TreeNodeId, isOpen: boolean) => void;
 }

+export type Props = TreeProps & AriaLabelRequired;


SPARK-582999

nataliadelmar · 2024-12-13T12:00:01Z

src/components/Reaction/Reaction.tsx

@@ -24,6 +24,9 @@ const Reaction: FC<Props> = (props: Props) => {
        autoplay: autoPlay,
        animationData: animationData,
        name,
+        rendererSettings: {
+          title: props['aria-label'], // adds aria-label to the svg


source: https://github.com/airbnb/lottie-web/?tab=readme-ov-file#other-loading-options

ndelmar added 2 commits December 12, 2024 19:51

feat(accessible-names): require accessible names in non-text wrapping…

5e0f951

… components

feat(accessible-names): fix reactions accessible name

85003dd

nataliadelmar commented Dec 13, 2024

View reviewed changes

nataliadelmar added the validated If the pull request is validated automation. label Dec 13, 2024

nataliadelmar commented Dec 13, 2024

View reviewed changes

ndelmar added 2 commits December 13, 2024 12:27

feat(accessible-names): fix snapshots

36b5602

feat(accessible-names): require accessible name to navigation tab

4b6198f

nataliadelmar marked this pull request as ready for review December 13, 2024 12:56

ndelmar added 4 commits December 13, 2024 14:54

feat(accessible-names): fix linting

0a5d313

feat(accessible-names): change aria props from camelCase

e3d0d51

feat(accessible-names-required): add ToastNotifications

425a722

feat(accessible-names-required): add ToastNotifications

1a34117

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(accessible-names): require accessible names in non-text wrapping components #732

feat(accessible-names): require accessible names in non-text wrapping components #732

nataliadelmar commented Dec 12, 2024 •

edited

Loading

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024 •

edited

Loading

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024 •

edited

Loading

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024 •

edited

Loading

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

nataliadelmar Dec 13, 2024

		@@ -7,11 +7,10 @@ import './ReactionPicker.style.scss';
		import AriaToolbar from '../AriaToolbar';

		const ReactionPicker: FC<Props> = (props: Props) => {

feat(accessible-names): require accessible names in non-text wrapping components #732

Are you sure you want to change the base?

feat(accessible-names): require accessible names in non-text wrapping components #732

Conversation

nataliadelmar commented Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

nataliadelmar Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nataliadelmar Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nataliadelmar Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nataliadelmar commented Dec 12, 2024 •

edited

Loading

nataliadelmar Dec 13, 2024 •

edited

Loading

nataliadelmar Dec 13, 2024 •

edited

Loading

nataliadelmar Dec 13, 2024 •

edited

Loading