fix(@clayui/drop-down): fixes bug with chrome blocking element focus when clicking outside the menu

[matuzalemsteles]

Context https://liferay.slack.com/archives/C04CPMRMXAL/p1723558235946469

Well, basically, the message from Chrome about blocking focus due to the use of aria-hidden seems intermittent and only happens once, at least in the tests I've done, but I can still see the focused element when adding a watch to document.activeElement.

So I think this is more of a warning about good practices or avoiding problems with SR, but I think this would be an unrealistic case, perhaps since when using SR in a real case, you are normally using keyboard interaction and when you close the menu, the focus returns to the trigger that does not have aria-hidden, so this will not happen in this use case. I believe that this may be another bug in Chrome that should show this message in case of keyboard interaction or is just being shown to the developer as a warning when the console is opened.

This is my assumption, but in any case, we can change to using inert, but this causes a different interaction, for example, closing the menu by clicking on an input will no longer receive focus, just by clicking it again.

Another option would be to remove the use of aria-hidden or inert when it's a mouse interaction and add it when it's a keyboard interaction. Thoughts?

[ethib137]

Hey @matuzalemsteles I'm able to reproduce this consistently with the following:

1. Navigate to https://clayui.com/docs/components/drop-down.html
2. Click on the Select button to open the dropdown.
3. Click on the Select button to close the dropdown.

Can you explain a bit more about why we need to add aria-hidden to the trigger element? It sounded like you were saying that enables the focus trap, but I didn't understand why. Thanks.

[matuzalemsteles]

@ethib137 Yes, I can do that too, but it doesn't appear anymore, only after refreshing the page a few times can I see the error...

Can you explain a bit more about why we need to add aria-hidden to the trigger element? It sounded like you were saying that enables the focus trap, but I didn't understand why. Thanks.

In fact, the trigger was not supposed to have aria-hidden, which was fixed in this PR... but I was able to reproduce it in the codesandbox environment that Dave shared and I can see this error in focusable elements besides the trigger, so this is not just the trigger.

The focus trap is only for assertiveness for SR, especially JAWS. For example, the trigger and the menu are not in the same hierarchy in the DOM, so when we have more focusable elements on the screen, JAWS would get lost, so we used aria-hidden to hide all the elements other than the menu and the trigger so that SR knows that there are only these elements on the screen. So basically we created a focus trap, although we still control the focus via JS as well.

Another detail is that using inert breaks the behavior, for example, when the menu is open and pressing Tab, the focus goes to the body instead of the next focusable element after the trigger. This is because all the elements are inert.

[ethib137]

Hey @matuzalemsteles, I think to fix this we just need to make sure that call the cleanup function returned by hideOthers on the start interaction of clickOutside and before focus is returned to the trigger on close. I tested this for clickOutside and it worked.

This is pretty low priority for us though, so lets just create a ticket for it and focus on the NextJS migration. Thanks.

[matuzalemsteles]

I think to fix this we just need to make sure that call the cleanup function returned by hideOthers on the start interaction of clickOutside and before focus is returned to the trigger on close. I tested this for clickOutside and it worked.

This works a little differently for inert if we use it, but I think it might work if we use it for aria-hidden. We tried to do this with inert

clay/packages/clay-shared/src/Overlay.tsx

Lines 92 to 98 in 3b6a49e

	// When inert is used to suppress user interaction with the rest of
	// the document, to retrieve the focus in the trigger we need to
	// first undo and then move the focus.
	if (unsuppressCallbackRef.current) {
	unsuppressCallbackRef.current();
	unsuppressCallbackRef.current = null;
	}

for the esc use case, it works fine because we have focus control. I'll make some adjustments, if it takes too long I'll create a ticket for it and put it on hold for now.

[matuzalemsteles]

Okay, I made some changes that seem to work fine now without having to use inert, removing aria-hidden before focusing on the element as @ethib137 mentioned.

[ethib137]

LGTM. Thanks @matuzalemsteles