From d0472b2503e6c80d2269f3fc6a507ced7f12d16c Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Thu, 28 Mar 2019 10:22:27 -0300 Subject: [PATCH 1/6] fix(form-group): don't render aria-labelledby on group when label-for provided (fixes #2933) Only render the `aria-labelledby` attribute on the `form-group` when `label-for` is not provided. Fixes #2933 --- src/components/form-group/form-group.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/form-group/form-group.js b/src/components/form-group/form-group.js index 67a0bc63eca..bf7034a4409 100644 --- a/src/components/form-group/form-group.js +++ b/src/components/form-group/form-group.js @@ -9,6 +9,7 @@ import warn from '../../utils/warn' import { select, selectAll, isVisible, setAttr, removeAttr, getAttr } from '../../utils/dom' import { arrayIncludes } from '../../utils/array' import { keys, create } from '../../utils/object' +import { inBrowser } from '../../utils/env' // Sub components import BFormRow from '../layout/form-row' import BCol from '../layout/col' @@ -379,7 +380,7 @@ export default (resolve, reject) => { setInputDescribedBy(add, remove) { // Sets the `aria-describedby` attribute on the input if label-for is set. // Optionally accepts a string of IDs to remove as the second parameter - if (this.labelFor && typeof document !== 'undefined') { + if (this.labelFor && inBrowser) { const input = select(`#${this.labelFor}`, this.$refs.content) if (input) { const adb = 'aria-describedby' @@ -434,7 +435,7 @@ export default (resolve, reject) => { disabled: isFieldset ? this.disabled : null, role: isFieldset ? null : 'group', 'aria-invalid': this.computedState === false ? 'true' : null, - 'aria-labelledby': this.labelId || null, + 'aria-labelledby': isFieldset ? this.labelId : null, 'aria-describedby': this.describedByIds || null } } From bc4cecf41f6f1775ad98bc8f8a475248e8ba2096 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Thu, 28 Mar 2019 10:45:57 -0300 Subject: [PATCH 2/6] Update form-group.js --- src/components/form-group/form-group.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/components/form-group/form-group.js b/src/components/form-group/form-group.js index bf7034a4409..0bbae981ff3 100644 --- a/src/components/form-group/form-group.js +++ b/src/components/form-group/form-group.js @@ -435,8 +435,12 @@ export default (resolve, reject) => { disabled: isFieldset ? this.disabled : null, role: isFieldset ? null : 'group', 'aria-invalid': this.computedState === false ? 'true' : null, - 'aria-labelledby': isFieldset ? this.labelId : null, - 'aria-describedby': this.describedByIds || null + // Only apply aria-labeledby if we are a horiontal fieldset + // as the legend is no longer a direct child of fieldset + 'aria-labelledby': isFieldset && isHorizontal ? this.labelId : null, + // Only apply aria-describedby IDs if we are a fieldset + // as the input will have the IDs when not a fieldset + 'aria-describedby': isFieldset ? this.describedByIds : null } } // Return it wrapped in a form-group. From bc59626610b928295241c51226169ef880971b08 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Thu, 28 Mar 2019 10:52:39 -0300 Subject: [PATCH 3/6] Update form-group.html --- src/components/form-group/fixtures/form-group.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/form-group/fixtures/form-group.html b/src/components/form-group/fixtures/form-group.html index eb8a392ce48..bafbb070c8b 100644 --- a/src/components/form-group/fixtures/form-group.html +++ b/src/components/form-group/fixtures/form-group.html @@ -8,7 +8,7 @@ :invalid-feedback="text.length ? '' : 'Please enter something'" :valid-feedback="text.length ? 'Thank you' : ''" :state="text.length ? true : false"> - + Date: Thu, 28 Mar 2019 10:54:01 -0300 Subject: [PATCH 4/6] Update form-group.spec.js --- src/components/form-group/form-group.spec.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/form-group/form-group.spec.js b/src/components/form-group/form-group.spec.js index a50ca9341a7..e596ea71a39 100644 --- a/src/components/form-group/form-group.spec.js +++ b/src/components/form-group/form-group.spec.js @@ -7,19 +7,20 @@ describe('form-group', () => { it('app changes validation state when text supplied', async () => { const { app } = window const $group = app.$refs.group1 + const $input = app.$refs.input1 expect($group.$el.getAttribute('aria-invalid')).toBe('true') - const oldADB = $group.$el.getAttribute('aria-describedby') + const oldADB = $input.$el.getAttribute('aria-describedby') await setData(app, 'text', 'foobar doodle') await nextTick() expect($group.$el.getAttribute('aria-invalid')).toBe(null) - const newADB = $group.$el.getAttribute('aria-describedby') + const newADB = $input.$el.getAttribute('aria-describedby') - expect(oldADB).not.toBe(newADB) + expect(oldADB).not.toEqual(newADB) }) describe('form-group > legend click', () => { From dcf0e9d218ba63f0712e734949bd64be96428333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Thu, 28 Mar 2019 17:36:10 +0100 Subject: [PATCH 5/6] Update form-group.js --- src/components/form-group/form-group.js | 70 ++++++++++++++----------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/src/components/form-group/form-group.js b/src/components/form-group/form-group.js index 0bbae981ff3..17dd3dac0d3 100644 --- a/src/components/form-group/form-group.js +++ b/src/components/form-group/form-group.js @@ -20,7 +20,8 @@ import BFormValidFeedback from '../form/form-valid-feedback' // Selector for finding first input in the form-group const SELECTOR = 'input:not(:disabled),textarea:not(:disabled),select:not(:disabled)' -// Memoize this function to return cached values to save time in computed functions +// Memoize this function to return cached values to +// save time in computed functions const makePropName = memoize((breakpoint = '', prefix) => { return `${prefix}${upperFirst(breakpoint)}` }) @@ -28,8 +29,8 @@ const makePropName = memoize((breakpoint = '', prefix) => { const DEPRECATED_MSG = 'Props "horizontal" and "breakpoint" are deprecated. Use "label-cols(-{breakpoint})" props instead.' -// render helper functions (here rather than polluting the instance with more methods) -function renderInvalidFeedback(h, ctx) { +// Render helper functions (here rather than polluting the instance with more methods) +const renderInvalidFeedback = (h, ctx) => { let content = ctx.$slots['invalid-feedback'] || ctx.invalidFeedback let invalidFeedback = h(false) if (content) { @@ -55,7 +56,7 @@ function renderInvalidFeedback(h, ctx) { return invalidFeedback } -function renderValidFeedback(h, ctx) { +const renderValidFeedback = (h, ctx) => { const content = ctx.$slots['valid-feedback'] || ctx.validFeedback let validFeedback = h(false) if (content) { @@ -81,7 +82,7 @@ function renderValidFeedback(h, ctx) { return validFeedback } -function renderHelpText(h, ctx) { +const renderHelpText = (h, ctx) => { // Form help text (description) const content = ctx.$slots['description'] || ctx.description let description = h(false) @@ -100,8 +101,8 @@ function renderHelpText(h, ctx) { return description } -function renderLabel(h, ctx) { - // render label/legend inside b-col if necessary +const renderLabel = (h, ctx) => { + // Render label/legend inside b-col if necessary const content = ctx.$slots['label'] || ctx.label const labelFor = ctx.labelFor const isLegend = !labelFor @@ -133,17 +134,20 @@ function renderLabel(h, ctx) { attrs: { id: ctx.labelId, for: labelFor || null, - // We add a tab index to legend so that screen readers will properly read the aria-labelledby in IE. + // We add a tab index to legend so that screen readers + // will properly read the aria-labelledby in IE. tabindex: isLegend ? '-1' : null }, class: [ - // When horizontal or if a legend is rendered, add col-form-label for correct sizing - // as Bootstrap has inconsitent font styling for legend in non-horiontal form-groups. + // When horizontal or if a legend is rendered, add col-form-label + // for correct sizing as Bootstrap has inconsistent font styling + // for legend in non-horizontal form-groups. // See: https://github.com/twbs/bootstrap/issues/27805 isHorizontal || isLegend ? 'col-form-label' : '', // Emulate label padding top of 0 on legend when not horizontal !isHorizontal && isLegend ? 'pt-0' : '', - // If not horizontal and not a legend, we add d-block to label so that label-align works + // If not horizontal and not a legend, we add d-block to label + // so that label-align works !isHorizontal && !isLegend ? 'd-block' : '', ctx.labelSize ? `col-form-label-${ctx.labelSize}` : '', ctx.labelAlignClasses, @@ -155,10 +159,9 @@ function renderLabel(h, ctx) { } } -// // Async (lazy) component for BFormGroup -// Needed so that the breakpoint specific props can be computed once hte config is created -// +// Needed so that the breakpoint specific props can be computed +// once the config is created export default (resolve, reject) => { // Grab the current config for breakpoints const BREAKPOINTS = getBreakpointsUp() @@ -269,13 +272,14 @@ export default (resolve, reject) => { const bp = this.breakpoint || BREAKPOINTS[1] // 'sm' const cols = parseInt(this.labelCols, 10) || 3 props[bp] = cols > 0 ? cols : 3 - // We then return the single breakpoint prop for legacy compatability + // We then return the single breakpoint prop for legacy compatibility return props } BREAKPOINTS.forEach(breakpoint => { // Grab the value if the label column breakpoint prop let propVal = this[makePropName(breakpoint, 'labelCols')] - // Handle case where the prop's value is an empty string, which represents true + // Handle case where the prop's value is an empty string, + // which represents true propVal = propVal === '' ? true : propVal || false if (typeof propVal !== 'boolean') { // Convert to column size to number @@ -284,8 +288,9 @@ export default (resolve, reject) => { propVal = propVal > 0 ? propVal : false } if (propVal) { - // Add the prop to the list of props to give to b-col. - // if breakpoint is '' (labelCols=true), then we use the col prop to make equal width at xs + // Add the prop to the list of props to give to b-col + // If breakpoint is '' (labelCols=true), then we use the + // col prop to make equal width at xs const bColPropName = breakpoint || (typeof propVal === 'boolean' ? 'col' : 'cols') // Add it to the props props[bColPropName] = propVal @@ -296,7 +301,7 @@ export default (resolve, reject) => { labelAlignClasses() { const classes = [] BREAKPOINTS.forEach(breakpoint => { - // assemble the label column breakpoint align classes + // Assemble the label column breakpoint align classes const propVal = this[makePropName(breakpoint, 'labelAlign')] || null if (propVal) { const className = breakpoint ? `text-${breakpoint}-${propVal}` : `text-${propVal}` @@ -319,7 +324,7 @@ export default (resolve, reject) => { : null }, hasInvalidFeedback() { - // used for computing aria-describedby + // Used for computing aria-describedby const $slots = this.$slots return this.computedState === false && ($slots['invalid-feedback'] || this.invalidFeedback) }, @@ -327,7 +332,7 @@ export default (resolve, reject) => { return this.hasInvalidFeedback ? this.safeId('_BV_feedback_invalid_') : null }, hasValidFeedback() { - // used for computing aria-describedby + // Used for computing aria-describedby return this.computedState === true && (this.$slots['valid-feedback'] || this.validFeedback) }, validFeedbackId() { @@ -335,7 +340,7 @@ export default (resolve, reject) => { }, describedByIds() { // Screen readers will read out any content linked to by aria-describedby - // even if the content is hidden with 'display: none', hence we only include + // even if the content is hidden with `display: none;`, hence we only include // feedback IDs if the form-group's state is explicitly valid or invalid. return ( [this.descriptionId, this.invalidFeedbackId, this.validFeedbackId] @@ -353,7 +358,7 @@ export default (resolve, reject) => { }, mounted() { this.$nextTick(() => { - // Set the adia-describedby IDs on the input specified by label-for + // Set the aria-describedby IDs on the input specified by label-for // We do this in a nextTick to ensure the children have finished rendering this.setInputDescribedBy(this.describedByIds) }) @@ -361,13 +366,14 @@ export default (resolve, reject) => { methods: { legendClick(evt) { if (this.labelFor) { - // don't do anything if labelFor is set + // Don't do anything if labelFor is set /* istanbul ignore next: clicking a label will focus the input, so no need to test */ return } const tagName = evt.target ? evt.target.tagName : '' if (/^(input|select|textarea|label|button|a)$/i.test(tagName)) { - // If clicked an interactive element inside legend, we just let the default happen + // If clicked an interactive element inside legend, + // we just let the default happen /* istanbul ignore next */ return } @@ -378,7 +384,7 @@ export default (resolve, reject) => { } }, setInputDescribedBy(add, remove) { - // Sets the `aria-describedby` attribute on the input if label-for is set. + // Sets the `aria-describedby` attribute on the input if label-for is set // Optionally accepts a string of IDs to remove as the second parameter if (this.labelFor && inBrowser) { const input = select(`#${this.labelFor}`, this.$refs.content) @@ -435,7 +441,7 @@ export default (resolve, reject) => { disabled: isFieldset ? this.disabled : null, role: isFieldset ? null : 'group', 'aria-invalid': this.computedState === false ? 'true' : null, - // Only apply aria-labeledby if we are a horiontal fieldset + // Only apply aria-labelledby if we are a horizontal fieldset // as the legend is no longer a direct child of fieldset 'aria-labelledby': isFieldset && isHorizontal ? this.labelId : null, // Only apply aria-describedby IDs if we are a fieldset @@ -443,10 +449,10 @@ export default (resolve, reject) => { 'aria-describedby': isFieldset ? this.describedByIds : null } } - // Return it wrapped in a form-group. - // Note: fieldsets do not support adding `row` or `form-row` directly to them - // due to browser specific render issues, so we move the form-row to an - // inner wrapper div when horizontal and using a fieldset + // Return it wrapped in a form-group + // Note: Fieldsets do not support adding `row` or `form-row` directly + // to them due to browser specific render issues, so we move the `form-row` + // to an inner wrapper div when horizontal and using a fieldset return h( isFieldset ? 'fieldset' : isHorizontal ? 'b-form-row' : 'div', data, @@ -455,6 +461,6 @@ export default (resolve, reject) => { } } - // Return the componwent options reference + // Return the component options reference resolve(BFormGroup) } From d24f8a5ae035517b74c32eda4c97d268fadc9d69 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Fri, 29 Mar 2019 00:12:56 -0300 Subject: [PATCH 6/6] Update form-group.js --- src/components/form-group/form-group.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/form-group/form-group.js b/src/components/form-group/form-group.js index 17dd3dac0d3..4d516a63b84 100644 --- a/src/components/form-group/form-group.js +++ b/src/components/form-group/form-group.js @@ -384,8 +384,9 @@ export default (resolve, reject) => { } }, setInputDescribedBy(add, remove) { - // Sets the `aria-describedby` attribute on the input if label-for is set - // Optionally accepts a string of IDs to remove as the second parameter + // Sets the `aria-describedby` attribute on the input if label-for is set. + // Optionally accepts a string of IDs to remove as the second parameter. + // Preserves any aria-describedby value(s) user may have on input. if (this.labelFor && inBrowser) { const input = select(`#${this.labelFor}`, this.$refs.content) if (input) { @@ -420,9 +421,7 @@ export default (resolve, reject) => { ref: 'content', attrs: { tabindex: isFieldset ? '-1' : null, - role: isFieldset ? 'group' : null, - 'aria-labelledby': isFieldset ? this.labelId : null, - 'aria-describedby': isFieldset ? this.ariaDescribedBy : null + role: isFieldset ? 'group' : null } }, [