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

Some accessibility fixes for welcome/home page #3179

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

peetucket
Copy link
Member

@peetucket peetucket commented Jun 30, 2023

Why was this change made? 🤔

Addresses some of the accessibility issues identified in #3171

How was this change tested? 🤨

Localhost and CI

Does your change introduce accessibility violations? 🩺

I hope it reduces them!

@peetucket peetucket changed the title Some accessibility fixes for welcome/home page [WIP] Some accessibility fixes for welcome/home page Jun 30, 2023
@peetucket peetucket changed the title [WIP] Some accessibility fixes for welcome/home page Some accessibility fixes for welcome/home page Jun 30, 2023
Copy link
Member

@mjgiarlo mjgiarlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we sit on this until we can convert I hope it reduces them! to We ran a set of tools to verify these issues, and now they're gone, and no new ones are introduced

Copy link
Contributor

@ndushay ndushay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you split this PR into the two commits, I'll merge a PR with the first commit (with one tweak I noted in a comment).

app/views/layouts/application.html.erb Outdated Show resolved Hide resolved
@peetucket
Copy link
Member Author

  1. split PR into two, second is here: ensure all popovers have aria-describedBy #3186
  2. removed <a name="content"> tag based on Mike's comments

Copy link
Contributor

@ndushay ndushay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving this. Agree that it would be good to have a tool to show us this fixed something and didn't break anything else.

@mjgiarlo mjgiarlo merged commit e6b4a0f into main Jul 5, 2023
@mjgiarlo mjgiarlo deleted the 3171-welcome-accessibility branch July 5, 2023 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants