Skip to content

Commit

Permalink
feat(picker): add forcePopover property (#5041)
Browse files Browse the repository at this point in the history
Co-authored-by: Casey Eickhoff <ceickhoff@adobe.com>
  • Loading branch information
rubencarvalho and caseyisonit authored Jan 28, 2025
1 parent 7a5f786 commit 3651e57
Show file tree
Hide file tree
Showing 11 changed files with 449 additions and 44 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ parameters:
# 3. Commit this change to the PR branch where the changes exist.
current_golden_images_hash:
type: string
default: 93ad92a2fc031db0277fee635434955e438e94a9
default: 266fb37749bc59cbde0e81148dddea4d1bae2053
wireit_cache_name:
type: string
default: wireit
Expand Down
34 changes: 29 additions & 5 deletions packages/action-menu/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ In order to deliver an `<sp-action-menu>` without an icon, use the `label-only`

### No visible label

The visible label that is be provided via the default `<slot>` interface can be ommitted in preference of an icon only interface. In this context be sure that the `<sp-action-menu>` continued to be accessible to screen readers by applying the `label` attribute. This will apply an `aria-label` attribute of the same value to the `<button>` element that toggles the menu list.
The visible label that is be provided via the default `<slot>` interface can be omitted in preference of an icon only interface. In this context be sure that the `<sp-action-menu>` continued to be accessible to screen readers by applying the `label` attribute. This will apply an `aria-label` attribute of the same value to the `<button>` element that toggles the menu list.

<!-- prettier-ignore -->
```html
Expand Down Expand Up @@ -249,7 +249,7 @@ A custom icon can be supplied via the `icon` slot in order to replace the defaul
</sp-action-menu>
```

### Selection.
### Selection

When `selects` is set to `single`, the `<sp-action-menu>` element will maintain one selected item after an initial selection is made.

Expand All @@ -272,13 +272,33 @@ When `selects` is set to `single`, the `<sp-action-menu>` element will maintain
</sp-action-menu>
```

## Accessibility
## Force Popover on Mobile Devices

An `<sp-action-menu>` parent will ensure that the internal `<sp-menu>` features a role of `listbox` and contains children with the role `option`. Upon focusing the `<sp-action-menu>` using `ArrowDown` will also open the menu while throwing focus into first selected (or unselected when none are selected) menu item to assist in selecting of a new value.
On mobile, the menu can be exposed in either a `sp-popover` or `sp-tray`. By default, `sp-action-menu` will render an `sp-tray`. If you would like to render `sp-popover` on mobile, add the attribute `forcePopover` to the `sp-action-menu`.

Usage Guidance:

- Use a tray when a menu’s proximity to its trigger is considered to be less important to the experience, or for showing a volume of content that is too overwhelming for a popover.
- Use a popover when a menu’s proximity to its trigger is considered to be important to the experience, or for showing a volume of content that is manageable for a popover.

To see this functionality in action, load this page from your mobile device or use Chrome DevTools (or equivalent) and select a mobile device once the Device Toolbar (the phone/tablet icon) is active.

```html
<sp-action-menu forcePopover>
<span slot="label">Action Menu</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>
```

## Adding tootip in action menu

Tooltip in action menu can be attached via adding <sp-tooltip> and can be customized by using various parameters (e.g. placement, content, etc) as needed.
Tooltip in action menu can be attached via adding `<sp-tooltip>` and can be customized by using various parameters (e.g. placement, content, etc) as needed.

```html
<sp-action-menu>
Expand All @@ -291,3 +311,7 @@ Tooltip in action menu can be attached via adding <sp-tooltip> and can be custom
<sp-menu-item value="shape-3-parallelogram">Parallelogram</sp-menu-item>
</sp-action-menu>
```

## Accessibility

An `<sp-action-menu>` parent will ensure that the internal `<sp-menu>` features a role of `listbox` and contains children with the role `option`. Upon focusing the `<sp-action-menu>` using `ArrowDown` will also open the menu while throwing focus into first selected (or unselected when none are selected) menu item to assist in selecting of a new value.
44 changes: 37 additions & 7 deletions packages/action-menu/stories/action-menu.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@ import { html, TemplateResult } from '@spectrum-web-components/base';
import { ifDefined } from '@spectrum-web-components/base/src/directives.js';

import '@spectrum-web-components/action-menu/sp-action-menu.js';
import '@spectrum-web-components/menu/sp-menu.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import '@spectrum-web-components/menu/sp-menu-group.js';
import '@spectrum-web-components/menu/sp-menu-divider.js';
import '@spectrum-web-components/tooltip/sp-tooltip.js';
import '@spectrum-web-components/menu/sp-menu-group.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import '@spectrum-web-components/menu/sp-menu.js';
import { slottableRequest } from '@spectrum-web-components/overlay/src/slottable-request-directive.js';
import { ActionMenuMarkup } from './';
import '@spectrum-web-components/tooltip/sp-tooltip.js';
import { makeOverBackground } from '../../button/stories/index.js';
import { isOverlayOpen } from '../../overlay/stories/index.js';
import { ActionMenuMarkup } from './';

import type { ActionMenu } from '@spectrum-web-components/action-menu';
import '@spectrum-web-components/icons-workflow/icons/sp-icon-settings.js';
import { Menu } from '@spectrum-web-components/menu';
import type { MenuItem } from '@spectrum-web-components/menu/src/MenuItem.js';
import { Placement } from '@spectrum-web-components/overlay/src/overlay-types.js';
import { Menu } from '@spectrum-web-components/menu';
import type { ActionMenu } from '@spectrum-web-components/action-menu';

export default {
component: 'sp-action-menu',
Expand Down Expand Up @@ -160,6 +160,7 @@ export default {
align: 'start',
visibleLabel: 'More Actions',
disabled: false,
forcePopover: false,
open: false,
quiet: false,
tooltipDescription: '',
Expand All @@ -180,6 +181,7 @@ interface StoryArgs {
staticValue?: 'white' | 'black' | undefined;
tooltipDescription?: string | 'none';
tooltipPlacement?: Placement;
forcePopover?: boolean;
}

const Template = (args: StoryArgs = {}): TemplateResult =>
Expand All @@ -204,6 +206,34 @@ quiet.args = {
quiet: true,
};

export const forcePopoverOnMobile = (): TemplateResult => html`
<h1>Force Popover on Mobile</h1>
<p>
The forcePopover attribute overrides the mobile device functionality of
rendering a tray so that a popover will always render no matter the
device.
</p>
<ol>
<li>Open Chrome DevTools (or equivalent).</li>
<li>Toggle the Device Toolbar (the phone/tablet icon).</li>
<li>Select a device preset (e.g. iPhone 12).</li>
<li>
Chrome will set user-agent strings, simulate touch, and adjust DPI.
</li>
<li>Reload the page</li>
<li>Click the Action Menu and see a popover</li>
</ol>
<sp-action-menu forcePopover>
<span slot="label">Action Menu</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>
`;
export const labelOnly = ({
align = 'start',
changeHandler = (() => undefined) as (event: Event) => void,
Expand Down
8 changes: 5 additions & 3 deletions packages/action-menu/stories/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,24 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/

import { ifDefined } from '@spectrum-web-components/base/src/directives.js';
import { html, nothing, TemplateResult } from '@spectrum-web-components/base';
import { ifDefined } from '@spectrum-web-components/base/src/directives.js';

import type { ActionMenu } from '@spectrum-web-components/action-menu';
import '@spectrum-web-components/action-menu/sp-action-menu.js';
import '@spectrum-web-components/icon/sp-icon.js';
import '@spectrum-web-components/menu/sp-menu-divider.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import '@spectrum-web-components/tooltip/sp-tooltip.js';
import { Placement } from '@spectrum-web-components/overlay/src/overlay-types.js';
import type { ActionMenu } from '@spectrum-web-components/action-menu';
import '@spectrum-web-components/tooltip/sp-tooltip.js';

export const ActionMenuMarkup = ({
align = 'start',
ariaLabel = 'More Actions',
onChange = (() => undefined) as (value: string) => void,
changeHandler = (() => undefined) as (value: string) => void,
disabled = false,
forcePopover = false,
open = false,
quiet = false,
staticValue = '',
Expand All @@ -44,6 +45,7 @@ export const ActionMenuMarkup = ({
?disabled=${disabled}
?open=${open}
?quiet=${quiet}
?forcePopover=${forcePopover}
static-color=${ifDefined(
staticValue === 'none'
? undefined
Expand Down
181 changes: 181 additions & 0 deletions packages/action-menu/test/action-menu-responsive.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
/*
Copyright 2020 Adobe. All rights reserved.
This file is licensed to you under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License. You may obtain a copy
of the License at http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software distributed under
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
OF ANY KIND, either express or implied. See the License for the specific language
governing permissions and limitations under the License.
*/

import {
elementUpdated,
expect,
fixture,
html,
nextFrame,
oneEvent,
} from '@open-wc/testing';
import { ActionMenu } from '@spectrum-web-components/action-menu';
import '@spectrum-web-components/action-menu/sync/sp-action-menu.js';
import '@spectrum-web-components/menu/sp-menu-divider.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import { setViewport } from '@web/test-runner-commands';
import { spreadProps } from '../../../test/lit-helpers.js';
import { sendMouse } from '../../../test/plugins/browser.js';

describe('ActionMenu, responsive', () => {
let el: ActionMenu;
const actionMenuFixture = async (args?: {
forcePopover: boolean;
}): Promise<ActionMenu> => {
const test = await fixture<HTMLDivElement>(html`
<div>
<sp-action-menu id="action-menu" ${spreadProps(args || {})}>
<span slot="label">Action Menu</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>
</div>
`);

return test.querySelector('sp-action-menu') as ActionMenu;
};

describe('container', () => {
beforeEach(async () => {
el = await actionMenuFixture();
await elementUpdated(el);
});

it('is a Tray in mobile', async () => {
/**
* This is a hack to set the `isMobile` property to true so that we can test the MobileController
*/
el.isMobile.matches = true;
el.bindEvents();

/**
* While we can set the view port, but not `(hover: none) and (pointer: coarse)`
* which prevents us from testing this at unit time. Hopefully there will be
* a future version of Playwright and/or @web/test-runner that does allow this.
* See: https://github.com/microsoft/playwright/issues/11781
**/
await setViewport({ width: 360, height: 640 });
// Allow viewport update to propagate.
await nextFrame();

const opened = oneEvent(el, 'sp-opened');

const boundingRect = el.button.getBoundingClientRect();
sendMouse({
steps: [
{
type: 'click',
position: [
boundingRect.x + boundingRect.width / 2,
boundingRect.y + boundingRect.height / 2,
],
},
],
});

await opened;

const tray = el.shadowRoot.querySelector('sp-tray');
const popover = el.shadowRoot.querySelector('sp-popover');

expect(tray).to.not.be.null;
expect(popover).to.be.null;
});

it('is a Popover in desktop', async () => {
await setViewport({ width: 701, height: 640 });
// Allow viewport update to propagate.
await nextFrame();
await nextFrame();

const opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;

const popover = el.shadowRoot.querySelector('sp-popover');
const tray = el.shadowRoot.querySelector('sp-tray');

expect(popover).to.not.be.null;
expect(tray).to.be.null;
});
});

describe('forcePopover', () => {
beforeEach(async () => {
el = await actionMenuFixture({ forcePopover: true });
await elementUpdated(el);
});

it('is a Popover in mobile', async () => {
/**
* This is a hack to set the `isMobile` property to true so that we can test the MobileController
*/
el.isMobile.matches = true;
el.bindEvents();

/**
* While we can set the view port, but not `(hover: none) and (pointer: coarse)`
* which prevents us from testing this at unit time. Hopefully there will be
* a future version of Playwright and/or @web/test-runner that does allow this.
* See: https://github.com/microsoft/playwright/issues/11781
**/
await setViewport({ width: 360, height: 640 });
// Allow viewport update to propagate.
await nextFrame();

const opened = oneEvent(el, 'sp-opened');

const boundingRect = el.button.getBoundingClientRect();
sendMouse({
steps: [
{
type: 'click',
position: [
boundingRect.x + boundingRect.width / 2,
boundingRect.y + boundingRect.height / 2,
],
},
],
});

await opened;

const tray = el.shadowRoot.querySelector('sp-tray');
const popover = el.shadowRoot.querySelector('sp-popover');

expect(tray).to.be.null;
expect(popover).to.not.be.null;
});

it('is a Popover in desktop', async () => {
await setViewport({ width: 701, height: 640 });
// Allow viewport update to propagate.
await nextFrame();
await nextFrame();

const opened = oneEvent(el, 'sp-opened');
el.open = true;
await opened;

const popover = el.shadowRoot.querySelector('sp-popover');
const tray = el.shadowRoot.querySelector('sp-tray');

expect(popover).to.not.be.null;
expect(tray).to.be.null;
});
});
});
6 changes: 2 additions & 4 deletions packages/overlay/test/overlay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,6 @@ describe('Overlay - type="modal"', () => {
expect(document.activeElement === trigger, 'trigger focused').to.be
.true;
});

});
describe('Overlay - timing', () => {
it('manages multiple modals in a row without preventing them from closing', async () => {
Expand Down Expand Up @@ -972,8 +971,7 @@ describe('Overlay - timing', () => {

describe('maintains focus consistency in webkit', () => {
it('should apply remove-focus-ring class in webkit when focus happens after click', async () => {

if (!isWebKit()){
if (!isWebKit()) {
return;
}

Expand Down Expand Up @@ -1017,4 +1015,4 @@ describe('maintains focus consistency in webkit', () => {

expect(trigger.classList.contains(SAFARI_FOCUS_RING_CLASS)).to.be.true;
});
});
});
Loading

0 comments on commit 3651e57

Please # to comment.