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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Select] moving focus with keyboard ArrowUp or ArrowDown is not working when [disablePortal] is set #34218

Open
2 tasks done
teetotum opened this issue Sep 7, 2022 · 8 comments
Assignees
Labels
bug 馃悰 Something doesn't work component: select This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material

Comments

@teetotum
Copy link

teetotum commented Sep 7, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 馃槸

When I click into a <Select> (with disablePortal) the dropdown opens, but ArrowUp and ArrowDown will not move the keyboard focus to any of the items.
Addendum: I verified that the autoFocus property does not influence the behavior. It does not matter whether autoFocus is set or not.

Expected behavior 馃

Without disablePortal the behavior is as expected: keyboard ArrowUp and ArrowDown keys will move the focus to the items and between the items.

Steps to reproduce 馃暪

I prepared a codesandbox that exhibits the behavior
https://codesandbox.io/s/pensive-cache-xk9js4?file=/src/App.tsx

The relevant usage to elicit the bug is:

<Select MenuProps={{ disablePortal: true }}>
    <MenuItem value="foo">{"foo"}</MenuItem>
</Select>
  1. click into the select
  2. use ArrowDown to move focus into the dropdown. but nothing happens.

Observation:
When disablePortal is set the dropdown paper will receive focus when the dropdown opens. This is not the case otherwise.

Context 馃敠

No response

Your environment 馃寧

No response

@teetotum teetotum added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 7, 2022
@teetotum
Copy link
Author

teetotum commented Sep 8, 2022

I've developed a workaround that anyone can use until this is fixed.
I listen for the native bubbling DOM event focusin and check if the paper has focus; if this is the case I just focus the first item. Done.

import { useFocusIn } from './useDOMEvent';

// ...

const { ref } = useFocusIn((e: any) => {
    if (e.target.matches('.MuiPaper-root')) {
      const list = e.target.querySelector('.MuiList-root');
      list?.firstElementChild?.focus();
    }
  });

<Select
    {...props}
    ref={ref}
>
    {options}
</Select>

and the custom hook

import { useEffect, useState } from 'react';

export const useDOMEvent = (
  type: keyof GlobalEventHandlersEventMap,
  listener: EventListenerOrEventListenerObject,
  options?: AddEventListenerOptions
) => {
  const [target, setTarget] = useState<any>();
  const { capture, once, passive, signal } = { ...options };

  useEffect(() => {
    if (target) {
      target.addEventListener(type, listener, { capture, once, passive, signal });
      return () => target.removeEventListener(type, listener, { capture });
      // be aware that EventListenerOptions used for 'remove' do not mirror AddEventListenerOptions
    }
  }, [target, type, listener, capture, once, passive, signal]);

  return { ref: setTarget };
};

export const useFocusIn = (listener: EventListenerOrEventListenerObject, options?: AddEventListenerOptions) =>
  useDOMEvent('focusin', listener, options);
export const useFocusOut = (listener: EventListenerOrEventListenerObject, options?: AddEventListenerOptions) =>
  useDOMEvent('focusout', listener, options);

@hbjORbj hbjORbj added the component: select This is the name of the generic UI component, not the React module! label Sep 9, 2022
@michaldudak
Copy link
Member

Thanks for reporting this issue and the workaround. Would you like to investigate the root cause of this in our Select component?

@michaldudak michaldudak added bug 馃悰 Something doesn't work package: material-ui Specific to @mui/material and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 12, 2022
@teetotum
Copy link
Author

I would like to give it a try. I can't say when I will find the time though. Maybe next friday.

@michaldudak michaldudak assigned teetotum and unassigned michaldudak Sep 13, 2022
@teetotum
Copy link
Author

teetotum commented Sep 16, 2022

I forked and cloned the repo today. Currently trying to write a failing testcase for the bug.
working in packages\mui-material\src\Select\Select.test.js
and trying to modify a duplicate of this existing testcase, which seems to be close to what I'm attempting:

it('should focus list if no selection', () => {
    const { getByRole } = render(<Select value="" autoFocus />);

    fireEvent.mouseDown(getByRole('button'));

    // TODO not matching WAI-ARIA authoring practices. It should focus the first (or selected) item.
    expect(getByRole('listbox')).toHaveFocus();
});

Update: I have to declare defeat for today. Didn't succeed in creating a failing testcase. I went down the rabbit hole from the Select to the SelectInput -> Menu -> Popover -> Modal -> ModalUnstyled -> Portal and TrapFocus.
My guess at the moment is that something in the TrapFocus might be interfering: it listens for native bubbling focusin events and redistributes focus if it catches any. But the DOM structure and therefore the way native bubbling events will travel up the tree is different depending on whether a portal is used or not. So the root cause might be somewhere there. But that's just speculation for now.
I have an idea how to tackle it further but will need a few carefree hours; maybe sunday. I have to do this in my spare time and that's scarce.

Update: no progress, had to work on other things. I want to try this idea to figure out what happens to focus.

@GLObus303
Copy link

GLObus303 commented Dec 5, 2022

Hey, it seems it could also fix the hinting when the user is typing during the opened list. Which also does not work with disablePortal: true. Both are quite impactful for the accessibility of the component

By "hinting" I mean this functionality:

CleanShot 2022-12-05 at 13 42 44

@litera
Copy link

litera commented Feb 1, 2023

When anyone tackles this issue keep in mind that the first item in the list may not be selectable (i.e. disabled or ListSubhead). Focus should be put on the first selectable item in the list.

@litera
Copy link

litera commented Feb 3, 2023

Some more behaviour details. Here is a working example for testing.

When portal is enabled (default), keyboard interaction seems to work as expected. Either when opening the dropdown using the keyboard (focus on the select pushing the ArrowDown/UpArrow/Enter/Space key) or opening it using a mouse (click the select field). After the dropdown opens, the list is fully navigatable by keyboard.

But when the portal is disabled, then opening the dropdown with a mouse completely looses keyboard interaction. Mind that keyboard interaction still works if you open the dropdown using the keyboard. SO if you open the dropdown using the keyboard, you can freely use the keyboard.

@yonimar87
Copy link

I assume this still isn't fixed? I've got the same problem at the moment. I'll try do the workaround meanwhile and see if that works for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 馃悰 Something doesn't work component: select This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
Development

No branches or pull requests

7 participants