syntax-tree / mdast-util-to-hast

utility to transform mdast to hast
https://unifiedjs.com
MIT License
102 stars 42 forks source link

Use footnoteLabel as an aria-label on the section element rather than in an h2 #60

Closed gtnbssn closed 2 years ago

gtnbssn commented 2 years ago

Initial checklist

Problem

When using footnotes, the following html will be output:

<section data-footnotes="" class="footnotes"><h2 id="footnote-label" class="sr-only">Footnotes</h2>
  <ol>
    <li id="user-content-fn-1">
      <p>a footnote <a href="#user-content-fnref-1" data-footnote-backref="" class="data-footnote-backref" aria-label="Back to content">↩</a></p>
    </li>
  </ol>
</section>

I think this is generated by this part:

https://github.com/syntax-tree/mdast-util-to-hast/blob/dfd724a5e62fc270e71bc2d5a2e4471be0c5ef5b/lib/footer.js#L104-L125

I understand this is to match the way GitHub handles footnotes.

However, this might not be the best way to make things accessible, and an aria-label on the section element might be better than an h2 with a custom class that will require dedicated css, see here:

https://stackoverflow.com/questions/26032089/in-html-how-can-i-have-text-that-is-only-accessible-for-screen-readers-i-e-fo

I haven't investigated how, but a number of projects that ultimately depend on this package display their footnotes without this h2. See this Gatsby example: https://using-remark.gatsbyjs.org/hello-world-kitchen-sink/#footnotes (Although this is missing an aria-label and i guess less accessible. I mention this to show that end users do want to remove this h2, which is hidden with css on GitHub.)

Apologies if i am missing something! But so far i can see that the text that will go in the h2 can be customized, but we do not have more flexibility. The a for the backref is using an aria-label so maybe there is something i do not understand properly here.

Solution

This is a package with close to 4 million weekly downloads so i suppose changes to behaviour and configuration need to be considered carefully.

Maybe a new boolean option could be added that would trigger adding the footnoteLabel as an aria-label on the section, and not adding the h2.

The default behaviour (i.e. when that new option is not set) would remain the same.

Alternatives

Although i haven't investigated how, projects relying on this one do have solutions to get rid of this h2.

The alternative is to let users add the css rules from GitHub to manage the sr-only class:

.sr-only {
    position: absolute;
    width: 1px;
    height: 1px;
    padding: 0;
    overflow: hidden;
    clip: rect(0, 0, 0, 0);
    word-wrap: normal;
    border: 0;
}

It feels a bit clunky to me though and aria-label seems like a more standard option that will output less clutter.

Ideally, it would even be possible to let users customize the html inserted, but it might be best to keep things minimal here? I suppose what is happening with Gatsby and others is that they wrote their own plugins to manage this.

wooorm commented 2 years ago

Hi!

an aria-label on the section element might be better than an h2 with a custom class [...]

You mean labels on each footnote call right? I am not sure it is for this use case: namely a footnote section. Perhaps you’re right, what you’re suggesting is how many footnotes in markdown work, but GitHub intentionally did it different here. My assumption is that they probably tested it. Rather than that they made a mistake. react-a11y-footnotes/eleventy-plugin-footnotes also uses a section.

a number of projects that ultimately depend on this package display their footnotes without this h2.

Because they use out of date dependencies. From before GitHub added support for footnotes.

I mention this to show that end users do want to remove this h2, which is hidden with css on GitHub.)

Given that their result is due to out of date dependencies, I do not think this proves your reading of what people want.

but we do not have more flexibility

What flexibility do you want and why?

so i suppose changes to behaviour and configuration need to be considered carefully.

This behavior changed in a major: https://github.com/syntax-tree/mdast-util-to-hast/releases/tag/12.0.0, I am not interested in changing the default behavior again. I am open to ideas on options, if the results make sense.

Note that we use ASTs in remark/rehype. You can already change anything you want. You can remove headings. You can add attributes.

The alternative is to let users add the css rules from GitHub to manage the sr-only class:

Well, the heading could also remain as it was!

gtnbssn commented 2 years ago

Thanks for your answer!

TL;DR: I think aria-label is a better solution here, but i agree the current default behavior shouldn't change.

an aria-label on the section element might be better than an h2 with a custom class [...]

You mean labels on each footnote call right?

I agree we should have a section, and only one for all the footnotes. The aria-label would go on that section (and replace the h2). The output html i have in mind would be like:

<section data-footnotes="" class="footnotes" aria-label="foonotes">
  <ol>
    <li id="user-content-fn-1">
      <p>a footnote <a href="#user-content-fnref-1" data-footnote-backref="" class="data-footnote-backref" aria-label="Back to content">↩</a></p>
    </li>
  </ol>
</section>

I'm not an accessibility expert, but it looks to me like we do not need aria-labels on each footnote. We have the one for "back-to-content" and i guess the rest would be clear for a screen reader.

a number of projects that ultimately depend on this package display their footnotes without this h2.

Because they use out of date dependencies. From before GitHub added support for footnotes.

I mention this to show that end users do want to remove this h2, which is hidden with css on GitHub.)

Given that their result is due to out of date dependencies, I do not think this proves your reading of what people want.

Well, GitHub is not showing this h2, it is hidden with a bit of css. So i think even GitHub does not want to show this on a regular screen, only have it read by screen readers. (The css is the same trick mentioned in the stack overflow answer about putting text that will be screen-reader specific, but in second place after using aria-label.)

What flexibility do you want and why?

I would like to be able to remove the h2 easily, and have an aria-label on the section instead.

This is because:

so i suppose changes to behaviour and configuration need to be considered carefully.

This behavior changed in a major: https://github.com/syntax-tree/mdast-util-to-hast/releases/tag/12.0.0, I am not interested in changing the default behavior again. I am open to ideas on options, if the results make sense.

I don't think the default behavior should change. In fact it would probably be a bad idea. But i'll be very happy to have another option arialabelInSection, or whatever, to trigger this behavior.

Note that we use ASTs in remark/rehype. You can already change anything you want. You can remove headings. You can add attributes.

I admit i do not know about this! Would this require writing code for a plugin that would handle the AST?

The alternative is to let users add the css rules from GitHub to manage the sr-only class:

Well, the heading could also remain as it was!

It could! But i wouldn't get the flexibility i am looking for.

Another related note is that due to the way the footnoteLabel value is being tested, i cannot just set this option to an empty string, it is also falsey and will display the default English "Footnotes". That would have been a lazy option for achieving what i am looking for, but i think we can do better and remain accessible. This might be intentional in order to insure that we always have a footnoteLabel, even if it is in the wrong language.

wooorm commented 2 years ago
<section data-footnotes="" class="footnotes" aria-label="foonotes">

[...]

I'm not an accessibility expert, but it looks to me like we do not need aria-labels on each footnote. We have the one for "back-to-content" and i guess the rest would be clear for a screen reader.

I wouldn’t call myself an expert either but that’s not how aria-label works. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-label.

Finally, the first rule of ARIA is to not use ARIA: https://www.w3.org/TR/using-aria/#rule1. The heading strategy is normal HTML without ARIA. It works. If you want don’t want to show it, you can hide it with CSS.

When in doubt, I’d prefer following experts, and GitHub + Kitty doing what’s done here, feels pretty solid!


Well, GitHub is not showing this h2, it is hidden with a bit of css.

https://github.com/syntax-tree/mdast-util-to-hast#css. CSS is needed for footnotes anyway, for other things than hiding the heading (which is optional).


the use of an h2 might not fit the headings hierarchy on my page

I can somewhat understand this because its the “right thing” to do. We could make that configurable. On the other hand: the HTML heading structure doesn’t matter much, because it’s all over the place on every website anyway, so screenreaders or other tools don’t depend on it.

i am (very clumsily) trying to add some flexibility in Astro's management of footnotes:

It would have been appreciated if you’d have shared this related issue and root problem earlier. As you mention there, the heading can be translated here, but Astro doesn’t have that feature yet.

I admit i do not know about this! Would this require writing code for a plugin that would handle the AST?

Yes. Anything can be done by transforming the AST.

gtnbssn commented 2 years ago

I admit my only source here is the stack overflow post, where the example given feels pretty similar to what i had in mind. I do find it surprising that this case (add text only for the screen reader) isn't managed by the standard and requires some css. But i just saw that tailwind includes the sr-only class in its utilities, so as you are pointing out, a good number of people with more knowledge than me seem to agree on this.

Feel free to close this issue and sorry for the noise.

I'll get back to trying to fix the translation issue in Astro.

wooorm commented 2 years ago

where the example given feels pretty similar to what i had in mind.

Yeah, it is similar, but not the same: aria-label *replaces* content, so your aria-label on the section is a replacement for everything.

I do find it surprising that this case (add text only for the screen reader) [...] requires some css

I think it’s in line with how the web works, namely the other way around. Similar to how the “mobile-first” design idea (start with “mobile” screens and add CSS for bigger screens) suits the web, here we start with HTML only making sense on its own, and then we add CSS on top to change things for visual users.

[...] sorry for the noise.

No problem at all!

Feel free to close this issue [...]

Okay, I will close the issue, but I am open to adding concrete features for different output. E.g., a tag name other than h2 probably makes sense, and probably different attributes too. But I’m not so sure about a toggle that produces different output, which isn’t (proven to be) accessible!

I'll get back to trying to fix the translation issue in Astro.

Feel free to ping me if the discussion there is useful for me to weigh in on!

Good luck and all the best! :)