Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Oct 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/components/nav/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
27 changes: 24 additions & 3 deletions src/components/nav/nav-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}
})
65 changes: 62 additions & 3 deletions src/components/nav/nav-form.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('')
})

Expand All @@ -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()
})
})
9 changes: 2 additions & 7 deletions src/components/nav/nav-text.js
Original file line number Diff line number Diff line change
@@ -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)
}
})
17 changes: 2 additions & 15 deletions src/components/nav/nav-text.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('')
Expand All @@ -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')
Expand Down
13 changes: 7 additions & 6 deletions src/components/navbar/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>`

Expand Down Expand Up @@ -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.
Expand Down