Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use semantic HTML tags and aria attributes #415

Closed
wants to merge 19 commits into from

Conversation

zax-xyz
Copy link
Contributor

@zax-xyz zax-xyz commented Nov 24, 2021

This introduces the use of semantic HTML tags throughout the frontend, as well as the use of various ARIA attributes to improve accessibility (see https://developer.mozilla.org/en-US/docs/Learn/Accessibility/HTML).

Semantic section-related HTML tags are used (main, nav, article, etc), as well as more component based-tags (button).

Generic div tags were previously used in many parts throughout the codebase, hindering the use of accessibility tools such as screen readers1, and preventing keyboard navigation (tab focus) on clickable elements as they were not semantically focusable elements (buttons and links).

ARIA attributes are used in elements for which their semantics/purpose cannot be easily represented specifically enough simply through choice of HTML tags.

In a couple of the settings pages, some of the links were also defined using onClick attributes with a switchPage function. While some of these were anchor tags, they were still not focusable due to not having href attributes, so the browser sees it as having no location. Using react-router-dom's <Link> element resolves this.

In addition to improving accessibility, as a side-effect, these changes also enable an alternative solution to #149, as we can now make relatively specific selectors using these semantic tags and attributes. As a demonstration of this, I have rewritten a large portion of the Discord theme that was previous broken due to non-persistent class names, using this method.

Note that there are still some parts of the frontend that have not yet been changed to use semantic tags, hence the draft status of this PR.

Footnotes

  1. Disclaimer: I have never personally used a screen reader or other assistive technologies and so my understanding of them may not be accurate

This allows focusing these elements via the keyboard or other accessibility tools, which was
previously impossible with generic div tags
This improves accessibility further by providing semantics to elements that can't easily be defined
just through the tags
This ensures tab-navigability through links by adding anchor tags where previously unpresent, and
adding href attributes to those that didn't previously have them.
@insertish
Copy link
Member

Looks awesome so far!
Didn't know you can do stuff like https://github.com/revoltchat/revite/pull/415/files#diff-86a2bd14a6079d4e70278e3ff08515ed0b7666dadaaf584a5a95456cb2f93628R59, this could definitely go a long way towards #149.

This makes the element focusable for accessibility
This provides an accessible alternative to the self-baked data-active
attribute, that can be used by assistive technology
This provides an accessible alternative to the self-baked data-active
attribute, that can be used by assistive technology
Using buttons enables tab/keyboard navigation for accessibility
As both anchor tags (with a href attribute) and buttons are focusable,
trying to focus the buttons with a wrapping <a> tag (Source Code and
Donate) would result in being able to focus both elements, which is both
inconvienent and confusing for users using keyboard navigation, not
knowing which element is correct, and slowing down navigation.
This wraps the user icon in an anchor tag with an aria-label that
describes what it's for
@insertish
Copy link
Member

This may be an uphill battle as components are changed so it may be worth doing this across several PRs.

@zax-xyz zax-xyz marked this pull request as ready for review January 17, 2022 08:40
@zax-xyz
Copy link
Contributor Author

zax-xyz commented Jan 17, 2022

This may be an uphill battle as components are changed so it may be worth doing this across several PRs.

Fair enough, this has been open for far longer than I intended, I'm happy to have this merged right now if it all looks good to you (and the others).

@Rexogamer Rexogamer added tech debt This issue or pull request is related to technical maintenance and removed tech debt This issue or pull request is related to technical maintenance labels May 8, 2022
@insertish
Copy link
Member

insertish commented Jan 24, 2023

Conflicting again 😭

New client is using semantic HTML where possible

Closing due to rewrite, marking as potential issue in future by linking to revoltchat/frontend#64.

@insertish insertish closed this Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Areas that can be improved on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants