[Tabs] Improve focus management on list with no active tabs

[alexmotoc]

Closes #21854

New behaviour: Using the TAB key it is now possible to focus on the first tab of a Tab List with no active tabs.

This improves accessibility through easier keyboard navigation.

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against 0de77f5

[oliviertassinari]

Could you add a test case?

[alexmotoc]

I've been working on a test case and it seems to me like the key press is not being simulated correctly or I am not doing it properly. The focused element is the body and not the first tab for some reason. Focus is pulled correctly when testing manually using the docs. Any thoughts on this?

diff --git a/packages/material-ui/src/Tabs/Tabs.test.js b/packages/material-ui/src/Tabs/Tabs.test.js
index 11d417268..6c82d79ad 100644
--- a/packages/material-ui/src/Tabs/Tabs.test.js
+++ b/packages/material-ui/src/Tabs/Tabs.test.js
@@ -1066,5 +1066,26 @@ describe('<Tabs />', () => {
         });
       });
     });
+
+    it('moves focus to the first tab when there are no active tabs', () => {
+      const handleChange = spy();
+      const handleKeyDown = spy((event) => event.defaultPrevented);
+      const { getAllByRole } = render(
+        <Tabs
+          onChange={handleChange}
+          onKeyDown={handleKeyDown}
+          value={false}
+        >
+          <Tab />
+          <Tab />
+          <Tab />
+        </Tabs>
+      );
+      const [firstTab] = getAllByRole('tab');
+
+      fireEvent.keyDown(document.body, { key: 'Tab', keyCode: 9 });
+
+      expect(firstTab).toHaveFocus();
+    });
   });
 });```

[oliviertassinari]

@alexmotoc The tab behavior can't be tested. It doesn't work in jsdom, cypress, nor puppeteer (nowhere I'm aware of). In #21857, we have looked a bit into the issue to introduce a custom version of https://github.com/testing-library/user-event#tabshift-focustrap. For now, testing the tab-index attribute should be more than enough.

[alexmotoc]

@oliviertassinari Thanks for the clarification. Would this be sufficient then? I can go ahead and commit it if it looks good :)

diff --git a/packages/material-ui/src/Tabs/Tabs.test.js b/packages/material-ui/src/Tabs/Tabs.test.js
index 11d417268..36b82843d 100644
--- a/packages/material-ui/src/Tabs/Tabs.test.js
+++ b/packages/material-ui/src/Tabs/Tabs.test.js
@@ -1066,5 +1066,20 @@ describe('<Tabs />', () => {
         });
       });
     });
+
+    it('moves focus to the first tab when there are no active tabs', () => {
+      const { getAllByRole } = render(
+        <Tabs
+          value={false}
+        >
+          <Tab />
+          <Tab />
+          <Tab />
+        </Tabs>
+      );
+      const [firstTab] = getAllByRole('tab');
+
+      expect(firstTab.tabIndex).to.equal(0);
+    });
   });
 });

[oliviertassinari]

@alexmotoc I think that it can be improved (testing all the tab), but it's a good start.

[alexmotoc]

@oliviertassinari Ahhhh I get what you mean. Reused this solution from another test - it's cleaner and tests all the Tab elements.

expect(getAllByRole('tab').map((tab) => tab.tabIndex)).to.have.ordered.members([0, -1]);

[oliviertassinari]

?

expect(getAllByRole('tab').map((tab) => tab.getAttribute('tabIndex'))).to.deep.equal([0, -1]);

[eps1lon]

@alexmotoc Thanks!

[geekingreen]

Any chance we could have this added to v4 as well?