fix(b-nav-form, b-nav-text): ensure these sub-components have <li> as root element for accessibility (#4100)

tmorehouse · web-flow · commit 677480035f34 · 2019-10-11T18:31:35.000-03:00
diff --git a/src/components/nav/README.md b/src/components/nav/README.md
@@ -23,6 +23,13 @@ types of navigation components. It includes some style overrides (for working wi
 padding for larger hit areas, and basic disabled styling. No active states are included in the base
 nav.
 
+`<b-nav>` supports teh following child components:
+
+- `<b-nav-item>` for actionable links (or router-links)
+- `<b-nav-item-dropdown>` for dropdowns
+- `<b-nav-text>` for plain text content
+- `<b-nav-form>` for inline forms
+
 ## Link appearance
 
 Two style variations are supported: `tabs` and `pills`, which support `active` state styling. These
@@ -215,6 +222,44 @@ shown. When there are a large number of dropdowns rendered on the same page, per
 impacted due to larger overall memory utilization. You can instruct `<b-nav-item-dropdown>` to
 render the menu contents only when it is shown by setting the `lazy` prop to true.
 
+## Nav text content
+
+Use the `<b-nav-text>` child component to place plain text content into the nav:
+
+```html
+<div>
+  <b-nav >
+    <b-nav-item href="#1">Link 1</b-nav-item>
+    <b-nav-item href="#2">Link 2</b-nav-item>
+    <b-nav-text>Plain text</b-nav-text>
+  </b-nav>
+</div>
+
+<!-- b-nav-text.vue -->
+```
+
+## Nav inline forms
+
+Use the `<b-nav-form>` child component to place an _inline_ form into the nav:
+
+```html
+<div>
+  <b-nav pills>
+    <b-nav-item href="#1" active>Link 1</b-nav-item>
+    <b-nav-item href="#2">Link 2</b-nav-item>
+    <b-nav-form @submit.stop.prevent="alert('Form Submitted')">
+      <b-form-input aria-label="Input" class="mr-1"></b-form-input>
+      <b-button type="submit">Ok</b-button>
+    </b-nav-form>
+  </b-nav>
+</div>
+
+<!-- b-nav-form.vue -->
+```
+
+Refer to the [`<b-form>` inline](/docs/components/form#inline-form) documentation for additional
+details on placing form controls.
+
 ## Tabbed local content support
 
 See the [`<b-tabs>`](/docs/components/tabs) component for creating tabbable panes of local content
diff --git a/src/components/nav/nav-form.js b/src/components/nav/nav-form.js
@@ -3,14 +3,35 @@ import { mergeData } from 'vue-functional-data-merge'
 import { omit } from '../../utils/object'
 import { BForm, props as BFormProps } from '../form/form'
 
-export const props = omit(BFormProps, ['inline'])
+export const props = {
+  ...omit(BFormProps, ['inline']),
+  formClass: {
+    type: [String, Array, Object],
+    default: null
+  }
+}
 
 // @vue/component
 export const BNavForm = /*#__PURE__*/ Vue.extend({
   name: 'BNavForm',
   functional: true,
   props,
-  render(h, { props, data, children }) {
-    return h(BForm, mergeData(data, { props: { ...props, inline: true } }), children)
+  render(h, { props, data, children, listeners = {} }) {
+    const attrs = data.attrs
+    // The following data properties are cleared out
+    // as they will be passed to BForm directly
+    data.attrs = {}
+    data.on = {}
+    const $form = h(
+      BForm,
+      {
+        class: props.formClass,
+        props: { ...props, inline: true },
+        attrs,
+        on: listeners
+      },
+      children
+    )
+    return h('li', mergeData(data, { staticClass: 'form-inline' }), [$form])
   }
 })
diff --git a/src/components/nav/nav-form.spec.js b/src/components/nav/nav-form.spec.js
@@ -5,9 +5,14 @@ describe('nav > nav-form', () => {
   it('has expected default structure', async () => {
     const wrapper = mount(BNavForm)
 
-    expect(wrapper.is('form')).toBe(true)
+    expect(wrapper.is('li')).toBe(true)
     expect(wrapper.classes()).toContain('form-inline')
     expect(wrapper.classes().length).toBe(1)
+
+    const $form = wrapper.find('form')
+    expect($form.exists()).toBe(true)
+    expect($form.classes()).toContain('form-inline')
+    expect($form.classes().length).toBe(1)
     expect(wrapper.text()).toEqual('')
   })
 
@@ -18,9 +23,63 @@ describe('nav > nav-form', () => {
       }
     })
 
-    expect(wrapper.is('form')).toBe(true)
+    expect(wrapper.is('li')).toBe(true)
+    expect(wrapper.classes()).toContain('form-inline')
+    expect(wrapper.classes().length).toBe(1)
+
+    const $form = wrapper.find('form')
+    expect($form.exists()).toBe(true)
+    expect($form.classes()).toContain('form-inline')
+    expect($form.text()).toEqual('foobar')
+  })
+
+  it('applies ID to form when prop ID is set', async () => {
+    const wrapper = mount(BNavForm, {
+      propsData: {
+        id: 'baz'
+      },
+      slots: {
+        default: 'foobar'
+      }
+    })
+
+    expect(wrapper.is('li')).toBe(true)
+    expect(wrapper.classes()).toContain('form-inline')
+    expect(wrapper.classes().length).toBe(1)
+
+    const $form = wrapper.find('form')
+    expect($form.exists()).toBe(true)
+    expect($form.classes()).toContain('form-inline')
+    expect($form.text()).toEqual('foobar')
+    expect($form.attributes('id')).toEqual('baz')
+  })
+
+  it('listeners are bound to form element', async () => {
+    const onSubmit = jest.fn()
+    const wrapper = mount(BNavForm, {
+      propsData: {
+        id: 'baz'
+      },
+      listeners: {
+        submit: onSubmit
+      },
+      slots: {
+        default: 'foobar'
+      }
+    })
+
+    expect(wrapper.is('li')).toBe(true)
     expect(wrapper.classes()).toContain('form-inline')
     expect(wrapper.classes().length).toBe(1)
-    expect(wrapper.text()).toEqual('foobar')
+
+    const $form = wrapper.find('form')
+    expect($form.exists()).toBe(true)
+    expect($form.classes()).toContain('form-inline')
+    expect($form.text()).toEqual('foobar')
+
+    expect(onSubmit).not.toHaveBeenCalled()
+
+    $form.trigger('submit')
+    expect(onSubmit).toHaveBeenCalled()
   })
 })
diff --git a/src/components/nav/nav-text.js b/src/components/nav/nav-text.js
@@ -1,19 +1,14 @@
 import Vue from '../../utils/vue'
 import { mergeData } from 'vue-functional-data-merge'
 
-export const props = {
-  tag: {
-    type: String,
-    default: 'span'
-  }
-}
+export const props = {}
 
 // @vue/component
 export const BNavText = /*#__PURE__*/ Vue.extend({
   name: 'BNavText',
   functional: true,
   props,
   render(h, { props, data, children }) {
-    return h(props.tag, mergeData(data, { staticClass: 'navbar-text' }), children)
+    return h('li', mergeData(data, { staticClass: 'navbar-text' }), children)
   }
 })
diff --git a/src/components/nav/nav-text.spec.js b/src/components/nav/nav-text.spec.js
@@ -5,20 +5,7 @@ describe('nav > nav-text', () => {
   it('has expected default structure', async () => {
     const wrapper = mount(BNavText)
 
-    expect(wrapper.is('span')).toBe(true)
-    expect(wrapper.classes()).toContain('navbar-text')
-    expect(wrapper.classes().length).toBe(1)
-    expect(wrapper.text()).toEqual('')
-  })
-
-  it('renders custom root element when prop tag is set', async () => {
-    const wrapper = mount(BNavText, {
-      propsData: {
-        tag: 'div'
-      }
-    })
-
-    expect(wrapper.is('div')).toBe(true)
+    expect(wrapper.is('li')).toBe(true)
     expect(wrapper.classes()).toContain('navbar-text')
     expect(wrapper.classes().length).toBe(1)
     expect(wrapper.text()).toEqual('')
@@ -31,7 +18,7 @@ describe('nav > nav-text', () => {
       }
     })
 
-    expect(wrapper.is('span')).toBe(true)
+    expect(wrapper.is('li')).toBe(true)
     expect(wrapper.classes()).toContain('navbar-text')
     expect(wrapper.classes().length).toBe(1)
     expect(wrapper.text()).toEqual('foobar')
diff --git a/src/components/navbar/README.md b/src/components/navbar/README.md
@@ -80,13 +80,14 @@ Navbars come with built-in support for a handful of sub-components. Choose from
 needed:
 
 - `<b-navbar-brand>` for your company, product, or project name.
-- `<b-navbar-nav>` for a full-height and lightweight navigation (including support for dropdowns).
-- `<b-nav-item>` for link (and router-link) action items
-- `<b-nav-item-dropdown>` for navbar dropdown menus
-- `<b-nav-text>` for adding vertically centered strings of text.
-- `<b-nav-form>` for any form controls and actions.
 - `<b-navbar-toggle>` for use with the `<b-collapse is-nav>` component.
 - `<b-collapse is-nav>` for grouping and hiding navbar contents by a parent breakpoint.
+- `<b-navbar-nav>` for a full-height and lightweight navigation (including support for dropdowns).
+  The following sub-components inside `<b-navbar-nav>` are supported:
+  - `<b-nav-item>` for link (and router-link) action items
+  - `<b-nav-item-dropdown>` for nav dropdown menus
+  - `<b-nav-text>` for adding vertically centered strings of text.
+  - `<b-nav-form>` for any form controls and actions.
 
 ### `<b-navbar-brand>`
 
@@ -153,7 +154,7 @@ Navbar navigation links build on the `<b-navbar-nav>` parent component and requi
 navbars will also grow to occupy as much horizontal space as possible to keep your navbar contents
 securely aligned.
 
-`<b-navbar-nav>` supports the following components:
+`<b-navbar-nav>` supports the following child components:
 
 - `<b-nav-item>` for link (and router-link) action items
 - `<b-nav-text>` for adding vertically centered strings of text.
