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

Flag review menu does not close when navigating it with a keyboard #2029

Open
vcarciu opened this issue Oct 25, 2017 · 16 comments
Open

Flag review menu does not close when navigating it with a keyboard #2029

vcarciu opened this issue Oct 25, 2017 · 16 comments

Comments

@vcarciu
Copy link

vcarciu commented Oct 25, 2017

Steps to reproduce:
1.Go an add-on detail page
2.Click on Read all reviews to see the list of reviews
3.When the page loads, use only the tab key to navigate the page
4.When you tab to the flag link press the space bar or enter key to flag a review
5.When drop down menu is collapsed, navigate through options by pressing Tab on keyboard

Expected results:
When reaching the last option, one more Tab press should focus the first option on the drop down(basically user should stay inside the opened drop down menu )

Actual results:
The next option on site is selected and user is not able to go back in the drop down menu without going through entire site with the keyboard

NOTES:
Reproduced in AMO-dev, FF Release

Please see video for this issue :
navigationindropdown

┆Issue is synchronized with this Jira Task

@vcarciu
Copy link
Author

vcarciu commented Oct 25, 2017

Derived from #10980

@kumar303 kumar303 changed the title In flag review drop down menu, when navigating via keyboard, focus should not jump out of the drop down menu until it is closed Flag review menu does not close when navigating it with a keyboard Oct 25, 2017
@seanprashad
Copy link

seanprashad commented Dec 19, 2018

I think we'd need to look into using tabindex or focus().

Do we want the user to be able to cycle through the 3 options or simply close the menu if they tab enough times to move off the final option?

Code in question can be found here:

https://github.com/mozilla/addons-frontend/blob/06b8556c2ecd33f7d2c68f7692994e809c16a42d/src/ui/components/TooltipMenu/index.js#L45

@kumar303
Copy link
Contributor

Yes, possibly. If you look at the DOM, the flag menu is buried at the bottom (iirc) which is part of the problem.

@seanprashad
Copy link

seanprashad commented Dec 20, 2018

I was looking into using focus() but after making this small codepen, it isn't cycling through each button as I hoped..

Just curious @kumar303 - you mentioned that FlagReviewMenu is "buried" at the bottom - should I look to actually move that around somewhere else?

Logically, we need to have the tabindex set such that button 1 will be before button 2, and button 2 will be before button 3, BUT button 3 will point to button 1's tabindex - still investigating alternatives to focus()

@kumar303
Copy link
Contributor

should I look to actually move that around somewhere else?

Well, I think that would be hard because this was the way that the third party component was implemented.

@ani-sha
Copy link

ani-sha commented Mar 6, 2020

Hi @vcarciu @kumar303, I am an Outreachy participant for May 2020. I would like to work on this issue.

@valkyr13
Copy link

@vcarciu @kumar303 I'm an Outreachy applicant, can I work on this issue?

@kumar303
Copy link
Contributor

Hi, thanks for your interest. @muffinresearch maybe you could help coordinate?

@muffinresearch
Copy link
Contributor

@ani-sha, @valkyr13 this issue is probably not the best place to start, maybe take a look at something from this list https://github.com/mozilla/addons-frontend/issues?q=is%3Aopen+is%3Aissue+label%3A%22contrib%3A+outreachy%22

@avgupt
Copy link

avgupt commented Mar 27, 2020

Can I take up this issue?

@bobsilverberg
Copy link
Contributor

@avgupt I see you have another PR in progress, and we think it best that you not work on more than a couple of issues at a time. You can take this on, but looking at it it looks pretty tricky to me, and I cannot with confidence say that we'll be able to give you much help in solving it. Please feel free to give it a try, but it might not be an ideal issue for your purposes.

@avgupt avgupt removed their assignment Apr 9, 2020
@avgupt
Copy link

avgupt commented Apr 9, 2020

Hey @bobsilverberg , I gave it a shot but the tooltip that's being used is a third party component. I'm just stuck at how I could trigger a close command for tooltip, when the user presses Tab after reaching the third row.
I'll give it a try in sometime, but for now I'm unassigning myself.
If you have any ideas please let me know.

@shribyte
Copy link

shribyte commented Sep 20, 2023

I've been working on an implementation for the bug.

The idea is to add React refs for the first and last list items in the items prop (will be used to control the focus when navigating the dropdown via keyboard)
Then utilize a handleKeyDown event handler that allows looping through the menu (loops to the first item after the last one)

Would love your feedback!

@shribyte
Copy link

I do realize that the third party component RCTooltip makes it not so easy to close the menu. Welcome any suggestions here.

I don't see suggestions regarding this in parallel efforts such as this.

One way out may be replacing the third party tooltip, but this doesn't seem to be the most efficient approach.

For now, I will be pushing changes that allow looping the menu, however, this isn't ideal given that we wouldn't be able to break out of the menu.

@KevinMind
Copy link
Contributor

@KevinMind KevinMind added repository:addons-frontend Issue relating to addons-frontend migration:2024 labels May 3, 2024
@KevinMind KevinMind transferred this issue from mozilla/addons-frontend May 3, 2024
@diox diox removed triaged labels May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.