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

popovers: Enable keyboard navigation on user profile menu. #9734

Merged
merged 6 commits into from
Jun 13, 2018

Conversation

shubham-padia
Copy link
Member

Fixes #9318.

peek 2018-06-12 16-09

From the original issue:

poke around other menus while you're at it to see if similar bugs exist

Keyboard navigation worked in the other menus which were being open by keyboard.

g | Toggle the gear menu
i | Open message menu
: | Open reactions menu
? | Show keyboard shortcuts

actions_menu_handle_keyboard now only gets the action menu items
and passes them to the newly added popover_items_handle_keyboard.
popover_items_handle_keyboard takes the key and menu items as its
parameters. The function can be used when handling keyboard input
like user profile popover. Similar refactor has been carried out in
focus_first_action_popover_item. This refactor is a part of adding
the missing support of keyboard navigation to user profile popover.
Gets menu items for the user profile popover. To be used to add
keyboard navigation to user_profile_menu.
Adds focus_user_info_popover_item to popovers.js. When menus are
opened by hotkeys, first menu item is focused to enable further
keyboard navigation.
actions_dropdown_hotkeys are also to be used for user profile menu
keyboard navigation, so renaming it to a more generic name.
Fixes zulip#9318.
Calls popovers.user_info_popover_handle_keyboard in process_hotkey.js.
Makes popovers.message_info_popped public.
@showell
Copy link
Contributor

showell commented Jun 12, 2018

The code looks nice here, but it doesn't seem to work if I'm Aaron and type "u" over a message sent from Othello. I can arrow to the second item, but not the third. I suspect the selector to find items is broken somehow.

For other users' profile, keyboard navigation worked only upto 2
items as the third item with '.mention_user' could not be focused.
This was due to href missing from the anchor tag. $.focus() requires
href to be defined, even if it is '#' to focus the anchor tag.
@shubham-padia
Copy link
Member Author

@showell I've updated the PR with the fix.

For other users' profile, keyboard navigation worked only upto 2
items as the third item with '.mention_user' could not be focused.
This was due to href missing from the anchor tag. $.focus() requires
href to be defined, even if it is '#' to focus the anchor tag.

peek 2018-06-13 06-17

@showell showell merged commit 3a742d8 into zulip:master Jun 13, 2018
@showell
Copy link
Contributor

showell commented Jun 13, 2018

Merged, thanks @shubham-padia!

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

Successfully merging this pull request may close these issues.

None yet

3 participants