Add prop to allow overrides of `aria-live` and friends

[Josh68]

https://github.com/CMSgov/design-system/blob/216acd1eb981f6ab21c52317c5043778368b6be3/packages/design-system/src/components/ChoiceList/Choice.jsx#L117

In situations where this component is used with other components, the hard-coded placement of these aria-live and related aria- attributes can result in unwanted, excessive over-reading of content. Please consider adding a prop to allow an override, so that the consumer is then responsible for adding appropriate aria- attributes at another level in a component or element elsewhere in the tree. This could be flagged in the prop's documentation.

See https://github.cms.gov/CMS-MCT/coverage-tools-frontend/pull/1802, https://github.cms.gov/CMS-MCT/coverage-tools-frontend/pull/1756

[line47]

https://jira.cms.gov/browse/WNMGDS-925

[Josh68]

Thanks, @line47!

[forrestbaer]

Hey @Josh68 , would this be for any component that uses aria-live props or just the one listed, ChoiceList?

[Josh68]

Hey, @forrestbaer . Good question. I could see the utility of having an optional override of baked-in aria-live and related attributes on any component, because the context in which the component will be used can't be known (e.g., using a ChoiceList inside a HelpDrawer). The aria-live spec doesn't provide a declarative way to override or turn off announcement behavior down the DOM tree, and the attributes can't be added dynamically (won't be respected).

I think you should start with this update, and we should probably have a discussion with the larger DS team about the idea of always providing overrides. Unless you think that's something your team can come to consensus on now, and want to expand the scope.

[Josh68]

One more thing (going back to look at source). So, in this case, it's the Choice component that includes aria-live, aria-relevant, and aria-atomic, and looks at the value of checkedChildren to control them. In the case of overriding, it's likely a parent component would need to use inputRef to get at this value and include its own aria-live (and related) attributes, with extra conditions. I'd want to test this out and also probably note that in the component's documentation.

[forrestbaer]

Thanks for the context Josh, I've provided a link here from that ticket, will pass this information along and work out a schedule to resolve it. It does seem like functionality we could/should roll out for all components.

[forrestbaer]

Discussed in #484

This looks like a more complex problem than I originally thought. And seems to require modification to the component to determine when to apply these aria tags. We can continue to track this here. Flagging @davidakennedy for visibility.

[Josh68]

@forrestbaer, here are 2 PRs from our repo that cite the issue:

https://github.cms.gov/CMS-MCT/coverage-tools-frontend/pull/1802 https://github.cms.gov/CMS-MCT/coverage-tools-frontend/pull/1756

In the 2nd case, the fundamental issue is that a state change on a component containing a list triggers the screen reader, and not changes to checkedChildren in the Choice component. In the first case, there's a suggestion about localizing aria-live to each of the checkedChildren and not keeping it on a containing div.

It's absolutely true that aria-live needs to be in the DOM on page load to be honored, which is an issue when it comes to trying to remove it, programmatically. That just doesn't work.

In terms of the A11Y concerns, I do think it should be clear to users that if there's an override option, to keep from rendering aria-live and related attributes, that documentation should be very clear about the implications and responsibility of the component user to determine where else to put them. Even console warnings could be warranted. My assumption was that including it (on page load) higher in the tree would suffice, but this would need to be tested out.

[davidakennedy]

Thanks for flagging me, @forrestbaer. My concerns from an accessibility standpoint if we allow an override: Teams understand the implications of overriding the prop and how best to handle that. Maybe we have a few basics demos in the docs.

The other thing I was thinking about is in the long run, it might be good to have a component that handles the aria-live announcements on a page. So that state could bubble up to a component, located further up in the tree. I don't know how much of a use case for this is out there. But I've seen it done in the past, and it might help teams in this kind of situation, or those needing to juggle multiple announcements throughout the experience.

[Josh68]

The other thing I was thinking about is in the long run, it might be good to have a component that handles the aria-live announcements on a page. So that state could bubble up to a component, located further up in the tree. I don't know how much of a use case for this is out there. But I've seen it done in the past, and it might help teams in this kind of situation, or those needing to juggle multiple announcements throughout the experience.

That's a really interesting idea, @davidakennedy. I hadn't thought of it. In my experience, aria-live just isn't as flexible as I think it should be. I actually started writing a blog article long ago about it, and learned a lot in the process, but in the end, felt like I hadn't come up with any solutions to issues I faced trying to optimize UX with lots of live regions. In this case, just nesting components in a highly interactive interface can lead to unintended consequences when live regions are baked in on the DS side, or at least that's my take.

[forrestbaer]

Interesting, it's a good idea but not really possible in the short term, though it would be good to have it on the roadmap.

We now have a ticket in our next sprint to come to a conclusion on a solve for this. @Josh68 I'm actually unable to access those issues so I can't see directly what was proposed there. We have another issue (https://jira.cms.gov/browse/MCT-5112) which is currently blocked by this same issue.

@davidakennedy, Josh, would moving the aria-live and aria-relevant properties representing items which are checked to the wrapping legend in a ChoiceList component resolve this issue enough to get us over the first hurdle?

[Josh68]

Thanks, @forrestbaer. I'll have to loop back to look at the original issue we had on MCT with these nested aria-live attributes and let you know. I still think the option to override them altogether, with adequate warning about the A11Y impact if components further up the tree don't handle live-update announcements, would be the most flexible solution. But I'll update here when I've reviewed the issue on our end.

[Josh68]

@forrestbaer, it's been so long, I had to dig in to figure out what looks like it would help. Ultimately, my best assessment is that an actual solution, in this case, would to have been able to move aria-live and friends down the tree, not up. I don't know if this can be handled with the ability to override and adequate refs or additional props, but at least this should give you an idea of the way possibly unexpected use of a component can give unexpected results with aria-live and friends (which, as you've noted, must be set on render and cannot be overridden down the DOM tree).

Since you can't see our repo (or let me know if I'm wrong about that), here's a breakdown:

This is a ChoiceList (radio buttons) with two nested Choices, one of which contains both another ChoiceList (checkboxes, referenced in the checkedChildren attribute) and also has a hint attribute that is a HelpDrawer (toggle and drawer).

So the issue was that having checkedChildren on the top-level Choice component automatically adds aria-live and friends to the wrapping div in that component. With the HelpDrawer inside, we have another live element that triggers announcements. Ultimately, it seems like the aria-live attributes would have worked correctly if placed directly on the checkboxes at the bottom of the tree. As it is, opening the HelpDrawer was activating announcements, because the hint prop passed to the FormLabel is also wrapped inside the Choice's parent div.

https://github.com/CMSgov/design-system/blob/1b4f28750869764ec4ee074ceb4fb743d3982e3c/packages/design-system/src/components/ChoiceList/Choice.tsx#L221

https://github.com/CMSgov/design-system/blob/1b4f28750869764ec4ee074ceb4fb743d3982e3c/packages/design-system/src/components/ChoiceList/Choice.tsx#L241

[Josh68]

In our source code:

checkedChildren: (
  <ChoiceList
    name={basicMode ? "MaCategoriesBasic" : "MaCategories"}
    type="checkbox"
    label={t("search_results_page.filters.types_of_health_plans")}
    hint={helpDrawer}
    className="ds-u-margin--0 ds-u-border-bottom--1"
    onChange={event => {
      const { checked, value } = event.currentTarget;
      const selected =
        checked && !currentSelections.includes(value)
          ? [...currentSelections, value]
          : currentSelections.filter(f => f !== value);
      setCurrentSelections(selected);
      if (basicMode) {
        onFilterUpdate(selected, planType !== selectedPlanType);
      }
    }}
    choices={maChoices}
  />
),

If we'd had the ability to 1) override default aria-live and 2) add aria-live (etc) to the maChoices, I think it would have solved the problem.

[Josh68]

Would be very nice if there was something like aria-live="exception" or if aria-live="off" could be nested inside aria-live="polite" and act as an override for one part of the tree, but no such luck.

[forrestbaer]

This is great Josh thank you for the thorough rundown it will really help get this solved. Also, correct I don't seem to have access to that particular repo so thanks again for posting some code examples.

I will bring this back to our current blocked issue and see if we can come up with a solution that can satisfy both needs, and a timeline as well..

[Josh68]

Sure thing. Glad this is helpful.

[pwolfert]

Moved this to the top of our backlog. We have a very long backlog, and even though a decision was made on March 18th about what needs to be done, it was not prioritized and got lost in our backlog. https://jira.cms.gov/browse/WNMGDS-925

[forrestbaer]

I believe this issue should finally be resolved with #2191 @Josh68, thanks for sticking it out. You can find this update in the latest 5.0 beta.

[Josh68]

Thank you @forrestbaer!

[forrestbaer]

Closing with updates made in #2191