Ai2svelte component accessibility fixes

[MinamiFunakoshiTR]

For discussion about improving the Ai2svelte component to make it work better for screen readers. Please leave comments if you have suggestions or thoughts. Thanks!

Problem

The html text in ai2svelte is usually unintelligible and unhelpful for screen readers.

Goal

Hide the text in ai2svelte from screen readers by default using aria-hidden='true' and make it easy for us to add AltText that better describes the graphic. Make it so we can turn off this feature in case we do want the text in ai2svelte to be read aloud.

Fixes the a11y group is proposing

In the Ai2svelte component:

• Wrap ai2svelte component in <div class="ai-wrapper" aria-hidden="{ariaHidden}"></div>, which hides it from screen readers by default
• Add ariaHidden variable which defaults to true but can be set to "false" in the google doc
• Add <slot name="srDescription" /> which pulls the AltText for screen readers from google doc

See PR here

In bluprint_graphics-kit:

• Add srDescription slot in Page.svelte when Ai2svelte.svelte is being called. This description is made invisible on the page with CSS that applies to sr-only class
• Bootstrap has a built-in CSS for .sr-only that makes text invisible on the page but is read aloud in screen readers. Hard-code and add this CSS to our rig in case botstrap gets depreciated or we stop using it. (Note: I tried adding the CSS for sr-only in main.scss but that didn't work)
• Add a warning that appears if AltText is empty or missing in the google doc

See PR here

In the master google doc:

• Add a note to editors that explains what AltText is and how it should be edited at the top of the google doc
• Add props for AltText and ariaHidden for the ai2svelte block in the master google doc

See suggestions in red in this dummy google doc

References for best practices

Read about aria-hidden here:

Adding aria-hidden="true" to an element removes that element and all of its children from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative content, such as icons or images, etc...

Semantic html is recommended as part of best practices.

[hobbes7878]

Thanks for this @MinamiFunakoshiTR! Have a couple thoughts on the code details, but my first question is do we have any resources that talk about best practice for handling aria-only (i.e., the inverse of aria-hidden) content on the page?

Basically looking for a source on where this CSS came and whether we're sure that's best practice.

[hobbes7878]

Quick google got me a couple different versions. Bootstrap 5 introduced a .visually-hidden class, which looks like yours, but with important tags, which in this case I think is a good use. (Also noticed this blog mentions that clip is technically deprecated so maybe we add clip-path as well to future-proof a bit.)

But for a global rule like this, I think the better thing would be to add it to our main SCSS library so it can apply across our projects. I can take that on as part of this PR, and I kinda like the idea of having some accessibility-related SCSS mixins we can document as well as the overriding class, like what Bootstrap 5 has done.

[hobbes7878]

OK, couple other things I'll suggest at a high-level, and then I think we can deal with small naming conventions and things in the PR:

Generally, I don't think this PR should add any SCSS to the kit's global or main.scss styles. I do like the idea I mentioned of adding bs5's .visually-hidden class to our main style lib, but we can also duplicate that rule locally-scoped within the Ai2svelte component if that gets this change out more quickly.

I also think we should use console.warn for missing aria attributes and accessibility issues generally. That lines up with what Svelte is already doing to warn us about a11y issues it finds, and I think the training for the team should be to clear those out. There are also some valid cases for ai2svelte that don't require aria description, I think, like when we use AI to make a purely decorative part of a page, that as currently written would look like errors.

One way to trigger that console warning within the Ai2Svelte component would be to check for agreement between ariaHidden prop and the aria description passed in. If it's set hidden but no description or a blank is given, we can then warn.

A good example is our BeforeAfter component. We warn in the console if altText is missing and then refuse to display the component if not provided (which helpfully sends folks looking in the console for what's wrong...). I think we can follow that model, generally: So, can omit markup if missing critical accessibility data, but don't add markup to the page and instead use the console to warn.

As I'm looking at it now, I'm not sure srDescription/ariaDescripton should be a slot. We're not hiding the title and chatter, which are external to AI anyway, so I don't really see the use case for making additional heading tags in markdown. I think we just want to pass in a string, no? I would be interested to know if there's a compelling reason to split that description into multiple paragraph tags, or if just one would do us, in which case, I think you just make that a string prop and wrap it in p's. But would love to see an example of screenreader-only description for a graphic with more complex structure to wrap my head around.

I'm not sure aria-hidden should necessarily be default true. I think hiding text from screen readers is something we always want to do intentionally. I'd set that false.

.... I think that's all the main stuffs I'd like to see changed or thought through in the PRs before we merge 'em. Looking good, tho. Keen to see how we handle the scrolly ones next ☺️.

[juruwolfe]

Just weighing in on that last point (default to true vs fault for hidden) because it's something we discussed.

As it stands, SRs will read every label in an ai2html chart which comes out like gibberish. Happy to make a recording for you if you're curious, but it just sounds like random unrelated words strung together as a sentence. @pkd2512 is thinking through cases where we might actually want the text to be read, but I think those will be exceptionally rare. I worry that leaving it as something we have to manually turn off will mean a lot get missed and I suspect in 90%+ cases we're not going to want an ai2html chart read aloud as is.

[hobbes7878]

Yeah, OK. I can see that balance. Would just want to make that very clear in the component docs for Ai2svelte that we've coded in a difference between what you're seeing and what a screen reader will read by default. So maybe @MinamiFunakoshiTR can add a shouty line in the docs page for that, then! 👍

[AditiBhandariTR]

So far I can only think of one instance where we may want multiple elements in the aria-description; if we have a simple chart or a locator map with several data points (e.g. https://tmsnrt.rs/3xVKzOQ) that has both a short text description and a data table (like 10-20 rows). Some readers may want to be able to go through the data, and it'd be easier for a map like this to have a short data table rather than writing out the death toll per place in the google doc.

[hobbes7878]

Thanks, @AditiBhandariTR! That's incredibly helpful context.

What I think I'd suggest then @MinamiFunakoshiTR is we make an ariaDescription prop on Ai2svelte that expects a string (which we'll feed from the Google doc). No markdown formatting.

Then why don't we just add a default slot (i.e., not named) that pops whatever markup it gets just above or below the graphic. In the docs, then, we can document an example of passing a simple data table to that slot while using the ariaHidden and ariaDescription props.

What I'm not sure of is whether a table with that .visually-hidden class or maybe a descendent of a div with that class will be properly hidden or not. May need to add some special rules specifically for tables? Dunno, but we can test that when writing the docs.

[MinamiFunakoshiTR]

Re: the srDescription getting markdown -- I proposed this because I know sometimes people in Asia/EMEA put the title and description inside the ai file instead of putting it on the google doc. I think it's partly to style the deck as they like, and partly out of habit.

If the chart title and description are all inside the .ai file, and we're rewriting it in srDescription, it would be nice to add some hierarchy to it. But I also get the argument that people should be writing chart title/description in the google doc anyway, not in the ai.

If we're going to get people to write the chart title/description in the google doc, I think it's fine to just pass a string. If we are going to have cases where we rehash the title/description in srDescription, though, I think adding markdown would be nice.

[MinamiFunakoshiTR]

Hello! Jon and I had a chat. These are the changes I'll be making:

• Add .visually-hidden class and CSS to the Ai2svelte component. Eventually, @hobbes7878 will add this CSS to our main SCSS library
• Add a slot for ariaDescription in the Ai2svelte component that takes a string
• To accommodate more complex cases where we might want to add a data table, lengthier descriptions with markdowns, etc, we made a slot name=hidden``. You can add anything in this slot and it will be read aloud by screen readers but be invisible on the page.
• I'll work on docs to explain all this

What's done

• PR for the components library #29

Next steps:

• Update and submit a new PR for the graphics kit repo
• Update the dummy google doc
• Merge the PR for the graphics kit and for the components library once everything is ready

[MinamiFunakoshiTR]

What's new

• Made edits to Page.svelte in the bluprint graphics kit. See PR here
• Made edits to the dummy google doc

Next steps

• Merge all PRs
• Update the master google doc

@hobbes7878 let me know if you want me to update the master google doc once you've had a chance to look at all the final edits. Thanks!

[hobbes7878]

This is now published as 0.2.0. PR was merged in the kit in reuters-graphics/bluprint_graphics-kit#51 and bumped components to the new version in its deps.

Leave it to @MinamiFunakoshiTR to update the master Google doc for kit.

Thanks, y'all for doing this. I'll close this issue out.