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

Update anchor tags and enhance navigation cues for screen readers #8334

Merged
merged 8 commits into from Aug 19, 2019

Conversation

@WilliamHolden
Copy link
Contributor

@WilliamHolden WilliamHolden commented Jul 14, 2019

This pull request does two things. The first is that it changes any non link anchor tags to use href="javascript:void(0)" instead of href="#".

Both of these href attributes are trying to achieve the same thing so we should only use one for consistency reasons. There are a few reasons to prefer href="javascript:void(0)" over href="#".

  1. href="#" changes the actual URL leading to unwanted effects like the page scrolling to the top
  2. href="#" causes backbone to call an internal checkURL() function which leads to unwanted effects like the issue below:
    #7893
  3. href="#" doesn't work well with screen readers (this can be mitigated by adding role="button")

There are other reasons as well. See the top answer on https://stackoverflow.com/questions/134845/which-href-value-should-i-use-for-javascript-links-or-javascriptvoid0

Ideally, a lint rule will be added to enforce this change in the future but I could not find a good eslint plugin. The closest one I found was "jsx-a11y/href-no-hash" but I think this rule was created for jsx specifically

The next thing this pull request does is make some enhancements to the navigational cues for screen readers through the use of aria-labels, tabindex, and role attributes. There is still some work to be done here and I will make another pull request with more navigational cue updates.

@galaxybot galaxybot added this to the 19.09 milestone Jul 14, 2019
@WilliamHolden
Copy link
Contributor Author

@WilliamHolden WilliamHolden commented Aug 16, 2019

I have another WIP branch which depends on some of the changes in this PR. When this PR passes review and gets merged I will make the PR for the WIP branch into galaxy dev (when it's not WIP anymore of course). For now I have set up a temporary PR in my own repo to keep track of the changes: WilliamHolden#1.

Loading

@martenson
Copy link
Member

@martenson martenson commented Aug 19, 2019

Thanks @WilliamHolden !

Loading

@martenson martenson merged commit 7c96ed2 into galaxyproject:dev Aug 19, 2019
6 of 8 checks passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants