[Select] Fix incorrect selecting of first element (#36024)

Co-authored-by: Marija Najdova <[email protected]>

docs/data/material/components/selects/selects.md

@@ -127,6 +127,67 @@ Display categories with the `ListSubheader` component or the native `<optgroup>`
 
 {{"demo": "GroupedSelect.js"}}
 
+:::warning
+If you wish to wrap the ListSubheader in a custom component, you'll have to annotate it so Material UI can handle it properly when determining focusable elements.
+
+You have two options for solving this:
+Option 1: Define a static boolean field called `muiSkipListHighlight` on your component function, and set it to `true`:
+
+```tsx
+function MyListSubheader(props: ListSubheaderProps) {
+  return <ListSubheader {...props} />;
+}
+
+MyListSubheader.muiSkipListHighlight = true;
+export default MyListSubheader;
+
+// elsewhere:
+
+return (
+  <Select>
+    <MyListSubheader>Group 1</MyListSubheader>
+    <MenuItem value={1}>Option 1</MenuItem>
+    <MenuItem value={2}>Option 2</MenuItem>
+    <MyListSubheader>Group 2</MyListSubheader>
+    <MenuItem value={3}>Option 3</MenuItem>
+    <MenuItem value={4}>Option 4</MenuItem>
+    {/* ... */}
+  </Select>
+```
+
+Option 2: Place a `muiSkipListHighlight` prop on each instance of your component.
+The prop doesn't have to be forwarded to the ListSubheader, nor present in the underlying DOM element.
+It just has to be placed on a component that's used as a subheader.
+
+```tsx
+export default function MyListSubheader(
+  props: ListSubheaderProps & { muiSkipListHighlight: boolean },
+) {
+  const { muiSkipListHighlight, ...other } = props;
+  return <ListSubheader {...other} />;
+}
+
+// elsewhere:
+
+return (
+  <Select>
+    <MyListSubheader muiSkipListHighlight>Group 1</MyListSubheader>
+    <MenuItem value={1}>Option 1</MenuItem>
+    <MenuItem value={2}>Option 2</MenuItem>
+    <MyListSubheader muiSkipListHighlight>Group 2</MyListSubheader>
+    <MenuItem value={3}>Option 3</MenuItem>
+    <MenuItem value={4}>Option 4</MenuItem>
+    {/* ... */}
+  </Select>
+);
+```
+
+We recommend the first option as it doesn't require updating all the usage sites of the component.
+
+Keep in mind this is **only necessary** if you wrap the ListSubheader in a custom component.
+If you use the ListSubheader directly, **no additional code is required**.
+:::
+
 ## Accessibility
 
 To properly label your `Select` input you need an extra element with an `id` that contains a label.

packages/mui-material/src/ListSubheader/ListSubheader.js

@@ -100,6 +100,8 @@ const ListSubheader = React.forwardRef(function ListSubheader(inProps, ref) {
   );
 });
 
+ListSubheader.muiSkipListHighlight = true;
+
 ListSubheader.propTypes /* remove-proptypes */ = {
   // ----------------------------- Warning --------------------------------
   // | These PropTypes are generated from the TypeScript type definitions |

packages/mui-material/src/MenuList/MenuList.js

@@ -232,6 +232,17 @@ const MenuList = React.forwardRef(function MenuList(props, ref) {
         activeItemIndex = index;
       }
     }
+
+    if (
+      activeItemIndex === index &&
+      (child.props.disabled || child.props.muiSkipListHighlight || child.type.muiSkipListHighlight)
+    ) {
+      activeItemIndex += 1;
+      if (activeItemIndex >= children.length) {
+        // there are no focusable items within the list.
+        activeItemIndex = -1;
+      }
+    }
   });
 
   const items = React.Children.map(children, (child, index) => {

packages/mui-material/src/Select/Select.test.js

@@ -10,7 +10,7 @@ import {
   screen,
 } from 'test/utils';
 import { createTheme, ThemeProvider } from '@mui/material/styles';
-import MenuItem from '@mui/material/MenuItem';
+import MenuItem, { menuItemClasses } from '@mui/material/MenuItem';
 import ListSubheader from '@mui/material/ListSubheader';
 import InputBase from '@mui/material/InputBase';
 import OutlinedInput from '@mui/material/OutlinedInput';
@@ -393,6 +393,21 @@ describe('<Select />', () => {
     });
   });
 
+  it('should not have the selectable option selected when inital value provided is empty string on Select with ListSubHeader item', () => {
+    render(
+      <Select open value="">
+        <ListSubheader>Category 1</ListSubheader>
+        <MenuItem value={10}>Ten</MenuItem>
+        <ListSubheader>Category 2</ListSubheader>
+        <MenuItem value={20}>Twenty</MenuItem>
+        <MenuItem value={30}>Thirty</MenuItem>
+      </Select>,
+    );
+
+    const options = screen.getAllByRole('option');
+    expect(options[1]).not.to.have.class(menuItemClasses.selected);
+  });
+
   describe('SVG icon', () => {
     it('should not present an SVG icon when native and multiple are specified', () => {
       const { container } = render(
@@ -549,8 +564,57 @@ describe('<Select />', () => {
       });
     });
 
+    describe('when the first child is a ListSubheader wrapped in a custom component', () => {
+      describe('with the `muiSkipListHighlight` static field', () => {
+        function WrappedListSubheader(props) {
+          return <ListSubheader {...props} />;
+        }
+
+        WrappedListSubheader.muiSkipListHighlight = true;
+
+        it('highlights the first selectable option below the header', () => {
+          const { getByText } = render(
+            <Select defaultValue="" open>
+              <WrappedListSubheader>Category 1</WrappedListSubheader>
+              <MenuItem value={1}>Option 1</MenuItem>
+              <MenuItem value={2}>Option 2</MenuItem>
+              <WrappedListSubheader>Category 2</WrappedListSubheader>
+              <MenuItem value={3}>Option 3</MenuItem>
+              <MenuItem value={4}>Option 4</MenuItem>
+            </Select>,
+          );
+
+          const expectedHighlightedOption = getByText('Option 1');
+          expect(expectedHighlightedOption).to.have.attribute('tabindex', '0');
+        });
+      });
+
+      describe('with the `muiSkipListHighlight` prop', () => {
+        function WrappedListSubheader(props) {
+          const { muiSkipListHighlight, ...other } = props;
+          return <ListSubheader {...other} />;
+        }
+
+        it('highlights the first selectable option below the header', () => {
+          const { getByText } = render(
+            <Select defaultValue="" open>
+              <WrappedListSubheader muiSkipListHighlight>Category 1</WrappedListSubheader>
+              <MenuItem value={1}>Option 1</MenuItem>
+              <MenuItem value={2}>Option 2</MenuItem>
+              <WrappedListSubheader muiSkipListHighlight>Category 2</WrappedListSubheader>
+              <MenuItem value={3}>Option 3</MenuItem>
+              <MenuItem value={4}>Option 4</MenuItem>
+            </Select>,
+          );
+
+          const expectedHighlightedOption = getByText('Option 1');
+          expect(expectedHighlightedOption).to.have.attribute('tabindex', '0');
+        });
+      });
+    });
+
     describe('when the first child is a MenuItem disabled', () => {
-      it('first selectable option is focused to use the arrow', () => {
+      it('highlights the first selectable option below the header', () => {
         const { getAllByRole } = render(
           <Select defaultValue="" open>
             <MenuItem value="" disabled>

packages/mui-material/src/Select/SelectInput.js

@@ -350,7 +350,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
     }
   }
 
-  const items = childrenArray.map((child, index, arr) => {
+  const items = childrenArray.map((child) => {
     if (!React.isValidElement(child)) {
       return null;
     }
@@ -391,26 +391,6 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
       foundMatch = true;
     }
 
-    if (child.props.value === undefined) {
-      return React.cloneElement(child, {
-        'aria-readonly': true,
-        role: 'option',
-      });
-    }
-
-    const isFirstSelectableElement = () => {
-      if (value) {
-        return selected;
-      }
-      const firstSelectableElement = arr.find(
-        (item) => item?.props?.value !== undefined && item.props.disabled !== true,
-      );
-      if (child === firstSelectableElement) {
-        return true;
-      }
-      return selected;
-    };
-
     return React.cloneElement(child, {
       'aria-selected': selected ? 'true' : 'false',
       onClick: handleItemClick(child),
@@ -427,10 +407,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
         }
       },
       role: 'option',
-      selected:
-        arr[0]?.props?.value === undefined || arr[0]?.props?.disabled === true
-          ? isFirstSelectableElement()
-          : selected,
+      selected,
       value: undefined, // The value is most likely not a valid HTML attribute.
       'data-value': child.props.value, // Instead, we provide it as a data attribute.
     });