[joy-ui][Accordion] Fix AccordionDetails tabindex when expanding (#43246

)

Signed-off-by: Shrey Bhadja <[email protected]>
Co-authored-by: Aarón García Hervás <[email protected]>

packages/mui-joy/src/AccordionDetails/AccordionDetails.test.tsx

@@ -85,5 +85,51 @@ describe('<AccordionDetails />', () => {
       expect(screen.getByRole('button')).to.have.attribute('aria-expanded', 'true');
       expect(screen.getByTestId('textbox')).to.have.property('tabIndex', 2);
     });
+
+    it('should retain the default tab index if not explicitly set', () => {
+      render(
+        <Accordion>
+          <AccordionSummary>title</AccordionSummary>
+          <AccordionDetails>
+            <input data-testid="textbox" />
+          </AccordionDetails>
+        </Accordion>,
+      );
+
+      expect(screen.getByTestId('textbox')).to.have.property('tabIndex', -1);
+
+      fireEvent.click(screen.getByRole('button')); // open
+
+      expect(screen.getByTestId('textbox')).to.have.property('tabIndex', 0);
+
+      fireEvent.click(screen.getByRole('button')); // close
+
+      expect(screen.getByTestId('textbox')).to.have.property('tabIndex', -1);
+
+      fireEvent.click(screen.getByRole('button')); // reopen
+
+      expect(screen.getByTestId('textbox')).to.have.property('tabIndex', 0);
+    });
+
+    it('should retain the -1 tab index when explicitly set', () => {
+      render(
+        <Accordion>
+          <AccordionSummary>title</AccordionSummary>
+          <AccordionDetails>
+            <input data-testid="textbox" tabIndex={-1} />
+          </AccordionDetails>
+        </Accordion>,
+      );
+
+      expect(screen.getByTestId('textbox')).to.have.property('tabIndex', -1);
+
+      fireEvent.click(screen.getByRole('button')); // open
+
+      expect(screen.getByTestId('textbox')).to.have.property('tabIndex', -1);
+
+      fireEvent.click(screen.getByRole('button')); // close
+
+      expect(screen.getByTestId('textbox')).to.have.property('tabIndex', -1);
+    });
   });
 });

packages/mui-joy/src/AccordionDetails/AccordionDetails.tsx

@@ -98,27 +98,28 @@ const AccordionDetails = React.forwardRef(function AccordionDetails(inProps, ref
   const handleRef = useForkRef(rootRef, ref);
 
   React.useEffect(() => {
-    // When accordion is closed, prevent tabbing into the details content.
     if (rootRef.current) {
       const elements = rootRef.current.querySelectorAll(
         'a, button, input, textarea, select, details, [tabindex]:not([tabindex="-1"])',
       );
-      elements.forEach((elm) => {
-        if (expanded) {
-          const prevTabIndex = elm.getAttribute('data-prev-tabindex');
-          const currentTabIndex = elm.getAttribute('tabindex');
 
-          if (currentTabIndex && prevTabIndex) {
-            // restore tabindex
-            elm.setAttribute('tabindex', prevTabIndex);
-            elm.removeAttribute('data-prev-tabindex');
-          }
+      elements.forEach((elm) => {
+        const currentTabIndex = elm.getAttribute('tabindex');
+        const prevTabIndex = elm.getAttribute('data-prev-tabindex');
 
-          if (!prevTabIndex && !currentTabIndex) {
+        if (expanded) {
+          // Restore the previous tabindex if it exists, or remove it if it was "unset"
+          if (prevTabIndex === 'unset') {
             elm.removeAttribute('tabindex');
+          } else if (prevTabIndex !== null) {
+            elm.setAttribute('tabindex', prevTabIndex);
           }
+          elm.removeAttribute('data-prev-tabindex');
         } else {
-          elm.setAttribute('data-prev-tabindex', elm.getAttribute('tabindex') || '');
+          // If element has no data-prev-tabindex, store the current tabindex or "unset"
+          if (prevTabIndex === null) {
+            elm.setAttribute('data-prev-tabindex', currentTabIndex || 'unset');
+          }
           elm.setAttribute('tabindex', '-1');
         }
       });