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

Fix tab navigation of menu in public share pages #17861

Merged
merged 3 commits into from
Jan 7, 2020

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Nov 8, 2019

@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-expanded attribute 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).

@danxuliu danxuliu added bug 3. to review Waiting for reviews labels Nov 8, 2019
@danxuliu danxuliu added this to the Nextcloud 18 milestone Nov 8, 2019
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Code looks good

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Looks good too me

@jancborchardt
Copy link
Member

@danxuliu how to test, or do you have screenshots? :)

Also, the test seems failing?

@rullzer
Copy link
Member

rullzer commented Dec 5, 2019

beta2 this is then I guess

This was referenced Dec 11, 2019
@rullzer rullzer mentioned this pull request Dec 19, 2019
18 tasks
@rullzer rullzer mentioned this pull request Dec 27, 2019
13 tasks
@jancborchardt
Copy link
Member

@danxuliu how to test, or do you have screenshots? :)

@danxuliu? :)

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 danxuliu force-pushed the fix-tab-navigation-of-menu-in-public-share-pages branch from 833d63d to 883a71c Compare December 30, 2019 10:34
@danxuliu
Copy link
Member Author

Rebased.

How to test

  • Share a file by link
  • Open the link
  • Press tab to navigate through the page elements with the keyboard

Result with this pull request

The 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 request

The three dots button that shows the menu in the public share page can not be focused by pressing the tab key.

@rullzer rullzer mentioned this pull request Jan 2, 2020
1 task
@rullzer rullzer merged commit 86bccde into master Jan 7, 2020
@rullzer rullzer deleted the fix-tab-navigation-of-menu-in-public-share-pages branch January 7, 2020 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants