fix: navs have the wrong role, as well as tabs navigation visual glit…

…ch (#4372)

fixes #4371

src/AbstractNav.js

@@ -1,10 +1,10 @@
-import React, { useEffect, useState, useRef, useContext } from 'react';
 import qsa from 'dom-helpers/query/querySelectorAll';
 import PropTypes from 'prop-types';
+import React, { useContext, useEffect, useRef } from 'react';
+import useForceUpdate from '@restart/hooks/useForceUpdate';
 import useMergedRefs from '@restart/hooks/useMergedRefs';
-
-import SelectableContext, { makeEventKey } from './SelectableContext';
 import NavContext from './NavContext';
+import SelectableContext, { makeEventKey } from './SelectableContext';
 import TabContext from './TabContext';
 
 const noop = () => {};
@@ -28,10 +28,6 @@ const propTypes = {
   activeKey: PropTypes.any,
 };
 
-const defaultProps = {
-  role: 'tablist',
-};
-
 const AbstractNav = React.forwardRef(
   (
     {
@@ -45,19 +41,23 @@ const AbstractNav = React.forwardRef(
     },
     ref,
   ) => {
+    // A ref and forceUpdate for refocus, b/c we only want to trigger when needed
+    // and don't want to reset the set in the effect
+    const forceUpdate = useForceUpdate();
+    const needsRefocusRef = useRef(false);
+
     const parentOnSelect = useContext(SelectableContext);
     const tabContext = useContext(TabContext);
 
     let getControlledId, getControllerId;
 
     if (tabContext) {
+      role = role || 'tablist';
       activeKey = tabContext.activeKey;
       getControlledId = tabContext.getControlledId;
       getControllerId = tabContext.getControllerId;
     }
 
-    const [needsRefocus, setRefocus] = useState(false);
-
     const listNode = useRef(null);
 
     const getNextActiveChild = offset => {
@@ -101,18 +101,21 @@ const AbstractNav = React.forwardRef(
 
       event.preventDefault();
       handleSelect(nextActiveChild.dataset.rbEventKey, event);
-      setRefocus(true);
+      needsRefocusRef.current = true;
+      forceUpdate();
     };
 
     useEffect(() => {
-      if (listNode.current && needsRefocus) {
+      if (listNode.current && needsRefocusRef.current) {
         let activeChild = listNode.current.querySelector(
           '[data-rb-event-key].active',
         );
 
         if (activeChild) activeChild.focus();
       }
-    }, [listNode, needsRefocus]);
+
+      needsRefocusRef.current = false;
+    });
 
     const mergedRef = useMergedRefs(ref, listNode);
 
@@ -139,6 +142,5 @@ const AbstractNav = React.forwardRef(
 );
 
 AbstractNav.propTypes = propTypes;
-AbstractNav.defaultProps = defaultProps;
 
 export default AbstractNav;

test/NavSpec.js

@@ -1,9 +1,7 @@
-import React from 'react';
 import { mount } from 'enzyme';
-
+import React from 'react';
 import Nav from '../src/Nav';
 import Navbar from '../src/Navbar';
-
 import { shouldWarn } from './helpers';
 
 describe('<Nav>', () => {
@@ -211,12 +209,17 @@ describe('<Nav>', () => {
   describe('Web Accessibility', () => {
     it('Should have tablist and tab roles', () => {
       const wrapper = mount(
-        <Nav role="tablist">
+        <Nav>
           <Nav.Link key={1}>Tab 1 content</Nav.Link>
           <Nav.Link key={2}>Tab 2 content</Nav.Link>
         </Nav>,
       );
 
+      wrapper.assertNone('div.nav[role="tablist"]');
+      wrapper.assertNone('a[role="tab"]');
+
+      wrapper.setProps({ role: 'tablist' });
+
       wrapper.assertSingle('div.nav[role="tablist"]');
       wrapper.find('a[role="tab"]').length.should.equal(2);
     });

www/gatsby-node.js

@@ -17,7 +17,7 @@ exports.onCreateWebpackConfig = function onCreateWebpackConfig({
 }) {
   actions.setWebpackConfig({
     devtool: stage.includes('develop')
-      ? 'cheap-inline-module-source-map'
+      ? 'inline-module-source-map'
       : 'source-map',
     module: {
       rules: [