Hello,
Following this documentation page https://fontawesome.com/how-to-use/accessibility, I will make a patch these recommendations taken into account if possible.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | fontawesome_menu_icons-a11y-2962737-3.patch | 2.33 KB | grimreaper |
Comments
Comment #2
grimreaperHere is the patch.
Thanks for the review.
Comment #3
grimreaperAdd a check in case of external link.
Comment #4
lonaloreHi! Thank you for your patch file, it looks good. Is that 'sr-only' span element really necessary (when the link text is hidden)? If it is, then we should include a CSS to visually hide the span element, because the 'sr-only' class is not hidden in non-bootstrap based themes.
Comment #5
grimreaperHello,
The "sr-only" class is used by the CSS provided by Fontawesome so no need to have Bootstrap-related CSS.
Is the span really necessary? I followed the documentation as much as I could.
Comment #6
grimreaperComment #8
lonaloreExcellent! Thank you!
Comment #10
andrewmacpherson commentedCore accessibility maintainer here. "accessibility" is the preferred tag. We're trying to avoid "a11y" because (a) lots of people don't understand the numeronym, and (b) we don't want two separate lists of issues to review. Hope you don't mind me removing this tag from a contrib project.
Comment #11
andrewmacpherson commentedcleaning up issues with this typo in the tag name, this time it was my fault