Skip to content

Commit

Permalink
fix(overlay): place longpress helper content in a more accessible, le…
Browse files Browse the repository at this point in the history 
…ss layout affecting location
  • Loading branch information
@Westbrook
Westbrook committed Oct 27, 2023
1 parent bdd7424 commit dd12c23
Show file tree
Hide file tree
Showing 8 changed files with 631 additions and 523 deletions.
16 changes: 15 additions & 1 deletion packages/overlay/src/Overlay.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,21 @@ export class Overlay extends OverlayFeatures {
const messageType = isIOS() || isAndroid() ? 'touch' : 'keyboard';
longpressDescription.textContent = LONGPRESS_INSTRUCTIONS[messageType];
longpressDescription.slot = 'longpress-describedby-descriptor';
trigger.insertAdjacentElement('afterend', longpressDescription);
const triggerParent = trigger.getRootNode() as HTMLElement;
const overlayParent = this.getRootNode() as HTMLElement;
// Manage the placement of the helper element in an accessible place with
// the lowest chance of negatively affecting the layout of the page.
if (triggerParent === overlayParent) {
// Trigger and Overlay in same DOM tree...
// Append helper element to Overlay.
this.append(longpressDescription);
} else {
// If Trigger in <body>, hide helper
longpressDescription.hidden = !('host' in triggerParent);
// Trigger and Overlay in different DOM tree, Trigger in shadow tree...
// Insert helper element after Trigger.
trigger.insertAdjacentElement('afterend', longpressDescription);
}

const releaseLongpressDescribedby = conditionAttributeWithId(
trigger,
Expand Down
34 changes: 23 additions & 11 deletions packages/overlay/test/overlay-trigger-click.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ import {
import '@spectrum-web-components/overlay/overlay-trigger.js';
import { spy } from 'sinon';
import { ActionButton } from '@spectrum-web-components/action-button';
import {
fixture,
isInteractive,
isOnTopLayer,
} from '../../../test/testing-helpers.js';
import { fixture, isOnTopLayer } from '../../../test/testing-helpers.js';

describe('Overlay Trigger - Click', () => {
it('displays `click` declaratively', async () => {
Expand Down Expand Up @@ -72,26 +68,35 @@ describe('Overlay Trigger - Click', () => {
});
});
describe('closes on scroll', () => {
afterEach(() => {
afterEach(async () => {
if (document.scrollingElement) {
document.scrollingElement.scrollTop = 0;
}
await waitUntil(() => {
if (document.scrollingElement) {
return document.scrollingElement.scrollTop === 0;
}
return true;
});
});
(['click', 'replace', 'inline'] as TriggerInteractionsV1[]).map(
(interaction) => {
it(`closes "${interaction}" overlay on scroll`, async () => {
it(`closes "${interaction}" overlay on scroll`, async function () {
const el = await fixture<OverlayTrigger>(html`
<overlay-trigger
placement="right-start"
placement="right"
type=${interaction}
content="click"
>
<sp-action-button
slot="trigger"
style="margin: 50vh 0 100vh;"
>
<sp-icon-magnify slot="icon"></sp-icon-magnify>
</sp-action-button>
<sp-popover slot="click-content" tip></sp-popover>
<sp-popover slot="click-content" tip>
Content
</sp-popover>
</overlay-trigger>
`);
await nextFrame();
Expand All @@ -100,19 +105,26 @@ describe('Overlay Trigger - Click', () => {

await elementUpdated(el);
const opened = oneEvent(el, 'sp-opened');
el.open = 'click';
const trigger = el.querySelector(
'sp-action-button'
) as HTMLElement;
trigger.click();

await opened;

expect(el.open).to.equal('click');
expect(await isInteractive(popover)).to.be.true;

expect(await isOnTopLayer(popover)).to.be.true;

const closed = oneEvent(el, 'sp-closed');
if (document.scrollingElement) {
document.scrollingElement.scrollTop = 100;
}

await closed;

expect(el.open).to.be.undefined;

expect(await isOnTopLayer(popover)).to.be.false;
});
}
Expand Down
49 changes: 32 additions & 17 deletions packages/overlay/test/overlay-trigger-longpress.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import '@spectrum-web-components/action-button/sp-action-button.js';
import '@spectrum-web-components/button/sp-button.js';
import '@spectrum-web-components/action-group/sp-action-group.js';
import '@spectrum-web-components/icons-workflow/icons/sp-icon-magnify.js';
import { Popover } from '@spectrum-web-components/popover';
import type { Popover } from '@spectrum-web-components/popover';
import type { Tooltip } from '@spectrum-web-components/tooltip';
import '@spectrum-web-components/popover/sp-popover.js';
import {
LONGPRESS_INSTRUCTIONS,
Expand All @@ -44,6 +45,9 @@ describe('Overlay Trigger - Longpress', () => {
this.trigger = this.el.querySelector(
'sp-action-button'
) as ActionButton;
this.tooltip = this.el.querySelector(
'[slot="hover-content"]'
) as Tooltip;
this.content = this.el.querySelector(
'[slot="longpress-content"]'
) as Popover;
Expand Down Expand Up @@ -183,13 +187,14 @@ describe('Overlay Trigger - Longpress', () => {
expect(this.content.open, 'closes for `pointerdown`').to.be.false;
});
it('opens/closes for `longpress` with Button', async function () {
this.tooltip.placement = 'bottom-start';
await elementUpdated(this.tooltip);
const button = document.createElement('sp-button');
button.slot = 'trigger';
this.trigger.remove();
await elementUpdated(this.el);
this.el.append(button);
await elementUpdated(this.el);
await nextFrame();
await nextFrame();

let open = oneEvent(this.el, 'sp-opened');
const rect = button.getBoundingClientRect();
Expand All @@ -202,16 +207,18 @@ describe('Overlay Trigger - Longpress', () => {
rect.top + rect.height / 2,
],
},
{
type: 'down',
},
],
});
// Hover content opens, first.
await open;
await nextFrame();
await nextFrame();
open = oneEvent(this.el, 'sp-opened');
await sendMouse({
steps: [
{
type: 'down',
},
],
});
// Then, the longpress content opens.
await open;
await nextFrame();
Expand Down Expand Up @@ -311,12 +318,10 @@ describe('Overlay Trigger - Longpress', () => {

expect(trigger.hasAttribute('aria-describedby')).to.be.true;
expect(el.open).to.be.undefined;
/*
* This test passes because `<sp-overlay>` adds a new node to describe
* the longpress interaction now available on the trigger element
*/
expect(el.childNodes.length, 'always').to.equal(6);

let longpressHelper = el.querySelector(
'[slot="longpress-describedby-descriptor"]'
) as HTMLElement;
expect(longpressHelper).to.not.be.null;
await findDescribedNode(
'Trigger with hold affordance',
LONGPRESS_INSTRUCTIONS.keyboard
Expand All @@ -329,7 +334,10 @@ describe('Overlay Trigger - Longpress', () => {
await opened;

expect(el.open).to.equal('longpress');
expect(el.childNodes.length, 'always').to.equal(6);
longpressHelper = el.querySelector(
'[slot="longpress-describedby-descriptor"]'
) as HTMLElement;
expect(longpressHelper).to.not.be.null;

await findDescribedNode(
'Trigger with hold affordance',
Expand All @@ -344,7 +352,10 @@ describe('Overlay Trigger - Longpress', () => {

expect(el.open).to.be.undefined;
expect(trigger.hasAttribute('aria-describedby')).to.be.true;
expect(el.childNodes.length, 'always').to.equal(6);
longpressHelper = el.querySelector(
'[slot="longpress-describedby-descriptor"]'
) as HTMLElement;
expect(longpressHelper).to.not.be.null;

await findDescribedNode(
'Trigger with hold affordance',
Expand Down Expand Up @@ -388,7 +399,11 @@ describe('Overlay Trigger - Longpress', () => {
trigger.hasAttribute('aria-describedby'),
'applies described by content'
).to.be.true;
expect(el.childNodes.length, 'always').to.equal(6);

const longpressHelper = el.querySelector(
'[slot="longpress-describedby-descriptor"]'
) as HTMLElement;
expect(longpressHelper).to.not.be.null;

el.removeAttribute('hold-affordance');
content.remove();
Expand Down
Loading

0 comments on commit dd12c23

Please sign in to comment.