Add `footnoteLabelTagName`, `footnoteLabelProperties`

syntax-tree / mdast-util-to-hast

utility to transform mdast to hast

https://unifiedjs.com

MIT License

101 stars 42 forks source link

Add `footnoteLabelTagName`, `footnoteLabelProperties` #64

Closed gtnbssn closed 2 years ago

gtnbssn commented 2 years ago

Initial checklist

[x] I read the support docs
[x] I read the contributing guide
[x] I agree to follow the code of conduct
[x] I searched issues and couldn’t find anything (or linked relevant results below)
[x] If applicable, I’ve added docs and tests

Description of changes

This attempts to add options to let users configure the html tag that will be used for the footnote section label, as well as the properties that will go on it, as discussed in #63

If this is deemed good enough, i will add changes to the readme to this PR in order for the new options to be documented.

github-actions[bot] commented 2 years ago

Hi! It seems some of the things asked in the template are missing? Please edit your post to fill out everything.

[ ] Initial checklist (todo)
[x] Description of changes

You won’t get any more notifications from me, but I’ll keep on updating this comment, and remove it when done!

If you need it, here’s the original template

```markdown ### Initial checklist * [ ] I read the support docs * [ ] I read the contributing guide * [ ] I agree to follow the code of conduct * [ ] I searched issues and couldn’t find anything (or linked relevant results below) * [ ] If applicable, I’ve added docs and tests ### Description of changes TODO ```

Thanks, — bb

gtnbssn commented 2 years ago

Ok seems i was a bit over enthusiastic about my abilities yet again. I will look deeper into the issue here. Meanwhile any advice welcome!

wooorm commented 2 years ago

Friendly ping!

github-actions[bot] commented 2 years ago

Hi! This was closed. Team: If this was merged, please describe when this is likely to be released. Otherwise, please add one of the no/* labels.

wooorm commented 2 years ago

Released in 12.2.0!

gtnbssn commented 2 years ago

Thanks a lot for your help with this!!

wooorm commented 2 years ago

@gtnbssn I just noticed that the rest of the generated code depends on a footnoteLabelProperties of at least id: 'footnote-label', due to accessibility concerns: it is linked to with an aria-describedby:

https://github.com/syntax-tree/mdast-util-to-hast/blob/b7623785f270b5225898d15327770327409878f8/lib/handlers/footnote-reference.js#L43

I think we should force that to exist? Or do you have an alternative idea? As an alternative, we could also force any id to exist (throwing otherwise), and reuse that there?

gtnbssn commented 2 years ago

Indeed!

I think there is a case for a custom class because users might want to be able to use the sr-only class elsewhere on their site, but keep the footnote title visible.

But I am less sure about the id. I cannot really think of a good reason to change it, so I'd say it would be ok to force it to exist with this value. Maybe there is a good use case, but i just cannot think of one for now.

wooorm commented 2 years ago

fixed in 12.2.1 :)