Skip to content

Commit

Permalink
chore(picker): added a new interaction strategy (#4396)
Browse files Browse the repository at this point in the history 
* chore: testing a click controller strategy for picker component

* chore: testing a new interaction strategy for picker

* fix: separated interaction strategy for picker

* chore: updated the strategy to include the dependency controller

* chore: updated open condition in interactioncontroller

* chore(picker): removed double event handling from action menu

* chore: added slottable-request handling in interactionController

* chore: change trigger button for split-button

* chore: updated split-button test

* chore: fixed conflicts after merging main

* chore: updated action-group test to remove flakiness

* chore: refractored interaction controller code

* chore: added delay for click event tests

* chore: added hacky mobile tests for mobilecontroller picker

* chore: removed unexpected only from picker responsive test

* chore: updated menu interaction model

* chore: don't dispatch synthetic click event on menu selection

* chore: removed split-button tests cz its deprecated

---------

Co-authored-by: Michael Jordan <mijordan@adobe.com>
  • Loading branch information
@TarunAdobe @majornista @rubencarvalho
2 people authored and rubencarvalho committed Jul 30, 2024
1 parent 7dc61a5 commit 888c2d7
Show file tree
Hide file tree
Showing 14 changed files with 598 additions and 407 deletions.
4 changes: 1 addition & 3 deletions packages/action-menu/src/ActionMenu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class ActionMenu extends ObserveSlotPresence(
return this.slotContentIsPresent;
}

protected override handleSlottableRequest = (
public override handleSlottableRequest = (
event: SlottableRequestEvent
): void => {
this.dispatchEvent(new SlottableRequestEvent(event.name, event.data));
Expand Down Expand Up @@ -113,8 +113,6 @@ export class ActionMenu extends ObserveSlotPresence(
class="button"
size=${this.size}
@blur=${this.handleButtonBlur}
@click=${this.handleActivate}
@pointerdown=${this.handleButtonPointerdown}
@focus=${this.handleButtonFocus}
@keydown=${{
handleEvent: this.handleEnterKeydown,
Expand Down
210 changes: 97 additions & 113 deletions packages/action-menu/test/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,58 +43,50 @@ import { sendKeys } from '@web/test-runner-commands';
ignoreResizeObserverLoopError(before, after);

const deprecatedActionMenuFixture = async (): Promise<ActionMenu> =>
await fixture<ActionMenu>(
html`
<sp-action-menu label="More Actions">
<sp-menu>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-menu>
</sp-action-menu>
`
);

const actionMenuFixture = async (): Promise<ActionMenu> =>
await fixture<ActionMenu>(
html`
<sp-action-menu label="More Actions">
await fixture<ActionMenu>(html`
<sp-action-menu label="More Actions">
<sp-menu>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`
);
</sp-menu>
</sp-action-menu>
`);

const actionMenuFixture = async (): Promise<ActionMenu> =>
await fixture<ActionMenu>(html`
<sp-action-menu label="More Actions">
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`);

const actionSubmenuFixture = async (): Promise<ActionMenu> =>
await fixture<ActionMenu>(
html`
<sp-action-menu label="More Actions">
<sp-menu-item>One</sp-menu-item>
<sp-menu-item selected id="root-selected-item">
Two
</sp-menu-item>
<sp-menu-item id="item-with-submenu">
B should be selected
<sp-menu slot="submenu">
<sp-menu-item>A</sp-menu-item>
<sp-menu-item selected id="sub-selected-item">
B
</sp-menu-item>
<sp-menu-item>C</sp-menu-item>
</sp-menu>
</sp-menu-item>
</sp-action-menu>
`
);
await fixture<ActionMenu>(html`
<sp-action-menu label="More Actions">
<sp-menu-item>One</sp-menu-item>
<sp-menu-item selected id="root-selected-item">Two</sp-menu-item>
<sp-menu-item id="item-with-submenu">
B should be selected
<sp-menu slot="submenu">
<sp-menu-item>A</sp-menu-item>
<sp-menu-item selected id="sub-selected-item">
B
</sp-menu-item>
<sp-menu-item>C</sp-menu-item>
</sp-menu>
</sp-menu-item>
</sp-action-menu>
`);

export const testActionMenu = (mode: 'sync' | 'async'): void => {
describe(`Action menu: ${mode}`, () => {
Expand All @@ -108,20 +100,18 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
await expect(el).to.be.accessible();
});
it('loads - [slot="label"]', async () => {
const el = await fixture<ActionMenu>(
html`
<sp-action-menu>
<span slot="label">More Actions</span>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`
);
const el = await fixture<ActionMenu>(html`
<sp-action-menu>
<span slot="label">More Actions</span>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`);

await elementUpdated(el);
await nextFrame();
Expand All @@ -130,20 +120,18 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
await expect(el).to.be.accessible();
});
it('loads - [custom icon]', async () => {
const el = await fixture<ActionMenu>(
html`
<sp-action-menu label="More Actions">
<sp-icon-settings slot="icon"></sp-icon-settings>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`
);
const el = await fixture<ActionMenu>(html`
<sp-action-menu label="More Actions">
<sp-icon-settings slot="icon"></sp-icon-settings>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`);

await elementUpdated(el);
await nextFrame();
Expand All @@ -154,27 +142,25 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
it('dispatches change events, no [href]', async () => {
const changeSpy = spy();

const el = await fixture<ActionMenu>(
html`
<sp-action-menu
label="More Actions"
@change=${({
target: { value },
}: Event & { target: ActionMenu }) => {
changeSpy(value);
}}
>
<sp-icon-settings slot="icon"></sp-icon-settings>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`
);
const el = await fixture<ActionMenu>(html`
<sp-action-menu
label="More Actions"
@change=${({
target: { value },
}: Event & { target: ActionMenu }) => {
changeSpy(value);
}}
>
<sp-icon-settings slot="icon"></sp-icon-settings>
<sp-menu-item>Deselect</sp-menu-item>
<sp-menu-item>Select Inverse</sp-menu-item>
<sp-menu-item>Feather...</sp-menu-item>
<sp-menu-item>Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item>Save Selection</sp-menu-item>
<sp-menu-item disabled>Make Work Path</sp-menu-item>
</sp-action-menu>
`);

expect(changeSpy.callCount).to.equal(0);
expect(el.open).to.be.false;
Expand All @@ -200,27 +186,25 @@ export const testActionMenu = (mode: 'sync' | 'async'): void => {
it('closes when Menu Item has [href]', async () => {
const changeSpy = spy();

const el = await fixture<ActionMenu>(
html`
<sp-action-menu
label="More Actions"
@change=${() => {
changeSpy();
}}
>
<sp-icon-settings slot="icon"></sp-icon-settings>
<sp-menu-item href="#">Deselect</sp-menu-item>
<sp-menu-item href="#">Select Inverse</sp-menu-item>
<sp-menu-item href="#">Feather...</sp-menu-item>
<sp-menu-item href="#">Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item href="#">Save Selection</sp-menu-item>
<sp-menu-item href="#" disabled>
Make Work Path
</sp-menu-item>
</sp-action-menu>
`
);
const el = await fixture<ActionMenu>(html`
<sp-action-menu
label="More Actions"
@change=${() => {
changeSpy();
}}
>
<sp-icon-settings slot="icon"></sp-icon-settings>
<sp-menu-item href="#">Deselect</sp-menu-item>
<sp-menu-item href="#">Select Inverse</sp-menu-item>
<sp-menu-item href="#">Feather...</sp-menu-item>
<sp-menu-item href="#">Select and Mask...</sp-menu-item>
<sp-menu-divider></sp-menu-divider>
<sp-menu-item href="#">Save Selection</sp-menu-item>
<sp-menu-item href="#" disabled>
Make Work Path
</sp-menu-item>
</sp-action-menu>
`);

expect(changeSpy.callCount).to.equal(0);
expect(el.open).to.be.false;
Expand Down
14 changes: 6 additions & 8 deletions packages/menu/src/Menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,22 +342,20 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
}
}

private willSynthesizeClick = 0;
// if the click and pointerup events are on the same target, we should not
// handle the click event.
private pointerUpTarget = null as EventTarget | null;

private handleClick(event: Event): void {
if (this.willSynthesizeClick) {
cancelAnimationFrame(this.willSynthesizeClick);
this.willSynthesizeClick = 0;
if (this.pointerUpTarget === event.target) {
this.pointerUpTarget = null;
return;
}
this.handlePointerBasedSelection(event);
}

private handlePointerup(event: Event): void {
this.willSynthesizeClick = requestAnimationFrame(() => {
event.target?.dispatchEvent(new Event('click'));
this.willSynthesizeClick = 0;
});
this.pointerUpTarget = event.target;
this.handlePointerBasedSelection(event);
}

Expand Down
16 changes: 16 additions & 0 deletions packages/picker/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@
"default": "./src/index.js"
},
"./package.json": "./package.json",
"./src/DesktopController.js": {
"development": "./src/DesktopController.dev.js",
"default": "./src/DesktopController.js"
},
"./src/InteractionController.js": {
"development": "./src/InteractionController.dev.js",
"default": "./src/InteractionController.js"
},
"./src/MobileController.js": {
"development": "./src/MobileController.dev.js",
"default": "./src/MobileController.js"
},
"./src/Picker.js": {
"development": "./src/Picker.dev.js",
"default": "./src/Picker.js"
Expand All @@ -34,6 +46,10 @@
"default": "./src/index.js"
},
"./src/picker.css.js": "./src/picker.css.js",
"./src/strategies.js": {
"development": "./src/strategies.dev.js",
"default": "./src/strategies.js"
},
"./sync/index.js": {
"development": "./sync/index.dev.js",
"default": "./sync/index.js"
Expand Down
Loading

0 comments on commit 888c2d7

Please sign in to comment.