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

Improve website accessibility #994

Merged

Conversation

lbeckman314
Copy link
Contributor

Ticks two of the boxes from #912

  • Include labels for inputs (helps with screen reader accessibility even if visibly hidden)

  • Improve keyboard navigation (I suggest that tabbing moves between project titles and skips project tags & "up-for-grabs" label)

I'm still very new to pull requests, so any edits or recommendations on the commits are more than welcome. : )

@lbeckman314 lbeckman314 mentioned this pull request Sep 11, 2018
5 tasks
<h5>Filter by name: </h5>
<select class="names-filter" multiple data-placeholder="Select a project..." >
<label for="project">Filter by name:</label>
<select class="names-filter" multiple data-placeholder="Select a project..." id="project">
<% _.each(names, function(entry, key){ %>

Choose a reason for hiding this comment

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

Almost identical to what I had in the works, only difference being that my ids were a smidgen more descriptive (filter-by-name-input etc.)

Copy link
Contributor Author

@lbeckman314 lbeckman314 Oct 13, 2018

Choose a reason for hiding this comment

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

You probably can't go wrong being more descriptive. In that case I would elect to use your improvements over mine!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@alexandraholcombe I think you haven't sent a PR, please send a PR.

Choose a reason for hiding this comment

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

@ritwik12 @lbeckman314 This PR includes a few changes mine doesn't include. I'll put that in this evening, but I recommend merging this first.

@shiftkey
Copy link
Member

@lbeckman314 it looks like there's some conflicts on this PR, and I'm not sure of the resolution. Are you able to take a look?

@lbeckman314
Copy link
Contributor Author

Absolutely. I will take a look.

- Include labels for inputs (helps with screen reader accessibility even if visibly hidden)
- Improve keyboard navigation (I suggest that tabbing moves between project titles and skips project tags & "up-for-grabs" label)
- Include labels for inputs (helps with screen reader accessibility even if visibly hidden)
- Improve keyboard navigation (I suggest that tabbing moves between project titles and skips project tags & "up-for-grabs" label)
@lbeckman314
Copy link
Contributor Author

Fetched changes from upstream. Resolved and ready for review.

Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

LGTM - thanks @lbeckman314!

@shiftkey shiftkey merged commit 610b0c8 into up-for-grabs:gh-pages Feb 12, 2019
@lbeckman314 lbeckman314 deleted the improve-website-accessibility branch February 13, 2019 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants