mozilla
Flag review menu does not close when navigating it with a keyboard
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 :
┆Issue is synchronized with this Jira Task
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
Yes, possibly. If you look at the DOM, the flag menu is buried at the bottom (iirc) which is part of the problem.
What Kumar is referring to: https://github.com/mozilla/addons-frontend/blob/2d0ba6219d6e213822d192991026f530ffe70b0e/src/amo/components/AddonReviewCard/index.js#L452-L456
https://github.com/mozilla/addons-frontend/blob/2d0ba6219d6e213822d192991026f530ffe70b0e/src/amo/components/FlagReviewMenu/index.js#L73
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()
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.
Hi @vcarciu @kumar303, I am an Outreachy participant for May 2020. I would like to work on this issue.
Hi, thanks for your interest. @muffinresearch maybe you could help coordinate?
@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 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.
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.
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!
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.
