From 765a1a298c0110b50e63208f7072b48d31f8e1e1 Mon Sep 17 00:00:00 2001 From: sainzrow Date: Mon, 28 Jul 2025 16:29:32 +0200 Subject: [PATCH] fix(combobox): add support for disabled and readonly states MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(combobox): don't use setAttribute for disabled/enabled attributes How to reproduce this bug: 1. Go to https://lion.js.org/components/combobox/overview/ or just use the combobox in your application. 2. Inspect the combobox in the browser. Click on the `` element at the DOM inspector. 3. At the JavaScript console, print `$0.disabled`. Observe it is currently `false`, and the user can interact with it normally. 4. (optional) Run `$0.disabled =true`. Observe the user cannot interact with it anymore. That's expected. 5. Run `$0.disabled = false`. BUG: The user cannot interact with the combobox anymore, despite `disabled` being false. Root cause: If you inspect the `` element, you can see it has `disabled="false"`. The [specs](https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#enabling-and-disabling-form-controls:-the-disabled-attribute) say: > The `disabled` content attribute is a [boolean attribute](https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attribute). > > A form control is **disabled** if any of the following are true: > * the element is a `button`, `input`, `select`, `textarea`, or form-associated custom element, and the `disabled` attribute is specified on this element (regardless of its value); And [also](https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#boolean-attribute): > A number of attributes are **boolean attributes**. The presence of a boolean attribute on an element represents the true value, and the absence of the attribute represents the false value. Thanks to @thematho for finding the root cause. * fix(combobox): add support for disabled and readonly states with corresponding tests * Update packages/ui/components/combobox/src/LionCombobox.js Co-authored-by: Thijs Louisse * Update packages/ui/components/combobox/src/LionCombobox.js Co-authored-by: Thijs Louisse * Update packages/ui/components/combobox/src/LionCombobox.js Co-authored-by: Thijs Louisse * fix(combobox): added accessibility tests --------- Co-authored-by: Denilson Sá Maia Co-authored-by: Thijs Louisse --- .changeset/sour-candles-worry.md | 5 + docs/components/combobox/use-cases.md | 89 +++++++++++ package-lock.json | 2 +- .../components/combobox/src/LionCombobox.js | 30 +++- .../combobox/test/lion-combobox.test.js | 139 +++++++++++++++++- 5 files changed, 256 insertions(+), 9 deletions(-) create mode 100644 .changeset/sour-candles-worry.md diff --git a/.changeset/sour-candles-worry.md b/.changeset/sour-candles-worry.md new file mode 100644 index 000000000..fd1e409bd --- /dev/null +++ b/.changeset/sour-candles-worry.md @@ -0,0 +1,5 @@ +--- +'@lion/ui': patch +--- + +Fixed disabled and readonly attribute handling for lion-combobox diff --git a/docs/components/combobox/use-cases.md b/docs/components/combobox/use-cases.md index 4d63e3d06..853076b9f 100644 --- a/docs/components/combobox/use-cases.md +++ b/docs/components/combobox/use-cases.md @@ -29,6 +29,7 @@ import { LitElement, html, repeat } from '@mdjs/mdjs-preview'; import { listboxData, listboxComplexData } from '../listbox/src/listboxData.js'; import { LionCombobox } from '@lion/ui/combobox.js'; import { Required } from '@lion/ui/form-core.js'; +import '@lion/ui/define/lion-button.js'; import '@lion/ui/define/lion-combobox.js'; import '@lion/ui/define/lion-option.js'; import './src/demo-selection-display.js'; @@ -176,6 +177,94 @@ export const customMatchCondition = () => html` `; ``` +## Disabled option + +```js preview-story +class DemoDisabledState extends LitElement { + static get properties() { + return { disabled: { type: Boolean } }; + } + + constructor() { + super(); + /** @type {string[]} */ + this.disabled = true; + } + + get combobox() { + return /** @type {LionCombobox} */ (this.shadowRoot?.querySelector('#combobox')); + } + + /** + * @param {InputEvent & {target: HTMLInputElement}} e + */ + toggleDisabled(e) { + this.disabled = !this.disabled; + this.requestUpdate(); + } + + render() { + return html` + Toggle disabled Disabled state: + ${this.disabled} + + ${lazyRender( + listboxData.map( + entry => html` ${entry} `, + ), + )} + + `; + } +} +customElements.define('demo-disabled-state', DemoDisabledState); +export const disabledState = () => html``; +``` + +## Readonly option + +```js preview-story +class DemoReadonlyState extends LitElement { + static get properties() { + return { readOnly: { type: Boolean } }; + } + + constructor() { + super(); + /** @type {string[]} */ + this.readOnly = true; + } + + get combobox() { + return /** @type {LionCombobox} */ (this.shadowRoot?.querySelector('#combobox')); + } + + /** + * @param {InputEvent & {target: HTMLInputElement}} e + */ + toggleReadonly(e) { + this.readOnly = !this.readOnly; + this.requestUpdate(); + } + + render() { + return html` + Toggle readonly ReadOnly state: + ${this.readOnly} + + ${lazyRender( + listboxData.map( + entry => html` ${entry} `, + ), + )} + + `; + } +} +customElements.define('demo-readonly-state', DemoReadonlyState); +export const readonlyState = () => html``; +``` + ## Options ```js preview-story diff --git a/package-lock.json b/package-lock.json index 7f0243c0f..14657a137 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28965,7 +28965,7 @@ }, "packages/ui": { "name": "@lion/ui", - "version": "0.11.2", + "version": "0.11.6", "license": "MIT", "dependencies": { "@bundled-es-modules/message-format": "^6.2.4", diff --git a/packages/ui/components/combobox/src/LionCombobox.js b/packages/ui/components/combobox/src/LionCombobox.js index d62486eac..8328a5e9f 100644 --- a/packages/ui/components/combobox/src/LionCombobox.js +++ b/packages/ui/components/combobox/src/LionCombobox.js @@ -502,6 +502,10 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi if (this._selectionDisplayNode) { this._selectionDisplayNode.comboboxElement = this; } + + if (this.disabled || this.readOnly) { + this.__setComboboxDisabledAndReadOnly(); + } } /** @@ -657,9 +661,11 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi const alwaysHideOn = ['Tab', 'Escape']; const notMultipleChoiceHideOn = ['Enter']; if ( - lastKey && - (alwaysHideOn.includes(lastKey) || - (!this.multipleChoice && notMultipleChoiceHideOn.includes(lastKey))) + this.disabled || + this.readOnly || + (lastKey && + (alwaysHideOn.includes(lastKey) || + (!this.multipleChoice && notMultipleChoiceHideOn.includes(lastKey)))) ) { return false; } @@ -1213,8 +1219,22 @@ export class LionCombobox extends LocalizeMixin(OverlayMixin(CustomChoiceGroupMi */ __setComboboxDisabledAndReadOnly() { if (this._comboboxNode) { - this._comboboxNode.setAttribute('disabled', `${this.disabled}`); - this._comboboxNode.setAttribute('readonly', `${this.readOnly}`); + // Since `._comboboxNode` can either be `
` or ``, + // we need to cater for both scenarios (aria for semantics, "native attr" for operability) + this._comboboxNode.toggleAttribute('disabled', this.disabled); + this._comboboxNode.setAttribute('aria-disabled', `${this.disabled}`); + this._comboboxNode.toggleAttribute('readonly', this.readOnly); + this._comboboxNode.setAttribute('aria-readonly', `${this.readOnly}`); + } + + if (this._inputNode) { + // N.B. in case ._inputNode === ._comboboxNode (we have ) + // this value has already been set above. This is fine, as a toggle with boolean flag is idempotent. + this._inputNode.toggleAttribute('disabled', this.disabled); + this._inputNode.toggleAttribute('readOnly', this.readOnly); + + this._inputNode.setAttribute('aria-readonly', `${this.readOnly}`); + this._inputNode.tabIndex = this.disabled ? -1 : 0; } } diff --git a/packages/ui/components/combobox/test/lion-combobox.test.js b/packages/ui/components/combobox/test/lion-combobox.test.js index dac814cb1..e1af919c3 100644 --- a/packages/ui/components/combobox/test/lion-combobox.test.js +++ b/packages/ui/components/combobox/test/lion-combobox.test.js @@ -24,12 +24,12 @@ import { isActiveElement } from '../../core/test-helpers/isActiveElement.js'; */ /** - * @param {{ autocomplete?:'none'|'list'|'both', matchMode?:'begin'|'all' }} config + * @param {{ autocomplete?:'none'|'list'|'both', matchMode?:'begin'|'all', disabled?: boolean, readonly?: boolean }} config */ -async function fruitFixture({ autocomplete, matchMode } = {}) { +async function fruitFixture({ autocomplete, matchMode, disabled, readonly } = {}) { const el = /** @type {LionCombobox} */ ( await fixture(html` - + Artichoke Chard Chicory @@ -43,6 +43,12 @@ async function fruitFixture({ autocomplete, matchMode } = {}) { if (matchMode) { el.matchMode = matchMode; } + if (disabled) { + el.disabled = disabled; + } + if (readonly) { + el.readOnly = readonly; + } await el.updateComplete; return [el, el.formElements]; } @@ -3431,4 +3437,131 @@ describe('lion-combobox', () => { }); }); }); + + describe('Disabled state', () => { + it('does not open overlay or allow input when disabled', async () => { + const [el] = await fruitFixture({ disabled: true }); + expect(el.disabled).to.equal(true); + + const { _inputNode } = getComboboxMembers(/** @type {LionCombobox} */ (el)); + + expect(el.disabled).to.be.true; + expect(_inputNode.disabled).to.be.true; + + // Try to open overlay by clicking input + _inputNode.dispatchEvent(new Event('click', { bubbles: true, composed: true })); + await el.updateComplete; + expect(el.opened).to.be.false; + }); + + it('does open overlay or allow input when disabled state is removed after it was previously disabled', async () => { + const [el] = await fruitFixture({ disabled: true }); + expect(el.disabled).to.equal(true); + + const { _inputNode } = getComboboxMembers(/** @type {LionCombobox} */ (el)); + + expect(el.disabled).to.be.true; + expect(_inputNode.disabled).to.be.true; + + // Try to open overlay by clicking input + _inputNode.dispatchEvent(new Event('click', { bubbles: true, composed: true })); + await el.updateComplete; + expect(el.opened).to.be.false; + + el.disabled = false; + await el.updateComplete; + + expect(el.disabled).to.be.false; + expect(_inputNode.disabled).to.be.false; + + // Try to open overlay by clicking input + async function open() { + await mimicUserTyping(/** @type {LionCombobox} */ (el), 'ch'); + return el.updateComplete; + } + + await open(); + expect(el.opened).to.be.true; + }); + + it('sets aria-disabled and disables focus when combobox is disabled', async () => { + const [el] = await fruitFixture({ disabled: true }); + const { _comboboxNode, _inputNode } = getComboboxMembers(/** @type {LionCombobox} */ (el)); + + // input should not be focusable + expect(_comboboxNode.tabIndex).to.equal(-1); + expect(_inputNode.tabIndex).to.equal(-1); + + // aria-disabled should be set + expect(el.getAttribute('aria-disabled')).to.equal('true'); + + // aria-disabled should be set + expect(_inputNode.getAttribute('aria-disabled')).to.equal('true'); + + await expect(el).to.be.accessible(); + }); + + it('ensure focus when combobox is readonly', async () => { + const [el] = await fruitFixture({ readonly: true }); + const { _comboboxNode, _inputNode } = getComboboxMembers(/** @type {LionCombobox} */ (el)); + + // input should be focusable + expect(_inputNode.tabIndex).to.equal(0); + + // aria-disabled should be set + expect(_comboboxNode.getAttribute('aria-readonly')).to.equal('true'); + + // aria-disabled should be set + expect(_inputNode.getAttribute('aria-readonly')).to.equal('true'); + + await expect(el).to.be.accessible(); + }); + }); + + describe('Readonly state', () => { + it('does not open overlay or allow input when readonly', async () => { + const [el] = await fruitFixture({ readonly: true }); + + const { _inputNode } = getComboboxMembers(/** @type {LionCombobox} */ (el)); + expect(el.readOnly).to.equal(true); + + expect(el.readOnly).to.be.true; + expect(_inputNode.readOnly).to.be.true; + + // Try to open overlay by clicking input + _inputNode.dispatchEvent(new Event('click', { bubbles: true, composed: true })); + await el.updateComplete; + expect(el.opened).to.be.false; + }); + + it('does open overlay or allow input when readonly state is removed after it was previously set', async () => { + const [el] = await fruitFixture({ readonly: true }); + expect(el.readOnly).to.equal(true); + + const { _inputNode } = getComboboxMembers(/** @type {LionCombobox} */ (el)); + + expect(el.readOnly).to.be.true; + expect(_inputNode.readOnly).to.be.true; + + // Try to open overlay by clicking input + _inputNode.dispatchEvent(new Event('click', { bubbles: true, composed: true })); + await el.updateComplete; + expect(el.opened).to.be.false; + + el.readOnly = false; + await el.updateComplete; + + expect(el.readOnly).to.be.false; + expect(_inputNode.readOnly).to.be.false; + + // Try to open overlay by clicking input + async function open() { + await mimicUserTyping(/** @type {LionCombobox} */ (el), 'ch'); + return el.updateComplete; + } + + await open(); + expect(el.opened).to.be.true; + }); + }); });