Change focus target on items when using keyboard navigation #2348
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks like there is a regression regarding the styling. The border color is now black with the light theme. Current:
This pull-request:
|
|
This looks all good to me now, also what was committed in the other PRs. I have no more issue on linux, can navigate in all ways (headding, j/k or the arrow keys) and all is read fine. Also on the Mac everything is good and titles are not longer announced twice. Very cool and a big tank you to @krvpb024 krvpb024 |
2 tasks
fguillot
pushed a commit
that referenced
this pull request
Feb 22, 2024
There are a few things that need to be done, to make this work. First, we need to register `Enter` as another hotkey that opens the selected item. However, by default the `KeyboardHandler` will override all default actions. That might make sense for any other key, but for the `Enter` key, we want to keep the default behavior (i.e. follow a selected link or press a button). So for this single key event, we do not call `preventDefault()`. I see this as unproblematic for the following reasons. 1. With the changes from #2348, when we're in a list of items (articles, categories, feeds), there is no link selected. This is what made the `Enter` key work _implicitly_ in the past. With nothing selected, the `Enter` key will do nothing by default. 2. If we have **any** link selected (including when we are in a view with a list of selectable items), we'll get the default action of `Enter` (i.e. follow a link), which is exactly what we had before. Lastly, we need to update the list of keyboard shortcuts displayed when pressing `?`. This fixes #2366.
jeankhawand
pushed a commit
to jeankhawand/v2
that referenced
this pull request
Mar 20, 2024
There are a few things that need to be done, to make this work. First, we need to register `Enter` as another hotkey that opens the selected item. However, by default the `KeyboardHandler` will override all default actions. That might make sense for any other key, but for the `Enter` key, we want to keep the default behavior (i.e. follow a selected link or press a button). So for this single key event, we do not call `preventDefault()`. I see this as unproblematic for the following reasons. 1. With the changes from miniflux#2348, when we're in a list of items (articles, categories, feeds), there is no link selected. This is what made the `Enter` key work _implicitly_ in the past. With nothing selected, the `Enter` key will do nothing by default. 2. If we have **any** link selected (including when we are in a view with a list of selectable items), we'll get the default action of `Enter` (i.e. follow a link), which is exactly what we had before. Lastly, we need to update the list of keyboard shortcuts displayed when pressing `?`. This fixes miniflux#2366.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of this pull request
When focusing on an element in JavaScript, some screen readers will also announce its parent's label. When the focused element's label is the same as the parent's label, users may get the same text twice. macOS VoiceOver will announce two times; NVDA and JAWS will announce three times. Some screen reader users think this behavior should be adjust.
Current behavior
The current version focuses on the link element, and it is wrapped by a h2 tag, and the h2 tag is inside an article that has a label. This caused the screen reader behavior that I mentioned above.
H2 and the article's label can't be removed because screen reader users who preferred to navigate by heading or landmark/article will need this information.
The benefit of this approach is that when users use the keyboard shortcut to navigate, they can press the Enter key directly to open a link, without other actions. This is the default behavior of links. Users don't need to remember the Miniflux keyboard shortcut to open the selected item.
Change focus target to article
I changed the focus target to the article element, and then it will just announce the entry title once when using the screen reader.
After this change, users who use keyboard navigation will be affected. Either they have to press the tab once to focus on the link, then press the Enter key to open the link, or they need to remember the open selected item shortcut, which is the O key, then press the O key to open entry.
And the benefit of this approach is that screen reader users can get concise information. I have tested on macOS VoiceOver and Windows NVDA. It only announces the entry title once.
Do you follow the guidelines?