Skip to content

Commit

Permalink
fix(action-menu): fix 2510, unable to control top-level action-menu s…
Browse files Browse the repository at this point in the history 
…election
  • Loading branch information
@hunterloftis
hunterloftis committed Sep 15, 2022
1 parent 60b2d3b commit c9198c2
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 9 deletions.
50 changes: 50 additions & 0 deletions packages/action-menu/stories/action-menu.stories.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import '@spectrum-web-components/menu/sp-menu-item.js';
import { ActionMenuMarkup } from './';

import '@spectrum-web-components/icons-workflow/icons/sp-icon-settings.js';
import { MenuItem } from '@spectrum-web-components/menu/src/MenuItem.js';

export default {
component: 'sp-action-menu',
Expand Down Expand Up @@ -116,3 +117,52 @@ export const submenu = (): TemplateResult => {
</sp-action-menu>
`;
};

export const controlled = (): TemplateResult => {
const state = {
snap: true,
grid: false,
guides: true,
};
function toggle(prop: keyof typeof state) {
return (event: Event): void => {
const item = event.target as MenuItem;
state[prop] = !state[prop];
// in Lit-based usage, this would be handled via render():
// <sp-menu-item ?selected=${this.isSomethingSelected}>
item.selected = state[prop];
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
document.getElementById(
'state-json'
)!.textContent = `application state: ${JSON.stringify(state)}`;
};
}
return html`
<sp-action-menu label="View">
<sp-menu-item @click=${() => alert('action')}>
Non-selectable action
</sp-menu-item>
<sp-menu-item ?selected=${state.snap} @click=${toggle('snap')}>
Snap
</sp-menu-item>
<sp-menu-item>
Show
<sp-menu slot="submenu">
<sp-menu-item
?selected=${state.grid}
@click=${toggle('grid')}
>
Grid
</sp-menu-item>
<sp-menu-item
?selected=${state.guides}
@click=${toggle('guides')}
>
Guides
</sp-menu-item>
</sp-menu>
</sp-menu-item>
</sp-action-menu>
<span id="state-json"></span>
`;
};
53 changes: 48 additions & 5 deletions packages/action-menu/test/action-menu.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ const actionSubmenuFixture = async (): Promise<ActionMenu> =>
html`
<sp-action-menu label="More Actions">
<sp-menu-item>One</sp-menu-item>
<sp-menu-item>Two</sp-menu-item>
<sp-menu-item selected class="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="selected-item">
<sp-menu-item selected class="selected-item">
B
</sp-menu-item>
<sp-menu-item>C</sp-menu-item>
Expand Down Expand Up @@ -242,10 +242,10 @@ describe('Action menu', () => {
'sp-menu[slot="submenu"]'
) as Menu;
const selectedItem = submenu.querySelector(
'#selected-item'
'.selected-item'
) as MenuItem;

expect(selectedItem.selected, 'item is not initially selected').to.be
expect(selectedItem.selected, 'item should be initially selected').to.be
.true;

let opened = oneEvent(root, 'sp-opened');
Expand All @@ -264,7 +264,50 @@ describe('Action menu', () => {
await elementUpdated(submenu);
expect(
selectedItem.selected,
'initially selected item is no longer selected'
'initially selected item should maintain selection'
).to.be.true;
});
it.only('allows top-level selection state to change', async () => {
const root = await actionSubmenuFixture();
const unselectedItem = root.querySelector('sp-menu-item') as MenuItem;
const selectedItem = root.querySelector('.selected-item') as MenuItem;

selectedItem.addEventListener('click', () => {
selectedItem.selected = false;
});

expect(unselectedItem.innerHTML).to.equal('One');
expect(unselectedItem.selected).to.be.false;
expect(selectedItem.innerHTML).to.equal('Two');
expect(selectedItem.selected).to.be.true;

let opened = oneEvent(root, 'sp-opened');
root.click();
await opened;

// close by clicking selected
// (with event listener: should set selected = false)
let closed = oneEvent(root, 'sp-closed');
selectedItem.click();
await closed;

opened = oneEvent(root, 'sp-opened');
root.click();
await opened;

// close by clicking unselected
// (no event listener: should remain selected = false)
closed = oneEvent(root, 'sp-closed');
unselectedItem.click();
await closed;

opened = oneEvent(root, 'sp-opened');
root.click();
await opened;

expect(unselectedItem.innerHTML).to.equal('One');
expect(unselectedItem.selected).to.be.false;
expect(selectedItem.innerHTML).to.equal('Two');
expect(selectedItem.selected).to.be.false;
});
});
6 changes: 4 additions & 2 deletions packages/picker/src/Picker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ export class PickerBase extends SizedMixin(Focusable) {
selected?: boolean;
}
) => {
if (this.value === el.value) {
if (this.selects != null && this.value === el.value) {
el.selected = true;
}
return (el) => {
Expand Down Expand Up @@ -458,7 +458,7 @@ export class PickerBase extends SizedMixin(Focusable) {
this,
`You no longer need to provide an <sp-menu> child to ${localName}. Any styling or attributes on the <sp-menu> will be ignored.`,
'https://opensource.adobe.com/spectrum-web-components/components/picker/#sizes',
{ level: 'deprecation' },
{ level: 'deprecation' }
);
}
}
Expand Down Expand Up @@ -556,6 +556,8 @@ export class PickerBase extends SizedMixin(Focusable) {
}

protected async manageSelection(): Promise<void> {
if (this.selects == null) return;

await this.menuStatePromise;
this.selectionPromise = new Promise(
(res) => (this.selectionResolver = res)
Expand Down
4 changes: 2 additions & 2 deletions web-test-runner.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default {
protocol: 'https:',
concurrency: 4,
concurrentBrowsers: 1,
testsFinishTimeout: 30000,
testsFinishTimeout: 60000,
coverageConfig: {
report: true,
reportDir: 'coverage',
Expand All @@ -109,7 +109,7 @@ export default {
},
testFramework: {
config: {
timeout: 5000,
timeout: 3000,
retries: 1,
},
},
Expand Down

0 comments on commit c9198c2

Please sign in to comment.