From 92a5cd1e3d75237049e03754550d9fe56eb9bc8a Mon Sep 17 00:00:00 2001 From: gerjanvangeest Date: Mon, 5 Feb 2024 13:39:03 +0100 Subject: [PATCH] fix(form-core): remove fieldset label from input-field aria-labelledby, add opt-in via data-label --- .changeset/slow-ties-carry.md | 6 +++ .../src/form-group/FormGroupMixin.js | 11 ++--- .../form-group/FormGroupMixin-input.suite.js | 47 ------------------- .../form-group/FormGroupMixin.suite.js | 24 ++++++++-- 4 files changed, 32 insertions(+), 56 deletions(-) create mode 100644 .changeset/slow-ties-carry.md diff --git a/.changeset/slow-ties-carry.md b/.changeset/slow-ties-carry.md new file mode 100644 index 000000000..95330d0bf --- /dev/null +++ b/.changeset/slow-ties-carry.md @@ -0,0 +1,6 @@ +--- +'@lion/ui': patch +--- + +[form-core] remove fieldset label from input-field aria-labelledby +[form-core] remove fieldset help-text from input-field aria-describedby diff --git a/packages/ui/components/form-core/src/form-group/FormGroupMixin.js b/packages/ui/components/form-core/src/form-group/FormGroupMixin.js index e9dcc0015..334c222a2 100644 --- a/packages/ui/components/form-core/src/form-group/FormGroupMixin.js +++ b/packages/ui/components/form-core/src/form-group/FormGroupMixin.js @@ -46,7 +46,7 @@ const FormGroupMixinImplementation = superclass => } /** - * The host element with role group (or radigroup or form) containing neccessary aria attributes + * The host element with role group (or radio-group or form) containing necessary aria attributes * @protected */ get _inputNode() { @@ -478,7 +478,7 @@ const FormGroupMixinImplementation = superclass => /** * Traverses the _parentFormGroup tree, and gathers all aria description elements - * (feedback and helptext) that should be provided to children. + * (feedback) that should be provided to children. * * In the example below, when the input for 'street' has focus, a screenreader user * would hear the #group-error. @@ -503,7 +503,9 @@ const FormGroupMixinImplementation = superclass => const descriptionElements = parent._getAriaDescriptionElements(); const orderedEls = getAriaElementsInRightDomOrder(descriptionElements, { reverse: true }); orderedEls.forEach(el => { - this.__descriptionElementsInParentChain.add(el); + if (el.getAttribute('slot') === 'feedback') { + this.__descriptionElementsInParentChain.add(el); + } }); // Also check if the newly added child needs to refer grandparents parent = parent._parentFormGroup; @@ -549,9 +551,6 @@ const FormGroupMixinImplementation = superclass => this.__linkParentMessages(child); this.validate({ clearCurrentResult: true }); - if (typeof child.addToAriaLabelledBy === 'function' && this._labelNode) { - child.addToAriaLabelledBy(this._labelNode, { reorder: false }); - } if (!child.modelValue) { const pVals = this.__pendingValues; if (pVals.modelValue && pVals.modelValue[child.name]) { diff --git a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin-input.suite.js b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin-input.suite.js index c96a72593..6e8fa40b2 100644 --- a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin-input.suite.js +++ b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin-input.suite.js @@ -1,7 +1,6 @@ import { LitElement } from 'lit'; import '@lion/ui/define/lion-field.js'; import '@lion/ui/define/lion-validation-feedback.js'; -import { getFormControlMembers } from '@lion/ui/form-core-test-helpers.js'; import { LionInput } from '@lion/ui/input.js'; import { localizeTearDown } from '@lion/ui/localize-test-helpers.js'; import { defineCE, expect, fixture, html, unsafeStatic } from '@open-wc/testing'; @@ -63,41 +62,6 @@ export function runFormGroupMixinInputSuite(cfg = {}) { 'custom[]': ['custom 1', ''], }); }); - - it('suffixes child labels with group label, just like in
', async () => { - const el = /** @type {FormGroup} */ ( - await fixture(html` - <${tag} label="set"> - <${childTag} name="A" label="fieldA"> - <${childTag} name="B" label="fieldB"> - - `) - ); - const { _labelNode } = getFormControlMembers(el); - - /** - * @param {LionInput} formControl - */ - function getLabels(formControl) { - const control = getFormControlMembers(formControl); - - return /** @type {string} */ (control._inputNode.getAttribute('aria-labelledby')).split( - ' ', - ); - } - const field1 = el.formElements[0]; - const field2 = el.formElements[1]; - - expect(getLabels(field1)).to.eql([field1._labelNode.id, _labelNode.id]); - expect(getLabels(field2)).to.eql([field2._labelNode.id, _labelNode.id]); - - // Test the cleanup on disconnected - el.removeChild(field1); - - // TODO: wait for updated on disconnected to be fixed: https://github.com/lit/lit/issues/1901 - // await field1.updateComplete; - // expect(getLabels(field1)).to.eql([field1._labelNode.id]); - }); }); describe('Screen reader relations (aria-describedby) for child inputs and fieldsets', () => { @@ -307,21 +271,10 @@ export function runFormGroupMixinInputSuite(cfg = {}) { await childAriaTest(await childAriaFixture('feedback')); }); - it(`reads help-text message belonging to fieldset when child input is focused - (via aria-describedby)`, async () => { - await childAriaTest(await childAriaFixture('help-text')); - }); - // TODO: wait for updated on disconnected to be fixed: https://github.com/lit/lit/issues/1901 it.skip(`cleans up feedback message belonging to fieldset on disconnect`, async () => { const el = await childAriaFixture('feedback'); await childAriaTest(el, { cleanupPhase: true }); }); - - // TODO: wait for updated on disconnected to be fixed: https://github.com/lit/lit/issues/1901 - it.skip(`cleans up help-text message belonging to fieldset on disconnect`, async () => { - const el = await childAriaFixture('help-text'); - await childAriaTest(el, { cleanupPhase: true }); - }); }); } diff --git a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js index 02a283d15..934ac5e0e 100644 --- a/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js +++ b/packages/ui/components/form-core/test-suites/form-group/FormGroupMixin.suite.js @@ -1356,6 +1356,24 @@ export function runFormGroupMixinSuite(cfg = {}) { expect(el.hasAttribute('aria-labelledby')).to.equal(true); expect(el.getAttribute('aria-labelledby')).contains(label.id); }); + + it('sets an aria-describedby to its children from element with slot="feedback"', async () => { + const el = /** @type {FormGroup} */ ( + await fixture(html` + <${tag}> + +
My Feedback
+ ${inputSlots} + + `) + ); + const feedback = /** @type {HTMLElement} */ ( + Array.from(el.children).find(child => child.slot === 'feedback') + ); + expect(el.formElements[0]._inputNode.getAttribute('aria-describedby')).contains( + feedback.id, + ); + }); }); describe('Dynamically rendered children', () => { @@ -1395,7 +1413,7 @@ export function runFormGroupMixinSuite(cfg = {}) { const dynamicChildrenTag = unsafeStatic(dynamicChildrenTagString); it(`when rendering children right from the start, sets their values correctly - based on prefilled model/seriazedValue`, async () => { + based on prefilled model/serializedValue`, async () => { const el = /** @type {DynamicCWrapper} */ ( await fixture(html` <${dynamicChildrenTag} @@ -1430,7 +1448,7 @@ export function runFormGroupMixinSuite(cfg = {}) { }); it(`when rendering children delayed, sets their values - correctly based on prefilled model/seriazedValue`, async () => { + correctly based on prefilled model/serializedValue`, async () => { const el = /** @type {DynamicCWrapper} */ ( await fixture(html` <${dynamicChildrenTag} .modelValue="${{ firstName: 'foo', lastName: 'bar' }}"> @@ -1463,7 +1481,7 @@ export function runFormGroupMixinSuite(cfg = {}) { }); it(`when rendering children partly delayed, sets their values correctly based on - prefilled model/seriazedValue`, async () => { + prefilled model/serializedValue`, async () => { const el = /** @type {DynamicCWrapper} */ ( await fixture(html` <${dynamicChildrenTag} .fields="${['firstName']}" .modelValue="${{