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

Accessibility improvements #764

Closed
wants to merge 1 commit into from
Closed

Accessibility improvements #764

wants to merge 1 commit into from

Conversation

amerikan
Copy link

@amerikan amerikan commented May 25, 2023

Changes

  • fixes some a11y issues

NOTE: this was individual volunteer work within my own personal capacity. I did it just for fun ¯\_(ツ)_/¯ Someone feel free to take over and make any code adjustments etc.

Purpose

For better accessibility.

Approach

How does this change address the problem?

Learning

Learned how to use i18next library :)

Pre-Testing TODOs

What needs to be done before testing

Testing

How should other contributors test this Pull Request?

  1. Pull in the changes to your local copy of this branch and restart Docker.

Screenshots

Closes #592, Closes #593, Closes #671

@amerikan amerikan changed the title adds several missing ES translations adds several missing ES translations and other fixes May 25, 2023
@amerikan amerikan changed the title adds several missing ES translations and other fixes adds several missing ES translations and applies a11y fixes May 25, 2023
@amerikan amerikan changed the title adds several missing ES translations and applies a11y fixes Adds several missing ES translations and applies a11y fixes May 25, 2023
@coreyshuman
Copy link
Member

Thanks for the PR Eric! We really want to build up community support for these internal tools so I appreciate you taking the initiative like this!

I actually just wrapped up a big i18n coverage PR which we merged into the develop branch. If you want to compare, check it out here: #762

I missed some of the aria features so I'll definitely refer to this work to improve those things.

@coreyshuman
Copy link
Member

I'm going to set this to a draft PR for now, since we want this type of feature work to go into the develop branch, and reserve main as a stable releases branch. That will let us reference this as we fix some of the a11y (which conveniently coincides with the QA pass we're doing right now!)

@coreyshuman coreyshuman marked this pull request as draft May 25, 2023 22:12
@amerikan amerikan changed the base branch from main to develop May 25, 2023 22:41
@amerikan
Copy link
Author

amerikan commented May 25, 2023

@coreyshuman wow that's a large, but great change you've got there!

I'm not too familiar with this project, but wanted to check it out. It was a blunder on my part to use main thinking it was the development branch. I'm just so use to having the "development" branch as the default branch on github repos. It didn't even occur I was working off main (lol) and that there was an actual development branch called develop. As a suggestion, maybe change the default branch to be develop instead of main on this github repo?

If I have time, I can try to rebase some of the conflicts. I've used atomic commits, so in theory should be easy to resolve/revert individual changes. But makes sense to move it to draft. 👍🏼

@coreyshuman
Copy link
Member

@amerikan if you can find the time, we'd love the support! If we get around to fixing this first, I'll make sure to close this PR and let you know.

@amerikan amerikan changed the title Adds several missing ES translations and applies a11y fixes Accessibility improvements May 25, 2023
@amerikan
Copy link
Author

@coreyshuman should be good now. Basically removed all the translation stuff and kept the a11y changes. 👍🏼

@amerikan amerikan closed this Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants