From b03ba401d94599e2ed1bfdab61a9ee91fe7f822f Mon Sep 17 00:00:00 2001 From: Andrei Gheorghiu Date: Wed, 27 Jan 2021 19:36:12 +0000 Subject: [PATCH 1/9] fixes #6326 --- src/components/table/_table.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/table/_table.scss b/src/components/table/_table.scss index dec1e498c0f..f553840d9dc 100644 --- a/src/components/table/_table.scss +++ b/src/components/table/_table.scss @@ -357,6 +357,10 @@ $bv-escaped-characters: (("<", "%3c"), (">", "%3e"), ("#", "%23")); } } } + // Fixes github.com/bootstrap-vue/bootstrap-vue/issues/6326 + .sr-only { + left: 0; + } } // --- Selectable rows --- From e4d752cb6df0b509bc7c085d81ab2463ec02d481 Mon Sep 17 00:00:00 2001 From: Andrei Gheorghiu Date: Sun, 31 Jan 2021 17:32:15 +0000 Subject: [PATCH 2/9] w.i.p. --- src/components/table/_table.scss | 4 ---- src/components/table/helpers/mixin-table-renderer.js | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/table/_table.scss b/src/components/table/_table.scss index f553840d9dc..dec1e498c0f 100644 --- a/src/components/table/_table.scss +++ b/src/components/table/_table.scss @@ -357,10 +357,6 @@ $bv-escaped-characters: (("<", "%3c"), (">", "%3e"), ("#", "%23")); } } } - // Fixes github.com/bootstrap-vue/bootstrap-vue/issues/6326 - .sr-only { - left: 0; - } } // --- Selectable rows --- diff --git a/src/components/table/helpers/mixin-table-renderer.js b/src/components/table/helpers/mixin-table-renderer.js index 0941861b216..70b245fd4c5 100644 --- a/src/components/table/helpers/mixin-table-renderer.js +++ b/src/components/table/helpers/mixin-table-renderer.js @@ -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 || 'grid', ...ariaAttrs, ...selectableTableAttrs } From 1478f74e03d7c68ed7fd5b2718d42006856112e4 Mon Sep 17 00:00:00 2001 From: Andrei Gheorghiu Date: Sun, 31 Jan 2021 20:57:08 +0000 Subject: [PATCH 3/9] use provided role on table or default to role=grid --- src/components/table/table-selectable.spec.js | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/components/table/table-selectable.spec.js b/src/components/table/table-selectable.spec.js index ac07994ddaa..fcf56714fd2 100644 --- a/src/components/table/table-selectable.spec.js +++ b/src/components/table/table-selectable.spec.js @@ -33,7 +33,6 @@ describe('table > row select', () => { expect(wrapper).toBeDefined() await waitNT(wrapper.vm) - expect(wrapper.attributes('role')).toBe('table') expect(wrapper.attributes('aria-multiselectable')).toBeUndefined() expect(wrapper.classes()).not.toContain('b-table-selectable') expect(wrapper.classes()).not.toContain('b-table-selectable-no-click') @@ -52,6 +51,35 @@ describe('table > row select', () => { wrapper.destroy() }) + it('should apply grid role if no role was provided', async () => { + let wrapper = mount(BTable, { + propsData: { + fields: testFields, + items: testItems + } + }) + + expect(wrapper).toBeDefined() + await waitNT(wrapper.vm) + + expect(wrapper.attributes('role')).toBe('grid') + + wrapper.destroy() + + wrapper = mount(BTable, { + propsData: { + fields: testFields, + items: testItems, + role: 'foobar' + } + }) + + await waitNT(wrapper.vm) + expect(wrapper.attributes('role')).toBe('foobar') + + wrapper.destroy() + }) + it('should have tabindex but not aria-selected when not selectable and has row-clicked listener', async () => { const wrapper = mount(BTable, { propsData: { @@ -66,7 +94,7 @@ describe('table > row select', () => { expect(wrapper).toBeDefined() await waitNT(wrapper.vm) - expect(wrapper.attributes('role')).toBe('table') + expect(wrapper.attributes('role')).toBe('grid') expect(wrapper.attributes('aria-multiselectable')).toBeUndefined() expect(wrapper.classes()).not.toContain('b-table-selectable') expect(wrapper.classes()).not.toContain('b-table-selectable-no-click') From bbb8b4ce2571fe00ac702c8920204aac1213bba0 Mon Sep 17 00:00:00 2001 From: Andrei Gheorghiu Date: Sun, 31 Jan 2021 21:34:14 +0000 Subject: [PATCH 4/9] default table role to table when not selectable, grid when selectable --- .../table/helpers/mixin-selectable.js | 2 +- src/components/table/table-selectable.spec.js | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/components/table/helpers/mixin-selectable.js b/src/components/table/helpers/mixin-selectable.js index 82cc580c618..41cd80bc408 100644 --- a/src/components/table/helpers/mixin-selectable.js +++ b/src/components/table/helpers/mixin-selectable.js @@ -80,7 +80,7 @@ export const selectableMixin = Vue.extend({ // since this attribute implies keyboard navigation? 'aria-multiselectable': role === 'grid' ? String(this.selectableIsMultiSelect) : null } - : {} + : { role: this.bvAttrs.role || 'table' } } }, watch: { diff --git a/src/components/table/table-selectable.spec.js b/src/components/table/table-selectable.spec.js index fcf56714fd2..78643e8d622 100644 --- a/src/components/table/table-selectable.spec.js +++ b/src/components/table/table-selectable.spec.js @@ -51,7 +51,7 @@ describe('table > row select', () => { wrapper.destroy() }) - it('should apply grid role if no role was provided', async () => { + it('should apply user role if provided, grid role if multiselectable or table role otherwise', async () => { let wrapper = mount(BTable, { propsData: { fields: testFields, @@ -62,8 +62,7 @@ describe('table > row select', () => { expect(wrapper).toBeDefined() await waitNT(wrapper.vm) - expect(wrapper.attributes('role')).toBe('grid') - + expect(wrapper.attributes('role')).toBe('table') wrapper.destroy() wrapper = mount(BTable, { @@ -75,8 +74,21 @@ describe('table > row select', () => { }) 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() }) @@ -94,7 +106,7 @@ describe('table > row select', () => { expect(wrapper).toBeDefined() await waitNT(wrapper.vm) - expect(wrapper.attributes('role')).toBe('grid') + expect(wrapper.attributes('role')).toBe('table') expect(wrapper.attributes('aria-multiselectable')).toBeUndefined() expect(wrapper.classes()).not.toContain('b-table-selectable') expect(wrapper.classes()).not.toContain('b-table-selectable-no-click') From ab2860877d5d61e7920882e5030843e0e33a9e53 Mon Sep 17 00:00:00 2001 From: Andrei Gheorghiu Date: Sun, 31 Jan 2021 21:45:06 +0000 Subject: [PATCH 5/9] removed unnecessary default --- src/components/table/helpers/mixin-table-renderer.js | 1 - src/components/table/table-selectable.spec.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/table/helpers/mixin-table-renderer.js b/src/components/table/helpers/mixin-table-renderer.js index 70b245fd4c5..1b9f90f2a9a 100644 --- a/src/components/table/helpers/mixin-table-renderer.js +++ b/src/components/table/helpers/mixin-table-renderer.js @@ -135,7 +135,6 @@ export const tableRendererMixin = Vue.extend({ ...this.bvAttrs, // Now we can override any `$attrs` here id: this.safeId(), - role: this.bvAttrs.role || 'grid', ...ariaAttrs, ...selectableTableAttrs } diff --git a/src/components/table/table-selectable.spec.js b/src/components/table/table-selectable.spec.js index 78643e8d622..10cd51c6905 100644 --- a/src/components/table/table-selectable.spec.js +++ b/src/components/table/table-selectable.spec.js @@ -82,7 +82,7 @@ describe('table > row select', () => { propsData: { fields: testFields, items: testItems, - selectable: 'true' + selectable: true } }) From 38788548658b20d2a7ae87225d032d5d6539262c Mon Sep 17 00:00:00 2001 From: Andrei Gheorghiu Date: Sun, 31 Jan 2021 21:49:20 +0000 Subject: [PATCH 6/9] moved defaulting table role from selectable into renderer --- src/components/table/helpers/mixin-selectable.js | 2 +- src/components/table/helpers/mixin-table-renderer.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/table/helpers/mixin-selectable.js b/src/components/table/helpers/mixin-selectable.js index 41cd80bc408..82cc580c618 100644 --- a/src/components/table/helpers/mixin-selectable.js +++ b/src/components/table/helpers/mixin-selectable.js @@ -80,7 +80,7 @@ export const selectableMixin = Vue.extend({ // since this attribute implies keyboard navigation? 'aria-multiselectable': role === 'grid' ? String(this.selectableIsMultiSelect) : null } - : { role: this.bvAttrs.role || 'table' } + : {} } }, watch: { diff --git a/src/components/table/helpers/mixin-table-renderer.js b/src/components/table/helpers/mixin-table-renderer.js index 1b9f90f2a9a..4b0148121b0 100644 --- a/src/components/table/helpers/mixin-table-renderer.js +++ b/src/components/table/helpers/mixin-table-renderer.js @@ -135,6 +135,7 @@ export const tableRendererMixin = Vue.extend({ ...this.bvAttrs, // Now we can override any `$attrs` here id: this.safeId(), + role: this.bvAttrs.role || 'table', ...ariaAttrs, ...selectableTableAttrs } From 786192282fd1cecc7a579c95a000a784e326cdd5 Mon Sep 17 00:00:00 2001 From: Andrei Gheorghiu Date: Sun, 31 Jan 2021 21:51:42 +0000 Subject: [PATCH 7/9] reverted change on test --- src/components/table/table-selectable.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/table/table-selectable.spec.js b/src/components/table/table-selectable.spec.js index 10cd51c6905..d5eb54c5703 100644 --- a/src/components/table/table-selectable.spec.js +++ b/src/components/table/table-selectable.spec.js @@ -33,6 +33,7 @@ describe('table > row select', () => { expect(wrapper).toBeDefined() await waitNT(wrapper.vm) + expect(wrapper.attributes('role')).toBe('table') expect(wrapper.attributes('aria-multiselectable')).toBeUndefined() expect(wrapper.classes()).not.toContain('b-table-selectable') expect(wrapper.classes()).not.toContain('b-table-selectable-no-click') From b7ad047e8b82994da97bd21291b91a26f60898f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Mon, 1 Feb 2021 00:04:05 +0100 Subject: [PATCH 8/9] Update mixin-selectable.js --- .../table/helpers/mixin-selectable.js | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/components/table/helpers/mixin-selectable.js b/src/components/table/helpers/mixin-selectable.js index 82cc580c618..b30806497aa 100644 --- a/src/components/table/helpers/mixin-selectable.js +++ b/src/components/table/helpers/mixin-selectable.js @@ -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 = { @@ -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: { From 83cf46c4dd070128ed24da6508fcb9409a49eac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Mon, 1 Feb 2021 00:04:09 +0100 Subject: [PATCH 9/9] Update mixin-table-renderer.js --- src/components/table/helpers/mixin-table-renderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/table/helpers/mixin-table-renderer.js b/src/components/table/helpers/mixin-table-renderer.js index 4b0148121b0..545da7af61c 100644 --- a/src/components/table/helpers/mixin-table-renderer.js +++ b/src/components/table/helpers/mixin-table-renderer.js @@ -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':