Skip to content

Commit

Permalink
fix(combobox): add support for external tooltip elements (#3930)
Browse files Browse the repository at this point in the history 
* fix(combobox): add support for external tooltip elements

* chore(combobox): remove unused code paths

* ci: update golden images cache

* docs(combobox): include slot present in API docs
  • Loading branch information
@Westbrook
Westbrook Johnson authored and Westbrook committed Jan 25, 2024
1 parent ca4cac6 commit 4849a08
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 46 deletions.
57 changes: 28 additions & 29 deletions packages/combobox/src/Combobox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import '@spectrum-web-components/menu/sp-menu.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import '@spectrum-web-components/picker-button/sp-picker-button.js';
import { Textfield } from '@spectrum-web-components/textfield';
import type { Tooltip } from '@spectrum-web-components/tooltip';

import styles from './combobox.css.js';
import chevronStyles from '@spectrum-web-components/icon/src/spectrum-icon-chevron.css.js';
Expand All @@ -47,6 +48,7 @@ export type ComboboxOption = {

/**
* @element sp-combobox
* @slot tooltip - Tooltip to to be applied to the the Picker Button
*/
export class Combobox extends Textfield {
public static override get styles(): CSSResultArray {
Expand Down Expand Up @@ -94,6 +96,8 @@ export class Combobox extends Textfield {
@property({ type: Array })
public optionEls: MenuItem[] = [];

protected tooltipEl?: Tooltip;

// { value: "String thing", id: "string1" }
public override focus(): void {
this.focusElement.focus();
Expand Down Expand Up @@ -165,6 +169,14 @@ export class Combobox extends Textfield {
});
}

protected handleTooltipSlotchange(
event: Event & { target: HTMLSlotElement }
): void {
this.tooltipEl = event.target.assignedElements()[0] as
| Tooltip
| undefined;
}

public setOptionsFromSlottedItems(): void {
const elements = this.optionSlot.assignedElements({
flatten: true,
Expand Down Expand Up @@ -223,18 +235,7 @@ export class Combobox extends Textfield {
this.open = true;
}

public handleListPointerenter(event: PointerEvent): void {
const descendent = event
.composedPath()
.find((el) => typeof (el as MenuItem).value !== 'undefined');
if (descendent) this.activeDescendent = descendent as MenuItem;
}

public handleListPointerleave(): void {
this.activeDescendent = undefined;
}

public handleMenuChange(event: PointerEvent & { target: Menu }): void {
protected handleMenuChange(event: PointerEvent & { target: Menu }): void {
const { target } = event;
this.value = target.selected[0];
event.preventDefault();
Expand All @@ -243,6 +244,10 @@ export class Combobox extends Textfield {
this.focus();
}

public handleClosed(): void {
this.open = false;
}

public handleOpened(): void {
// Do stuff here?
}
Expand All @@ -261,16 +266,6 @@ export class Combobox extends Textfield {
return super.shouldUpdate(changed);
}

private positionListbox(): void {
const targetRect = this.overlay.getBoundingClientRect();
const rootRect = this.getBoundingClientRect();
this.listbox.style.transform = `translate(${
targetRect.x - rootRect.x
}px, ${targetRect.y - rootRect.y}px)`;
this.listbox.style.height = `${targetRect.height}px`;
this.listbox.style.maxHeight = `${targetRect.height}px`;
}

protected override onBlur(event: FocusEvent): void {
if (
event.relatedTarget &&
Expand Down Expand Up @@ -308,9 +303,7 @@ export class Combobox extends Textfield {
type="text"
.value=${live(this.displayValue)}
tabindex="0"
@sp-closed=${() => {
this.open = false;
}}
@sp-closed=${this.handleClosed}
@sp-opened=${this.handleOpened}
type=${this.type}
aria-describedby=${this.helpTextId}
Expand All @@ -337,6 +330,9 @@ export class Combobox extends Textfield {

protected override render(): TemplateResult {
const width = (this.input || this).offsetWidth;
if (this.tooltipEl) {
this.tooltipEl.disabled = this.open;
}
return html`
${super.render()}
<sp-picker-button
Expand Down Expand Up @@ -367,7 +363,7 @@ export class Combobox extends Textfield {
>
<sp-menu
@change=${this.handleMenuChange}
tabindex="0"
tabindex="-1"
aria-labelledby="label"
id="listbox-menu"
role="listbox"
Expand Down Expand Up @@ -401,6 +397,12 @@ export class Combobox extends Textfield {
</sp-menu>
</sp-popover>
</sp-overlay>
<slot
aria-hidden="true"
name="tooltip"
id="tooltip"
@slotchange=${this.handleTooltipSlotchange}
></slot>
`;
}

Expand Down Expand Up @@ -437,9 +439,6 @@ export class Combobox extends Textfield {
if (!this.focused && this.open) {
this.open = false;
}
if (changed.has('value')) {
if (this.overlay && this.open) this.positionListbox();
}
if (changed.has('activeDescendent')) {
if (changed.get('activeDescendent')) {
(changed.get('activeDescendent') as MenuItem).focused = false;
Expand Down
27 changes: 25 additions & 2 deletions packages/combobox/stories/combobox.stories.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ governing permissions and limitations under the License.
import { html, TemplateResult } from '@spectrum-web-components/base';

import { ComboboxOption } from '..';
import '../sp-combobox.js';
import '../sp-combobox-item.js';
import '@spectrum-web-components/combobox/sp-combobox.js';
import '@spectrum-web-components/combobox/sp-combobox-item.js';
import '@spectrum-web-components/tooltip/sp-tooltip.js';

export default {
title: 'Combobox',
Expand Down Expand Up @@ -159,3 +160,25 @@ export const kerningLightDOM = (): TemplateResult => {
</sp-combobox>
`;
};

export const withTooltip = (): TemplateResult => {
return html`
<sp-combobox
.autocomplete=${'none'}
id="combobox-6"
label="Kerning"
style="min-width: 80px;--spectrum-textfield-m-min-width:0;width:100px;"
>
${optionsL.map(
(option) => html`
<sp-menu-item id=${option.id} value=${option.value}>
${option.value}
</sp-menu-item>
`
)}
<sp-tooltip slot="tooltip" self-managed placement="right" open>
Kerning
</sp-tooltip>
</sp-combobox>
`;
};
6 changes: 3 additions & 3 deletions packages/combobox/test/combobox.data.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ governing permissions and limitations under the License.
import {
elementUpdated,
expect,
fixture,
html,
nextFrame,
waitUntil,
} from '@open-wc/testing';

import '../sp-combobox.js';
import '../sp-combobox-item.js';
import '@spectrum-web-components/combobox/sp-combobox.js';
import '@spectrum-web-components/combobox/sp-combobox-item.js';
import { fixture } from '../../../test/testing-helpers.js';
import { Combobox, ComboboxOption } from '..';
import { TestableCombobox } from './index.js';
import { SpectrumElement, TemplateResult } from '@spectrum-web-components/base';
Expand Down
64 changes: 52 additions & 12 deletions packages/combobox/test/combobox.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,13 @@ governing permissions and limitations under the License.
import {
elementUpdated,
expect,
fixture,
html,
nextFrame,
oneEvent,
} from '@open-wc/testing';

import '../sp-combobox.js';
import '../sp-combobox-item.js';
import '@spectrum-web-components/theme/sp-theme.js';
import '@spectrum-web-components/theme/src/themes.js';
import type { Theme } from '@spectrum-web-components/theme';
import '@spectrum-web-components/combobox/sp-combobox.js';
import '@spectrum-web-components/combobox/sp-combobox-item.js';
import { ComboboxOption } from '..';
import {
arrowDownEvent,
Expand All @@ -32,16 +28,20 @@ import {
endEvent,
enterEvent,
escapeEvent,
fixture,
homeEvent,
} from '../../../test/testing-helpers.js';
import {
a11ySnapshot,
executeServerCommand,
findAccessibilityNode,
sendKeys,
} from '@web/test-runner-commands';
import { PickerButton } from '@spectrum-web-components/picker-button';
import { TestableCombobox, testActiveElement } from './index.js';
import { sendMouse } from '../../../test/plugins/browser.js';
import { withTooltip } from '../stories/combobox.stories.js';
import type { Tooltip } from '@spectrum-web-components/tooltip';

const comboboxFixture = async (): Promise<TestableCombobox> => {
const options: ComboboxOption[] = [
Expand All @@ -51,16 +51,12 @@ const comboboxFixture = async (): Promise<TestableCombobox> => {
{ id: 'thing4', value: 'Efg Thing 4' },
];

const test = await fixture<Theme>(
const el = await fixture<TestableCombobox>(
html`
<sp-theme theme="spectrum" color="light" scale="medium">
<sp-combobox .options=${options}>Combobox</sp-combobox>
</sp-theme>
<sp-combobox .options=${options}>Combobox</sp-combobox>
`
);

const el = test.querySelector('sp-combobox') as unknown as TestableCombobox;

return el;
};

Expand Down Expand Up @@ -976,4 +972,48 @@ describe('Combobox', () => {
expect(el.open).to.be.true;
});
});

it('closes tooltip on button blur', async () => {
const el = await fixture<TestableCombobox>(withTooltip());
await elementUpdated(el);
const input1 = document.createElement('input');
const input2 = document.createElement('input');
const tooltipEl = el.querySelector('sp-tooltip') as Tooltip;
el.insertAdjacentElement('beforebegin', input1);
el.insertAdjacentElement('afterend', input2);
input1.focus();
expect(document.activeElement === input1).to.be.true;
const tooltipOpened = oneEvent(el, 'sp-opened');
await sendKeys({
press: 'Tab',
});
await tooltipOpened;
expect(
document.activeElement === el,
`Actually, ${document.activeElement?.localName}`
).to.be.true;
expect(tooltipEl.open).to.be.true;
expect(el.open).to.be.false;
expect(el.focused).to.be.true;

const menuOpen = oneEvent(el, 'sp-opened');
const tooltipClosed = oneEvent(el, 'sp-closed');
await sendKeys({
press: 'ArrowDown',
});
await menuOpen;
await tooltipClosed;
expect(document.activeElement === el).to.be.true;
expect(tooltipEl.open).to.be.false;
expect(el.open).to.be.true;

const menuClosed = oneEvent(el, 'sp-closed');
await sendKeys({
press: 'Tab',
});
await menuClosed;
expect(document.activeElement === el).to.be.false;
expect(tooltipEl.open).to.be.false;
expect(el.open).to.be.false;
});
});

0 comments on commit 4849a08

Please # to comment.