feat: change default Nav components to non ul/li (#3339)

* feat: change default Nav components to none ul/li BREAKING CHANGE: components no longer default to a list * more * fix docs
react-bootstrap · Nov 7, 2018 · 59127e6 · 59127e6
1 parent 9e86838
commit 59127e6
Show file tree

Hide file tree

Showing 12 changed files with 3,116 additions and 19 deletions.
diff --git a/src/Nav.js b/src/Nav.js
@@ -94,7 +94,7 @@ class Nav extends React.Component {
   static defaultProps = {
     justify: false,
     fill: false,
-    as: 'ul',
+    as: 'div',
   };
 
   render() {

diff --git a/src/NavItem.js b/src/NavItem.js
@@ -19,7 +19,7 @@ class NavItem extends React.Component {
   };
 
   static defaultProps = {
-    as: 'li',
+    as: 'div',
   };
 
   render() {

diff --git a/src/TabPane.js b/src/TabPane.js
@@ -17,6 +17,8 @@ class TabPane extends React.Component {
      */
     bsPrefix: PropTypes.string,
 
+    as: elementType,
+
     /**
      * A key that associates the `TabPane` with it's controlling `NavLink`.
      */
@@ -102,14 +104,15 @@ class TabPane extends React.Component {
       mountOnEnter,
       unmountOnExit,
       transition: Transition,
+      as: Component = 'div',
       eventKey: _,
       ...props
     } = this.props;
 
     if (!active && unmountOnExit) return null;
 
     let pane = (
-      <div
+      <Component
         {...props}
         role="tabpanel"
         aria-hidden={!active}

diff --git a/test/NavDropdownSpec.js b/test/NavDropdownSpec.js
@@ -19,7 +19,7 @@ describe('<NavDropdown>', () => {
       </NavDropdown>,
     );
 
-    wrapper.assertSingle('li.dropdown.test-class');
+    wrapper.assertSingle('div.dropdown.test-class');
 
     wrapper
       .assertSingle('a.nav-link')

diff --git a/test/NavItemSpec.js b/test/NavItemSpec.js
@@ -9,7 +9,7 @@ describe('<NavItem>', () => {
       <NavItem className="custom-class">
         <strong>Children</strong>
       </NavItem>,
-    ).assertSingle('li.nav-item.custom-class strong');
+    ).assertSingle('div.nav-item.custom-class strong');
   });
 
   it('should allow custom elements instead of "div"', () => {

diff --git a/test/NavSpec.js b/test/NavSpec.js
@@ -20,7 +20,7 @@ describe('<Nav>', () => {
 
   it('Should set the correct item active', () => {
     const wrapper = mount(
-      <Nav as="div" variant="pills" defaultActiveKey={1}>
+      <Nav variant="pills" defaultActiveKey={1}>
         <Nav.Link eventKey={1}>Pill 1 content</Nav.Link>
         <Nav.Link eventKey={2}>Pill 2 content</Nav.Link>
       </Nav>,
@@ -34,7 +34,7 @@ describe('<Nav>', () => {
 
   it('Should adds variant class', () => {
     mount(
-      <Nav as="div" variant="tabs">
+      <Nav variant="tabs">
         <Nav.Link eventKey={1}>Pill 1 content</Nav.Link>
         <Nav.Link eventKey={2}>Pill 2 content</Nav.Link>
       </Nav>,
@@ -43,7 +43,7 @@ describe('<Nav>', () => {
 
   it('Should adds justified class', () => {
     mount(
-      <Nav justify as="div">
+      <Nav justify>
         <Nav.Link eventKey={1}>Pill 1 content</Nav.Link>
         <Nav.Link eventKey={2}>Pill 2 content</Nav.Link>
       </Nav>,
@@ -52,7 +52,7 @@ describe('<Nav>', () => {
 
   it('Should adds fill class', () => {
     mount(
-      <Nav fill as="div">
+      <Nav fill>
         <Nav.Link eventKey={1}>Pill 1 content</Nav.Link>
         <Nav.Link eventKey={2}>Pill 2 content</Nav.Link>
       </Nav>,
@@ -62,7 +62,7 @@ describe('<Nav>', () => {
   it('Should be navbar aware', () => {
     mount(
       <Navbar>
-        <Nav as="div">
+        <Nav>
           <Nav.Link eventKey={1}>Pill 1 content</Nav.Link>
           <Nav.Link eventKey={2}>Pill 2 content</Nav.Link>
         </Nav>
@@ -77,7 +77,7 @@ describe('<Nav>', () => {
     }
 
     mount(
-      <Nav as="div" onSelect={handleSelect}>
+      <Nav onSelect={handleSelect}>
         <Nav.Link eventKey={1}>Tab 1 content</Nav.Link>
         <Nav.Link eventKey={2}>
           <span>Tab 2 content</span>
@@ -91,7 +91,7 @@ describe('<Nav>', () => {
 
   it('Should set the correct item active by href', () => {
     mount(
-      <Nav as="div" defaultActiveKey="#item1">
+      <Nav defaultActiveKey="#item1">
         <Nav.Link href="#item1" className="test-selected">
           Pill 1 content
         </Nav.Link>
@@ -207,7 +207,7 @@ describe('<Nav>', () => {
   describe('Web Accessibility', () => {
     it('Should have tablist and tab roles', () => {
       const wrapper = mount(
-        <Nav role="tablist" as="div">
+        <Nav role="tablist">
           <Nav.Link key={1}>Tab 1 content</Nav.Link>
           <Nav.Link key={2}>Tab 2 content</Nav.Link>
         </Nav>,

diff --git a/test/NavbarSpec.js b/test/NavbarSpec.js
@@ -47,7 +47,7 @@ describe('<Navbar>', () => {
       <Navbar>
         <Nav />
       </Navbar>,
-    ).assertSingle('ul.navbar-nav');
+    ).assertSingle('div.navbar-nav');
   });
 
   it('Should add custom toggle', () => {
@@ -58,8 +58,8 @@ describe('<Navbar>', () => {
         </Navbar>,
       )
         .assertSingle('p.navbar-toggler')
-        .text(),
-      'hi',
+        .text()
+        .should.equal('hi'),
     );
   });
 

diff --git a/www/src/components/SideNav.js b/www/src/components/SideNav.js
@@ -140,7 +140,7 @@ function attachSearch(ref) {
     window.docsearch({
       apiKey: '00f98b765b687b91399288e7c4c68ce1',
       indexName: 'react_bootstrap_v4',
-      inputSelector: ref,
+      inputSelector: `#${ref.id}`,
       debug: process.env.NODE_ENV !== 'production', // Set debug to true if you want to inspect the dropdown
     });
 }
@@ -188,7 +188,12 @@ class SideNav extends React.Component {
     return (
       <SidePanel {...props}>
         <form className="py-3 d-flex align-items-center">
-          <FormControl type="text" placeholder="Search…" ref={attachSearch} />
+          <FormControl
+            id="docs-search-input"
+            type="text"
+            placeholder="Search…"
+            ref={attachSearch}
+          />
           <MenuButton onClick={this.handleCollapse}>
             <svg
               xmlns="http://www.w3.org/2000/svg"

diff --git a/www/src/examples/Nav/List.js b/www/src/examples/Nav/List.js
@@ -0,0 +1,11 @@
+<Nav defaultActiveKey="/home" as="ul">
+  <Nav.Item as="li">
+    <Nav.Link href="/home">Active</Nav.Link>
+  </Nav.Item>
+  <Nav.Item as="li">
+    <Nav.Link eventKey="link-1">Link</Nav.Link>
+  </Nav.Item>
+  <Nav.Item as="li">
+    <Nav.Link eventKey="link-2">Link</Nav.Link>
+  </Nav.Item>
+</Nav>;
diff --git a/www/src/pages/components/navs.mdx b/www/src/pages/components/navs.mdx
@@ -6,6 +6,7 @@ import ComponentApi from '../../components/ComponentApi';
 import ReactPlayground from '../../components/ReactPlayground';
 import NavAlignement from '../../examples/Nav/Alignment';
 import NavBasic from '../../examples/Nav/Basic';
+import NavList from '../../examples/Nav/List';
 import NavDropdown from '../../examples/Nav/Dropdown';
 import NavDropdownImpl from '../../examples/Nav/DropdownImpl';
 import NavFill from '../../examples/Nav/Fill';
@@ -51,6 +52,18 @@ components.
 
 <ReactPlayground codeText={NavBasic} />
 
+`<Nav>` markup is very flexible and styling is controlled via classes so you can
+use whatever elements you like to build your navs. By default `<Nav>` and `<Nav.Item>` both
+render `<div>`s instead of `<ul>` and `<li>` elements respectively.
+This because it's possible (and common) to leave off the `<Nav.Item>`'s and
+render a `<Nav.Link>` directly, which would create invalid markup by default (`ul > a`).
+
+When a `<ul>` is appropriate you can render one via the `as` prop; be sure to
+also set your items to `<li>` as well!
+
+<ReactPlayground codeText={NavList} />
+
+
 ## Alignment and orientation
 
 You can control the the direction and orientation of the