Fix tab navigation of menu in public share pages#17861
Merged
Conversation
Member
|
@danxuliu how to test, or do you have screenshots? :) Also, the test seems failing? |
Member
|
beta2 this is then I guess |
Member
This makes possible to navigate to the menu toggle using the keyboard, as well as being semantically more correct. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The external shares entry showed a "button" that, when pressed, replaced the button with the input to set the remote share address. The "button" was actually a label for the input, so when the label was focused it transferred the focus to the input and thus pressing enter or space did not show the input. Moreover, inputs inside links are not valid HTML, and once shown there was no way to hide the input again. Due to all this, and for consistency with the direct link input, the external share input was moved to a different menu item that is shown and hidden when the button, which nows is also a real button, is clicked. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
force-pushed
the
fix-tab-navigation-of-menu-in-public-share-pages
branch
from
833d63d to
883a71c
Compare
Member
Author
|
Rebased. How to test
Result with this pull requestThe three dots button that shows the menu in the public share page can be focused by pressing the tab key. The menu can be opened by pressing enter/space when the button is focused, and it is possible to navigate through the menu with the tab key. Result without this pull requestThe three dots button that shows the menu in the public share page can not be focused by pressing the tab key. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
@jancborchardt I tried to show and hide the menu item with a nice transition... but I failed, sorry :-P
I have also noticed that the
aria-expandedattribute in the menu toggle is not changed when the menu is opened, but I do not have the time right now to check what is wrong (but it seems that OC.Menu.registerMenu is not used in this menu).