Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(picker): added a new interaction strategy #4396

Merged
merged 21 commits into from
Jul 29, 2024
Merged

chore(picker): added a new interaction strategy #4396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b9ab321
chore: testing a click controller strategy for picker component
TarunAdobe May 3, 2024
f350d1a
chore: testing a new interaction strategy for picker
TarunAdobe May 14, 2024
1b83611
fix: separated interaction strategy for picker
TarunAdobe May 22, 2024
1290ba7
chore: updated the strategy to include the dependency controller
TarunAdobe May 28, 2024
f3e64e9
chore: updated open condition in interactioncontroller
TarunAdobe Jun 25, 2024
f61dc96
chore(picker): removed double event handling from action menu
TarunAdobe Jun 25, 2024
b163d0c
chore: added slottable-request handling in interactionController
TarunAdobe Jul 8, 2024
4fee105
chore: change trigger button for split-button
TarunAdobe Jul 8, 2024
88a52d8
chore: updated split-button test
TarunAdobe Jul 9, 2024
dcd0459
Merge branch 'main' into bug/picker-auto-slection
TarunAdobe Jul 15, 2024
b5b40d4
Merge branch 'main' into bug/picker-auto-slection
majornista Jul 15, 2024
e320657
chore: fixed conflicts after merging main
TarunAdobe Jul 16, 2024
c9b84f9
chore: updated action-group test to remove flakiness
TarunAdobe Jul 16, 2024
91ddb4e
chore: refractored interaction controller code
TarunAdobe Jul 17, 2024
088f793
chore: added delay for click event tests
TarunAdobe Jul 18, 2024
1c26e77
chore: added hacky mobile tests for mobilecontroller picker
TarunAdobe Jul 23, 2024
3e0562e
Merge branch 'main' into bug/picker-auto-slection
TarunAdobe Jul 23, 2024
344344e
chore: removed unexpected only from picker responsive test
TarunAdobe Jul 24, 2024
7f051a3
chore: updated menu interaction model
TarunAdobe Jul 25, 2024
6b214b8
chore: don't dispatch synthetic click event on menu selection
TarunAdobe Jul 25, 2024
fee37bd
chore: removed split-button tests cz its deprecated
TarunAdobe Jul 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/action-menu/src/ActionMenu.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading