Skip to content

Commit

Permalink
fix(menu): manage deeply slotted menu items and initial focus
Browse files Browse the repository at this point in the history
  • Loading branch information
@Westbrook
Westbrook committed Sep 18, 2023
1 parent 40b3975 commit 7f9ad69
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 20 deletions.
71 changes: 52 additions & 19 deletions packages/menu/src/Menu.ts
View file
Expand Up @@ -114,18 +114,28 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
private updateCachedMenuItems(): MenuItem[] {
this.cachedChildItems = [];

const slotElements = this.menuSlot.assignedElements({ flatten: true });
for (const slotElement of slotElements) {
const childMenuItems: MenuItem[] =
slotElement instanceof MenuItem
? [slotElement as MenuItem]
: ([...slotElement.querySelectorAll(`*`)] as MenuItem[]);

for (const childMenuItem of childMenuItems) {
if (this.childItemSet.has(childMenuItem)) {
this.cachedChildItems.push(childMenuItem);
}
const slottedElements = this.menuSlot.assignedElements({
flatten: true,
}) as HTMLElement[];
// Recursively flatten <slot> and non-<sp-menu-item> elements assigned to the menu into a single array.
for (const [i, slottedElement] of slottedElements.entries()) {
if (this.childItemSet.has(slottedElement as MenuItem)) {
// Assign <sp-menu-item> members of the array that are in this.childItemSet to this.chachedChildItems.
this.cachedChildItems.push(slottedElement as MenuItem);
continue;
}
const isHTMLSlotElement = slottedElement.localName === 'slot';
const flattenedChildren = isHTMLSlotElement
? (slottedElement as HTMLSlotElement).assignedElements({
flatten: true,
})
: [...slottedElement.querySelectorAll(`:scope > *`)];
slottedElements.splice(
i,
1,
slottedElement,
...(flattenedChildren as HTMLElement[])
);
}

return this.cachedChildItems;
Expand Down Expand Up @@ -304,9 +314,6 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
}

private handleClick(event: Event): void {
if (event.defaultPrevented) {
return;
}
const path = event.composedPath();
const target = path.find((el) => {
/* c8 ignore next 3 */
Expand All @@ -315,6 +322,13 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
}
return el.getAttribute('role') === this.childRole;
}) as MenuItem;
if (event.defaultPrevented) {
const index = this.childItems.indexOf(target);
if (target?.menuData.focusRoot === this && index > -1) {
this.focusedItemIndex = index;
}
return;
}
if (target?.href && target.href.length) {
// This event will NOT ALLOW CANCELATION as link action
// cancelation should occur on the `<sp-menu-item>` itself.
Expand Down Expand Up @@ -367,9 +381,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
const offset = this.childItems.findIndex(
(childItem) => childItem === activeElement
);
if (offset > 0) {
this.focusMenuItemByOffset(offset);
}
this.focusMenuItemByOffset(Math.max(offset, 0));
}
}
this.startListeningToKeyboard();
Expand Down Expand Up @@ -568,6 +580,27 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
lastFocusedItem.focused = true;
}
const { code } = event;
if (
event.shiftKey &&
event.target !== this &&
this.hasAttribute('tabindex')
) {
this.removeAttribute('tabindex');
const replaceTabindex = (
event: FocusEvent | KeyboardEvent
): void => {
if (
!(event as KeyboardEvent).shiftKey &&
!this.hasAttribute('tabindex')
) {
this.tabIndex = 0;
document.removeEventListener('keyup', replaceTabindex);
this.removeEventListener('focusout', replaceTabindex);
}
};
document.addEventListener('keyup', replaceTabindex);
this.addEventListener('focusout', replaceTabindex);
}
if (code === 'Tab') {
this.prepareToCleanUp();
return;
Expand Down Expand Up @@ -606,7 +639,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
let itemToFocus = this.childItems[this.focusedItemIndex];
let availableItems = this.childItems.length;
// cycle through the available items in the directions of the offset to find the next non-disabled item
while (itemToFocus.disabled && availableItems) {
while (itemToFocus?.disabled && availableItems) {
availableItems -= 1;
this.focusedItemIndex =
(this.childItems.length + this.focusedItemIndex + step) %
Expand Down Expand Up @@ -727,7 +760,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) {
}

private forwardFocusVisibleToItem(item: MenuItem): void {
if (item.menuData.focusRoot !== this) {
if (!item || item.menuData.focusRoot !== this) {
return;
}
this.closeDescendentOverlays();
Expand Down
58 changes: 57 additions & 1 deletion packages/menu/stories/index.ts
View file
Expand Up @@ -10,11 +10,16 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/

import { html, TemplateResult } from '@spectrum-web-components/base';
import {
html,
LitElement,
TemplateResult,
} from '@spectrum-web-components/base';
import '@spectrum-web-components/menu/sp-menu.js';
import '@spectrum-web-components/menu/sp-menu-divider.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import '@spectrum-web-components/popover/sp-popover.js';
import { Menu } from '@spectrum-web-components/menu';

export const MenuMarkup = ({
size = 'm' as 's' | 'm' | 'l' | 'xl',
Expand Down Expand Up @@ -43,3 +48,54 @@ export const MenuMarkup = ({
</sp-popover>
`;
};

export class ComplexSlottedGroup extends LitElement {
get menu(): Menu {
return this.renderRoot.querySelector('sp-menu') as Menu;
}
protected override render(): TemplateResult {
return html`
<sp-menu>
<sp-menu-group>
<sp-menu-item id="i-1">Before First</sp-menu-item>
<slot name="before"></slot>
</sp-menu-group>
<sp-menu-group>
<sp-menu-item id="i-4">Sibling 1</sp-menu-item>
<slot></slot>
<sp-menu-item id="i-10">Sibling 2</sp-menu-item>
</sp-menu-group>
<sp-menu-group>
<sp-menu-item id="i-11">After 1</sp-menu-item>
<sp-menu-item id="i-12">After 2</sp-menu-item>
</sp-menu-group>
</sp-menu>
`;
}
}

customElements.define('complex-slotted-group', ComplexSlottedGroup);

export class ComplexSlottedMenu extends LitElement {
get menu(): Menu {
return (
this.renderRoot.querySelector(
'complex-slotted-group'
) as ComplexSlottedGroup
).menu;
}
protected override render(): TemplateResult {
return html`
<complex-slotted-group id="group">
<sp-menu-item id="i-5">Middle 1</sp-menu-item>
<sp-menu-item id="i-6">Middle 2</sp-menu-item>
<sp-menu-item id="i-7">Middle 3</sp-menu-item>
<slot></slot>
<slot name="before" slot="before"></slot>
<sp-menu-item slot="before" id="i-3">Before Last</sp-menu-item>
</complex-slotted-group>
`;
}
}

customElements.define('complex-slotted-menu', ComplexSlottedMenu);
12 changes: 12 additions & 0 deletions packages/menu/stories/menu-group.stories.ts
View file
Expand Up @@ -20,11 +20,23 @@ import '@spectrum-web-components/menu/sp-menu-item.js';
import '@spectrum-web-components/menu/sp-menu-divider.js';
import '@spectrum-web-components/menu/sp-menu-group.js';

import './index.js';

export default {
component: 'sp-menu',
title: 'Menu Group',
};

export const complexSlotted = (): TemplateResult => {
return html`
<complex-slotted-menu>
<sp-menu-item slot="before" id="i-2">External A</sp-menu-item>
<sp-menu-item id="i-8">External 1</sp-menu-item>
<sp-menu-item id="i-9">External 2</sp-menu-item>
</complex-slotted-menu>
`;
};

export const mixed = (): TemplateResult => {
let style = 'italic';
let weight = '700';
Expand Down
74 changes: 74 additions & 0 deletions packages/menu/test/menu-group.test.ts
View file
Expand Up @@ -23,13 +23,23 @@ import {
waitUntil,
} from '@open-wc/testing';
import { testForLitDevWarnings } from '../../../test/testing-helpers.js';
import { complexSlotted } from '../stories/menu-group.stories.js';
import { ComplexSlottedGroup, ComplexSlottedMenu } from '../stories/index.js';
import { sendKeys } from '@web/test-runner-commands';
import { sendMouse } from '../../../test/plugins/browser.js';

const managedItems = (menu: Menu | MenuGroup): MenuItem[] => {
return menu.childItems.filter(
(item: MenuItem) => item.menuData.selectionRoot === menu
);
};

const focusableItems = (menu: Menu | MenuGroup): MenuItem[] => {
return menu.childItems.filter(
(item: MenuItem) => item.menuData.focusRoot === menu
);
};

describe('Menu group', () => {
testForLitDevWarnings(
async () =>
Expand Down Expand Up @@ -389,4 +399,68 @@ describe('Menu group', () => {
expect(subInheritItem1.getAttribute('role')).to.equal('menuitemradio');
expect(subInheritItem2.getAttribute('role')).to.equal('menuitemradio');
});
it('manages complex slotted menu items', async () => {
const el = await fixture<ComplexSlottedMenu>(complexSlotted());

await waitUntil(
() => focusableItems(el.menu).length == 12,
`expected outer menu to manage 12 items, ${
el.menu.localName
} manages ${focusableItems(el.menu).length}`
);

const items: Record<string, MenuItem> = {};
items.i2 = el.querySelector('#i-2') as MenuItem;
items.i8 = el.querySelector('#i-8') as MenuItem;
items.i9 = el.querySelector('#i-9') as MenuItem;
items.i3 = el.renderRoot.querySelector('#i-3') as MenuItem;
items.i5 = el.renderRoot.querySelector('#i-5') as MenuItem;
items.i6 = el.renderRoot.querySelector('#i-6') as MenuItem;
items.i7 = el.renderRoot.querySelector('#i-7') as MenuItem;
const group = el.renderRoot.querySelector(
'#group'
) as ComplexSlottedGroup;
items.i1 = group.renderRoot.querySelector('#i-1') as MenuItem;
items.i4 = group.renderRoot.querySelector('#i-4') as MenuItem;
items.i10 = group.renderRoot.querySelector('#i-10') as MenuItem;
items.i11 = group.renderRoot.querySelector('#i-11') as MenuItem;
items.i12 = group.renderRoot.querySelector('#i-12') as MenuItem;

const rect = items.i9.getBoundingClientRect();
await sendMouse({
steps: [
{
position: [
rect.left + rect.width / 2,
rect.top + rect.height / 2,
],
type: 'click',
},
],
});
await elementUpdated(items.i9);

await sendKeys({
press: 'ArrowDown',
});
await sendKeys({
press: 'ArrowUp',
});
await elementUpdated(items.i9);
expect(items.i9.focused).to.be.true;
await sendKeys({
press: 'ArrowDown',
});
let i = 9;
const count = Object.keys(items).length + 1;
while (!items.i9.focused) {
i = Math.max(1, (i + 1 + count) % count);
await elementUpdated(items[`i${i}`]);
expect(items[`i${i}`].focused, `i${i} should be focused`).to.be
.true;
await sendKeys({
press: 'ArrowDown',
});
}
});
});

0 comments on commit 7f9ad69

Please sign in to comment.