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

Search bar takes tab focus #400

Closed
paulgibbs opened this issue Jul 20, 2017 · 9 comments · Fixed by #659
Closed

Search bar takes tab focus #400

paulgibbs opened this issue Jul 20, 2017 · 9 comments · Fixed by #659
Assignees
Labels
bug Issue reports a bug

Comments

@paulgibbs
Copy link

paulgibbs commented Jul 20, 2017

Description

When I press tab, I expect to be able to navigation around the page with keyboard navigation. Instead, the search bar comes into focus when I press tab, regardless of how many times I try pressing tab again.

Is there a way to disable the JS capturing tab (and other keys) apart from not loading the entire JS file? This breaks keyboard accessibility for users on my site.

Expected behavior

Use the tab key to navigate through interactive elements on a web page.

Actual behavior

Search bar comes into focus.

Steps to reproduce the bug

  1. Go to http://squidfunk.github.io/mkdocs-material/
  2. Press tab
  3. Observe you are in the Search bar, as opposed to any other content on the page.
  4. Press tab more.
  5. You're still in the Search bar.

Package versions

  • Python: 2.7.10
  • MkDocs: 0.16.3
  • Material: 1.7.4

Project configuration

https://github.com/squidfunk/mkdocs-material/blob/master/mkdocs.yml :)

@squidfunk
Copy link
Owner

Keyboard accessibility is a long standing thing I wanted to optimize, but I never had the time. If you come up with a good concept, I'm happy to integrate it. The problem is that the blur event cannot be used to close the search bar because that would not allow clicking or navigating through the results.

Secondly, when there are search results, Tab is mapped to Key Down while Shift+ Tab is mapped to Key Up and it wraps at the end of the search results.

How would you handle that and what is the order in which you would expect it to go through the page?

@squidfunk squidfunk self-assigned this Jul 20, 2017
@squidfunk squidfunk added the bug Issue reports a bug label Jul 20, 2017
@paulgibbs
Copy link
Author

I'll talk to the accessibility expert on our team and get back to you with her suggestions.

@rianrietveld
Copy link

Looking at:

<div class="md-flex__cell md-flex__cell--shrink">
        
	<label class="md-icon md-icon--search md-header-nav__button" for="search"></label>

	<div class="md-search" data-md-component="search">
		<label class="md-search__overlay" for="search"></label>
		<div class="md-search__inner">
			<form class="md-search__form" name="search">
				<input type="text" class="md-search__input" name="query" required="" placeholder="Search" accesskey="s" autocapitalize="off" autocorrect="off" autocomplete="off" spellcheck="false" data-md-component="query">
				<label class="md-icon md-search__icon" for="search"></label>
				<button type="reset" class="md-icon md-search__icon" data-md-component="reset">close</button>
			</form>
			<div class="md-search__output">
				<div class="md-search__scrollwrap" data-md-scrollfix="">
					<div class="md-search-result" data-md-component="result" data-md-lang-search="">
						<div class="md-search-result__meta" data-md-lang-result-none="No matching documents" data-md-lang-result-one="1 matching document" data-md-lang-result-other="# matching documents">
							Type to start searching
						</div>
						<ol class="md-search-result__list"></ol>
					</div>
				</div>
			</div>
		</div>
	</div>

</div>

I noticed the following:

Issue: The keyboard focus drops out of the search modal and continues in the site itself, without closing the modal.
Fix: Add role="dialog" to the modal and keep the focus inside the modal (with JS firstInput.focus() and lastInput.focus(), only to leave when submitting or pressing Esc.
See for docs and an example: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_dialog_role

Issue: There is no submit button (only a button type="reset"). I think the submit action is set on the three labels for different views? A label gets no keyboard focus and is not the semantically right element for submitting
Fix: add real submit button (inside the form) and add a visual change on focus.

Issue: There are 3 labels for="search" with no content and no associated id. Don't use a label other then in a form for describing a connected form field.
Fix: Add one label for="search" inside the form and hide the text with screen reader text:
for example Search
Add an id="search" to the corresponding input field.
This way the label and input field are connected.
For screen-reader-text see: https://blog.rrwd.nl/2015/04/04/the-screen-reader-text-class-why-and-how/

@squidfunk
Copy link
Owner

Thanks for your input and the linked resources. That's a lot of work to do and as I said, accessibility is an important topic and I will see when I can find the time. Any help is appreciated.

@rianrietveld
Copy link

rianrietveld commented Jul 21, 2017

Hey @squidfunk Thanks for reading.

Just saw that now, quick win:
Please don't use accesskey="s". It hinders people using assistive technology, like screen readers.

@squidfunk
Copy link
Owner

BTW: the search doesn't have a submit button because it's "search-as-you-type" and then you select the result which is a link to follow that is focused.

@rianrietveld
Copy link

@squidfunk
If you have time for this some day, we can look at this together to find a good solution that works with keyboard and is semantically correct. You know where to find me :-)
cc @paulgibbs

@max-ci
Copy link

max-ci commented Nov 5, 2017

I noticed another small thing. When you find some documents in the search you often have to press TAB/Key Down twice to focus on first result.

@squidfunk
Copy link
Owner

See #659 for a first improval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants