Skip to content

Commit

Permalink
fix(combobox): allow numeric values and trigger change event on keybo… (
Browse files Browse the repository at this point in the history 
#4405)

* fix(combobox): allow numeric values and trigger change event on keyboard selection

* chore: call click method to trigger change

* chore: update tests

---------

Co-authored-by: rmanole <rmanole@adobe.com>
  • Loading branch information
@Rocss
Rocss and rmanole committed May 8, 2024
1 parent 45d69d0 commit 235ae7c
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 107 deletions.
17 changes: 13 additions & 4 deletions packages/combobox/src/Combobox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,12 @@ export class Combobox extends Textfield {
}

private scrollToActiveDescendant(): void {
const activeEl = this.shadowRoot.querySelector(
`#${this.activeDescendant?.value}`
) as HTMLElement;
if (!this.activeDescendant) {
return;
}
const activeEl = this.shadowRoot.getElementById(
this.activeDescendant.value
);
if (activeEl) {
activeEl.scrollIntoView({ block: 'nearest' });
}
Expand Down Expand Up @@ -210,7 +213,13 @@ export class Combobox extends Textfield {
if (!this.activeDescendant) {
return;
}
this.value = this.activeDescendant.itemText;

const activeEl = this.shadowRoot.getElementById(
this.activeDescendant.value
);
if (activeEl) {
activeEl.click();
}
}

public filterAvailableOptions(): void {
Expand Down
223 changes: 126 additions & 97 deletions packages/combobox/test/combobox.data.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import {
expect,
html,
nextFrame,
oneEvent,
waitUntil,
} from '@open-wc/testing';

import { sendKeys } from '@web/test-runner-commands';
import '@spectrum-web-components/combobox/sp-combobox.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import { fixture } from '../../../test/testing-helpers.js';
Expand Down Expand Up @@ -51,18 +52,16 @@ describe('Combobox Data', () => {
expect(el.options).to.deep.equal(options);
});
it('accepts options as html', async () => {
const el = await fixture<TestableCombobox>(
html`
<sp-combobox>
Combobox Test
<sp-menu-item value="pineapple">Pineapple</sp-menu-item>
<sp-menu-item value="yuzu">Yuzu</sp-menu-item>
<sp-menu-item value="kumquat">Kumquat</sp-menu-item>
<sp-menu-item value="lychee">Lychee</sp-menu-item>
<sp-menu-item value="durian">Durian</sp-menu-item>
</sp-combobox>
`
);
const el = await fixture<TestableCombobox>(html`
<sp-combobox>
Combobox Test
<sp-menu-item value="pineapple">Pineapple</sp-menu-item>
<sp-menu-item value="yuzu">Yuzu</sp-menu-item>
<sp-menu-item value="kumquat">Kumquat</sp-menu-item>
<sp-menu-item value="lychee">Lychee</sp-menu-item>
<sp-menu-item value="durian">Durian</sp-menu-item>
</sp-combobox>
`);
await elementUpdated(el);

const processedOptions = el.availableOptions.map(
Expand Down Expand Up @@ -95,20 +94,18 @@ describe('Combobox Data', () => {
]);
});
it('accepts additional options as html', async () => {
const el = await fixture<TestableCombobox>(
html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`
);
const el = await fixture<TestableCombobox>(html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`);
await elementUpdated(el);

let processedOptions = el.availableOptions.map(
Expand Down Expand Up @@ -140,20 +137,18 @@ describe('Combobox Data', () => {
expect(processedOptions).to.deep.equal([...options, newOption]);
});
it('accepts updated value as html', async () => {
const el = await fixture<TestableCombobox>(
html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`
);
const el = await fixture<TestableCombobox>(html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`);
await elementUpdated(el);

let processedOptions = el.availableOptions.map(
Expand Down Expand Up @@ -192,20 +187,18 @@ describe('Combobox Data', () => {
expect(processedOptions).to.deep.equal(newOptions);
});
it('accepts updated id as html', async () => {
const el = await fixture<TestableCombobox>(
html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`
);
const el = await fixture<TestableCombobox>(html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`);
await elementUpdated(el);

let processedOptions = el.availableOptions.map(
Expand Down Expand Up @@ -243,20 +236,18 @@ describe('Combobox Data', () => {
expect(processedOptions).to.deep.equal(newOptions);
});
it('accepts replacement options as html', async () => {
const el = await fixture<TestableCombobox>(
html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`
);
const el = await fixture<TestableCombobox>(html`
<sp-combobox>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</sp-combobox>
`);
await elementUpdated(el);

let processedOptions = el.availableOptions.map(
Expand Down Expand Up @@ -299,20 +290,18 @@ describe('Combobox Data', () => {
expect(processedOptions).to.deep.equal(options);
});
it('accepts options through slots', async () => {
const test = await fixture<SpectrumElement>(
html`
<combobox-slot-test-el>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</combobox-slot-test-el>
`
);
const test = await fixture<SpectrumElement>(html`
<combobox-slot-test-el>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</combobox-slot-test-el>
`);
const el = test.shadowRoot.querySelector(
'sp-combobox'
) as unknown as TestableCombobox;
Expand All @@ -333,20 +322,18 @@ describe('Combobox Data', () => {
expect(processedOptions).to.deep.equal(options);
});
it('accepts adding through slots', async function () {
const test = await fixture<SpectrumElement>(
html`
<combobox-slot-test-el>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</combobox-slot-test-el>
`
);
const test = await fixture<SpectrumElement>(html`
<combobox-slot-test-el>
Combobox Test
${options.map((option) => {
return html`
<sp-menu-item value=${option.value}>
${option.itemText}
</sp-menu-item>
`;
})}
</combobox-slot-test-el>
`);
const el = test.shadowRoot.querySelector(
'sp-combobox'
) as unknown as TestableCombobox;
Expand Down Expand Up @@ -389,4 +376,46 @@ describe('Combobox Data', () => {
}));
expect(processedOptions).to.deep.equal([...options, newOption]);
});
it('accepts numeric values as html', async () => {
const el = await fixture<TestableCombobox>(html`
<sp-combobox>
Combobox Test
<sp-menu-item value="1">Mambo no. 1</sp-menu-item>
<sp-menu-item value="2">Mambo no. 2</sp-menu-item>
<sp-menu-item value="3">Mambo no. 3</sp-menu-item>
<sp-menu-item value="4">Mambo no. 4</sp-menu-item>
<sp-menu-item value="5">Mambo no. 5</sp-menu-item>
</sp-combobox>
`);
await elementUpdated(el);

expect(el.activeDescendant).to.be.undefined;

el.focus();
await elementUpdated(el);

await sendKeys({
press: 'ArrowDown',
});
await elementUpdated(el);

expect(el.activeDescendant).to.not.be.undefined;
expect(el.activeDescendant.value).to.equal('1');

const activeDescendant = el.shadowRoot.getElementById('#1') as MenuItem;

await elementUpdated(activeDescendant);
await nextFrame();
await nextFrame();

const change = oneEvent(el, 'change');
await sendKeys({
press: 'Enter',
});

await change;
expect(el.open).to.be.false;
expect(el.activeDescendant).to.be.undefined;
expect(el.value).to.equal('Mambo no. 1');
});
});
10 changes: 4 additions & 6 deletions test/testing-helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,9 @@ export async function testForMemoryLeaks(
let active = false;

// Call fixture with 'htmlString' as the additional argument
const el = await fixture<HTMLElement>(
html`
<div></div>
`
);
const el = await fixture<HTMLElement>(html`
<div></div>
`);

async function toggle(
forced: boolean | undefined = undefined
Expand Down Expand Up @@ -292,7 +290,7 @@ export async function fixture<T extends Element>(
dir: 'ltr' | 'rtl' | 'auto' = 'ltr'
): Promise<T> {
const test = await owcFixture<Theme>(html`
<sp-theme theme="spectrum" scale="medium" color="light">
<sp-theme system="spectrum" scale="medium" color="light">
${story}
<style>
sp-theme {
Expand Down

0 comments on commit 235ae7c

Please sign in to comment.