Skip to content

fix(b-table): default role to grid when selectable and table otherwise #6383

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 11 commits into from
Jan 31, 2021
Merged
25 changes: 15 additions & 10 deletions src/components/table/helpers/mixin-selectable.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ import { isArray, isNumber } from '../../../utils/inspect'
import { looseEqual } from '../../../utils/loose-equal'
import { mathMax, mathMin } from '../../../utils/math'
import { makeProp } from '../../../utils/props'
import { toString } from '../../../utils/string'
import { sanitizeRow } from './sanitize-row'

// --- Constants ---

const SELECT_MODES = ['range', 'multi', 'single']

const ROLE_GRID = 'grid'

// --- Props ---

export const props = {
Expand Down Expand Up @@ -70,17 +73,19 @@ export const selectableMixin = Vue.extend({
}
},
selectableTableAttrs() {
const role = this.bvAttrs.role || 'grid'
if (!this.isSelectable) {
return {}
}

return this.isSelectable
? {
role,
// TODO:
// Should this attribute not be included when `no-select-on-click` is set
// since this attribute implies keyboard navigation?
'aria-multiselectable': role === 'grid' ? String(this.selectableIsMultiSelect) : null
}
: {}
const role = this.bvAttrs.role || ROLE_GRID

return {
role,
// TODO:
// Should this attribute not be included when `no-select-on-click` is set
// since this attribute implies keyboard navigation?
'aria-multiselectable': role === ROLE_GRID ? toString(this.selectableIsMultiSelect) : null
}
}
},
watch: {
Expand Down
4 changes: 2 additions & 2 deletions src/components/table/helpers/mixin-table-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export const tableRendererMixin = Vue.extend({
const ariaAttrs = this.isTableSimple
? {}
: {
'aria-busy': this.computedBusy ? 'true' : 'false',
'aria-busy': toString(this.computedBusy),
'aria-colcount': toString(fields.length),
// Preserve user supplied `aria-describedby`, if provided
'aria-describedby':
Expand All @@ -135,7 +135,7 @@ export const tableRendererMixin = Vue.extend({
...this.bvAttrs,
// Now we can override any `$attrs` here
id: this.safeId(),
role: 'table',
role: this.bvAttrs.role || 'table',
...ariaAttrs,
...selectableTableAttrs
}
Expand Down
41 changes: 41 additions & 0 deletions src/components/table/table-selectable.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,47 @@ describe('table > row select', () => {
wrapper.destroy()
})

it('should apply user role if provided, grid role if multiselectable or table role otherwise', async () => {
let wrapper = mount(BTable, {
propsData: {
fields: testFields,
items: testItems
}
})

expect(wrapper).toBeDefined()
await waitNT(wrapper.vm)

expect(wrapper.attributes('role')).toBe('table')
wrapper.destroy()

wrapper = mount(BTable, {
propsData: {
fields: testFields,
items: testItems,
role: 'foobar'
}
})

await waitNT(wrapper.vm)

expect(wrapper.attributes('role')).toBe('foobar')
wrapper.destroy()

wrapper = mount(BTable, {
propsData: {
fields: testFields,
items: testItems,
selectable: true
}
})

await waitNT(wrapper.vm)

expect(wrapper.attributes('role')).toBe('grid')
wrapper.destroy()
})

it('should have tabindex but not aria-selected when not selectable and has row-clicked listener', async () => {
const wrapper = mount(BTable, {
propsData: {
Expand Down