Regression: keyboard navigation in Popovers

[manuhabitela]

Description:

👋 Manu here, I helped a bit on screen reader/keyboard usage of Jitsi last year with your help @Calinteodor and @saghul.

The issue I raise here is related to popovers. Last year work was made so that using "tab" to navigate between toolbar buttons and popovers was straightforward:

Before:

jitsi-popovers-before.mp4

The issue now is: it seems broken. You can't focus popover content anymore in a clear manner. It's back to its old behavior, where you have to tab to the end of the DOM since popovers are added at the bottom of the DOM. This is not ideal as screen reader users, or even keyboard users, can't really guess you have to do that (especially screen reader users).

After (current behavior):

jitsi-popovers-after.mp4

Steps to reproduce:

1. Open Jitsi on any screen containing the toolbar
2. Use the Tab key to navigate through toolbar items until you focus an arrow to show content related to a button, for example, the Audio settings popover related to the mic mute/unmute item.
3. Press Enter to open the popover
4. Press the Tab key

Expected behavior:

Keyboard focus should be on the popover itself.

Actual behavior:

Keyboard focus is on the next toolbar item. Popover can be focused when going manually to the end of the DOM.

Server information:

• Jitsi Meet version: jitsi-meet_9404
• Operating System: Linux

Client information:

• Browser / app version: Chrome 122
• Operating System: Linux

Additional information:

It seems the regression happened on this commit by @hristoterezov: 6187bb9. Maybe you can help? :) I admit I have trouble understanding what the commit does…

Locally, to fix the issue, I just have to change the line 285 on react/features/base/popover/components/Popover.web.tsx:

<FocusOn
                                // Use the `enabled` prop instead of conditionally rendering ReactFocusOn
                                // to prevent UI stutter on dialog appearance. It seems the focus guards generated annoy
                                // our DialogPortal positioning calculations.
---                            enabled = { Boolean(this._contextMenuRef) && this.state.enableFocusLock }
+++                            enabled = { this.state.enableFocusLock }

Sadly I don't have much time to look this more right now but hopefully this is something that could be quickly fixed if you think this tiny change is OK?

Thanks!

[saghul]

Alas the commit doesn't go into details :-/

By the looks of it, however, it feels like the popover should have a reference? or maybe the problem is due to some shared code with contect menus...

[manuhabitela]

At the time I added this (my "fix" proposal here is just me setting back the line to what I wrote it last year), I didn't get into any trouble, neither did @Calinteodor, but yeah maybe we missed something.

[hristoterezov]

When I looked into my notes it seems #13497 is fixing an issue with invalid reference and the UX have been that the popups are flickering or not displaying.

[manuhabitela]

OK thanks. In what case was there flickering / non displaying popovers?

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.