ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(combobox): add support for disabled and readonly states

Browse source 
* 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 `<lion-combobox>` 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 `<input>` 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 <t_louisse@hotmail.com>

* Update packages/ui/components/combobox/src/LionCombobox.js

Co-authored-by: Thijs Louisse <t_louisse@hotmail.com>

* Update packages/ui/components/combobox/src/LionCombobox.js

Co-authored-by: Thijs Louisse <t_louisse@hotmail.com>

* fix(combobox): added accessibility tests

---------

Co-authored-by: Denilson Sá Maia <denilsonsa@gmail.com>
Co-authored-by: Thijs Louisse <t_louisse@hotmail.com>
This commit is contained in:
sainzrow 2025-07-28 16:29:32 +02:00 committed by GitHub
parent 9a80ba9c55
commit 765a1a298c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 256 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/sour-candles-worry.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/ui': patch
---
Fixed disabled and readonly attribute handling for lion-combobox

89
docs/components/combobox/use-cases.md
View file

@ -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`
<lion-button @click=${this.toggleDisabled}>Toggle disabled</lion-button> Disabled state:
${this.disabled}
<lion-combobox name="search" label="Search" ?disabled=${this.disabled}>
${lazyRender(
listboxData.map(
entry => html` <lion-option .choiceValue="${entry}">${entry}</lion-option> `,
),
)}
</lion-combobox>
`;
}
}
customElements.define('demo-disabled-state', DemoDisabledState);
export const disabledState = () => html`<demo-disabled-state></demo-disabled-state>`;
```
## 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`
<lion-button @click=${this.toggleReadonly}>Toggle readonly</lion-button> ReadOnly state:
${this.readOnly}
<lion-combobox name="search" label="Search" ?readOnly=${this.readOnly}>
${lazyRender(
listboxData.map(
entry => html` <lion-option .choiceValue="${entry}">${entry}</lion-option> `,
),
)}
</lion-combobox>
`;
}
}
customElements.define('demo-readonly-state', DemoReadonlyState);
export const readonlyState = () => html`<demo-readonly-state></demo-readonly-state>`;
```
## Options
```js preview-story

2
package-lock.json generated
View file

@ -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",

30
packages/ui/components/combobox/src/LionCombobox.js
View file

@ -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 `<div "role=combobox">` or `<input>`,
// 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 <input role="combobox">)
// 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;
}
}

139
packages/ui/components/combobox/test/lion-combobox.test.js
View file

@ -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`
<lion-combobox name="foo">
<lion-combobox label="Search" name="foo">
<lion-option .choiceValue="${'Artichoke'}">Artichoke</lion-option>
<lion-option .choiceValue="${'Chard'}">Chard</lion-option>
<lion-option .choiceValue="${'Chicory'}">Chicory</lion-option>
@ -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;
});
});
});