fix(queue): draw the scrim only where it can be tapped
Below 600px `.panel-content` is `width: 100%`, so the scrim sat entirely underneath an opaque panel -- measured at 424x439, host, panel and scrim all 424x318. It dimmed nothing and dismissed nothing there while wearing `cursor: pointer`, so #24's tap-outside-to-close did not exist on the device it was drawn for. Of the issue's two directions this takes the second. A gutter is the drawer pattern and buys the affordance by taking width off a full-screen surface on a 424px viewport; #55 already made the queue a *screen* at that width, whose ways out are back and a 44px close button. So there is no scrim there rather than an unreachable one. Existence is `matchMedia` rather than `display: none`, on `job-band`'s rule: a hidden scrim is still an element carrying the handler. The 600-899 band, where the panel is a 320px column of a wider content area and the scrim has real uncovered pixels, is untouched. The e2e half asserts *absence* at 424x439 rather than clicking, because a phone-width case that clicks the scrim's centre hits the panel and passes on the broken build -- which the issue anticipates. Closes #171
This commit is contained in:
@@ -2006,9 +2006,26 @@ one that closes the queue — the reported defect moved one press later,
|
|||||||
which looks exactly like a press that did nothing.
|
which looks exactly like a press that did nothing.
|
||||||
|
|
||||||
**And the way out is 44px on a phone.** With the panel spanning the
|
**And the way out is 44px on a phone.** With the panel spanning the
|
||||||
whole width the scrim has no uncovered pixels at all, so the close
|
whole width there is no scrim there at all, so the close button is the
|
||||||
button is the only pointer route out of a full-screen surface; it was
|
only pointer route out of a full-screen surface; it was **25×21px**.
|
||||||
**25×21px**.
|
|
||||||
|
**The scrim is drawn only where it can be tapped** (#171). Below 600px
|
||||||
|
`.panel-content` is `width: 100%`, so the scrim sat entirely underneath
|
||||||
|
an opaque panel — measured at 424×439, host, panel and scrim all
|
||||||
|
424×318 — dimming nothing and dismissing nothing while wearing
|
||||||
|
`cursor: pointer`. #24's tap-outside-to-close cannot exist on a surface
|
||||||
|
with no outside, and the screen above is what answers it instead: back,
|
||||||
|
and a 44px close button. The alternative — a gutter, which is the
|
||||||
|
drawer pattern — was declined, because it buys the affordance by taking
|
||||||
|
width off a full-screen surface on a 424px viewport. Two things about
|
||||||
|
it are load-bearing. Its **existence** is `matchMedia`, not
|
||||||
|
`display: none`, on `job-band`'s rule: a hidden scrim is still an
|
||||||
|
element carrying the dismissal handler. And **the 600–899 band is
|
||||||
|
untouched**, where the panel is a 320px column of a wider content area
|
||||||
|
and the scrim has real uncovered pixels — which is why the e2e half
|
||||||
|
asserts *absence* at 424×439 rather than clicking, since a phone-width
|
||||||
|
case that clicks the scrim's centre hits the panel and passes on the
|
||||||
|
broken build.
|
||||||
|
|
||||||
What this does **not** fix is `page-header` overflowing on its own:
|
What this does **not** fix is `page-header` overflowing on its own:
|
||||||
at 900×600 "New Smart Playlist" is still clipped to 114 of 162px with
|
at 900×600 "New Smart Playlist" is still clipped to 114 of 162px with
|
||||||
|
|||||||
@@ -179,8 +179,8 @@ test.describe('the queue is a screen where it covers the content', () => {
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* With the panel spanning the whole width the scrim has no uncovered
|
* With the panel spanning the whole width there is no scrim here at
|
||||||
* pixels, so the close button is the only pointer route out of a
|
* all (#171), so the close button is the only pointer route out of a
|
||||||
* full-screen surface. Measured at 424×439 before #55: **25×21px**.
|
* full-screen surface. Measured at 424×439 before #55: **25×21px**.
|
||||||
*/
|
*/
|
||||||
test('offers a way out a thumb can hit', async ({ app }) => {
|
test('offers a way out a thumb can hit', async ({ app }) => {
|
||||||
@@ -194,6 +194,33 @@ test.describe('the queue is a screen where it covers the content', () => {
|
|||||||
expect(box!.width).toBeGreaterThanOrEqual(44);
|
expect(box!.width).toBeGreaterThanOrEqual(44);
|
||||||
expect(box!.height).toBeGreaterThanOrEqual(44);
|
expect(box!.height).toBeGreaterThanOrEqual(44);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #171 — and it draws no scrim, because there is nowhere to tap.
|
||||||
|
*
|
||||||
|
* `.panel-content` is `width: 100%` here, so the scrim sat entirely
|
||||||
|
* underneath it: measured at 424×439, host, panel and scrim all
|
||||||
|
* 424×318. #24's tap-outside-to-close cannot exist on a surface with
|
||||||
|
* no outside, and a `cursor: pointer` layer nobody can reach is a
|
||||||
|
* claim the component cannot keep.
|
||||||
|
*
|
||||||
|
* Asserted as absence rather than by clicking, for the reason the
|
||||||
|
* issue gives: a naive phone case clicks the scrim's centre and hits
|
||||||
|
* the panel, so it passes on the build this exists to fail. The scrim
|
||||||
|
* is still real between 600 and 899px, which `queue-overlay.spec.ts`
|
||||||
|
* asserts at 900×600 by clicking it.
|
||||||
|
*/
|
||||||
|
test('draws no scrim, because a screen has no outside to tap', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
await openTheQueue(app);
|
||||||
|
|
||||||
|
const scrim = await queue(app).evaluate(
|
||||||
|
(el) => el.shadowRoot!.querySelector('.scrim') !== null,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(scrim).toBe(false);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import type { MenuSurface } from '../menu-surface/menu-surface';
|
|||||||
import '../menu-surface/menu-surface';
|
import '../menu-surface/menu-surface';
|
||||||
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
import '@awesome.me/webawesome/dist/components/dropdown-item/dropdown-item.js';
|
||||||
import { QueueController } from '@store/controllers/queue-controller';
|
import { QueueController } from '@store/controllers/queue-controller';
|
||||||
|
import { PHONE_QUERY } from '@utils/breakpoints';
|
||||||
import { creditStore } from '@store/credit-store';
|
import { creditStore } from '@store/credit-store';
|
||||||
import {
|
import {
|
||||||
describeQueueSource,
|
describeQueueSource,
|
||||||
@@ -120,6 +121,25 @@ export class QueuePanel
|
|||||||
@property({ type: Boolean, reflect: true })
|
@property({ type: Boolean, reflect: true })
|
||||||
overlay = false;
|
overlay = false;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Phone width, from `matchMedia` rather than from a media query,
|
||||||
|
* because it decides whether the scrim *exists* (#171) —
|
||||||
|
* `job-band`'s rule, and a stylesheet cannot express it: a
|
||||||
|
* `display: none` scrim is still an element with a click handler.
|
||||||
|
*
|
||||||
|
* Below 600px the panel spans the whole content area, so the scrim
|
||||||
|
* has no uncovered pixels: measured at 424x439, host, panel and
|
||||||
|
* scrim are all 424x318 with the scrim entirely underneath. It dims
|
||||||
|
* nothing and dismisses nothing there, and the queue is a *screen*
|
||||||
|
* at that width anyway (#55) — back and a 44px close button are its
|
||||||
|
* ways out. Between 600 and 899 the panel is a 320px column of a
|
||||||
|
* wider content area, the scrim is reachable, and #24's
|
||||||
|
* tap-outside-to-close is real; that band is untouched.
|
||||||
|
*/
|
||||||
|
@state() private phone = false;
|
||||||
|
|
||||||
|
private phoneQuery?: MediaQueryList;
|
||||||
|
|
||||||
@state()
|
@state()
|
||||||
private isDragging = false;
|
private isDragging = false;
|
||||||
|
|
||||||
@@ -391,6 +411,8 @@ export class QueuePanel
|
|||||||
display: none;
|
display: none;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Overlay only, and above 600px only -- see the phone field,
|
||||||
|
which is where that half is decided (#171). */
|
||||||
.scrim {
|
.scrim {
|
||||||
position: absolute;
|
position: absolute;
|
||||||
inset: 0;
|
inset: 0;
|
||||||
@@ -414,10 +436,10 @@ export class QueuePanel
|
|||||||
|
|
||||||
/* A screen's way out has to be hittable with a thumb.
|
/* A screen's way out has to be hittable with a thumb.
|
||||||
Measured at 424x439 before #55: these were **25x21px**,
|
Measured at 424x439 before #55: these were **25x21px**,
|
||||||
and with the panel spanning the whole width the scrim
|
and with the panel spanning the whole width there is no
|
||||||
underneath has no uncovered pixels at all -- so it was
|
scrim here at all (#171) -- so this is the only pointer
|
||||||
the only pointer route out of a full-screen surface.
|
route out of a full-screen surface. Back answers it now
|
||||||
Back answers it now as well, which is the other half.
|
as well, which is the other half.
|
||||||
|
|
||||||
Sized only in overlay mode: inline these sit in a 320px
|
Sized only in overlay mode: inline these sit in a 320px
|
||||||
column beside the content, where a mouse is what reaches
|
column beside the content, where a mouse is what reaches
|
||||||
@@ -828,6 +850,12 @@ export class QueuePanel
|
|||||||
// desktop width rather than the minimum.
|
// desktop width rather than the minimum.
|
||||||
this.updateOverlayMode();
|
this.updateOverlayMode();
|
||||||
|
|
||||||
|
// Read here rather than in a field initialiser, so a test can
|
||||||
|
// install its own matchMedia before the element is created.
|
||||||
|
this.phoneQuery = window.matchMedia?.(PHONE_QUERY);
|
||||||
|
this.phone = this.phoneQuery?.matches ?? false;
|
||||||
|
this.phoneQuery?.addEventListener('change', this.onPhoneMedia);
|
||||||
|
|
||||||
if (this.parentElement) {
|
if (this.parentElement) {
|
||||||
this.spaceObserver = new ResizeObserver(() =>
|
this.spaceObserver = new ResizeObserver(() =>
|
||||||
this.updateOverlayMode(),
|
this.updateOverlayMode(),
|
||||||
@@ -870,6 +898,8 @@ export class QueuePanel
|
|||||||
this.creditsUnsub = undefined;
|
this.creditsUnsub = undefined;
|
||||||
this.spaceObserver?.disconnect();
|
this.spaceObserver?.disconnect();
|
||||||
this.spaceObserver = undefined;
|
this.spaceObserver = undefined;
|
||||||
|
this.phoneQuery?.removeEventListener('change', this.onPhoneMedia);
|
||||||
|
this.phoneQuery = undefined;
|
||||||
document.removeEventListener('keydown', this.onOverlayKeydown);
|
document.removeEventListener('keydown', this.onOverlayKeydown);
|
||||||
document.removeEventListener(
|
document.removeEventListener(
|
||||||
'mousemove',
|
'mousemove',
|
||||||
@@ -936,6 +966,10 @@ export class QueuePanel
|
|||||||
this.overlay = available - this.panelWidth < MAIN_PANEL_FLOOR;
|
this.overlay = available - this.panelWidth < MAIN_PANEL_FLOOR;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
private onPhoneMedia = (e: MediaQueryListEvent): void => {
|
||||||
|
this.phone = e.matches;
|
||||||
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Escape closes a scrimmed overlay, which is the one keyboard rule
|
* Escape closes a scrimmed overlay, which is the one keyboard rule
|
||||||
* every dialog in this app already follows.
|
* every dialog in this app already follows.
|
||||||
@@ -1972,7 +2006,7 @@ export class QueuePanel
|
|||||||
const tracks = this.queue.tracks;
|
const tracks = this.queue.tracks;
|
||||||
|
|
||||||
return html`
|
return html`
|
||||||
${this.overlay
|
${this.overlay && !this.phone
|
||||||
? html`<div
|
? html`<div
|
||||||
class="scrim"
|
class="scrim"
|
||||||
part="scrim"
|
part="scrim"
|
||||||
|
|||||||
@@ -29,8 +29,38 @@ import { shadow } from '@test/support/render';
|
|||||||
|
|
||||||
const wrappers: HTMLElement[] = [];
|
const wrappers: HTMLElement[] = [];
|
||||||
|
|
||||||
|
let restoreMedia: (() => void) | null = null;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Answer the shell's phone query with `phone` until restored.
|
||||||
|
*
|
||||||
|
* Stubbed rather than emulated, for the reason `search-dialog.test.ts`
|
||||||
|
* gives: the runner's viewport is fixed at 1280x800, and the panel
|
||||||
|
* reads `matchMedia` in `connectedCallback` precisely so a test can
|
||||||
|
* answer it first.
|
||||||
|
*/
|
||||||
|
function stubPhone(phone: boolean): void {
|
||||||
|
const real = window.matchMedia.bind(window);
|
||||||
|
|
||||||
|
window.matchMedia = ((q: string) =>
|
||||||
|
q.includes('max-width: 599px')
|
||||||
|
? {
|
||||||
|
matches: phone,
|
||||||
|
media: q,
|
||||||
|
addEventListener() {},
|
||||||
|
removeEventListener() {},
|
||||||
|
}
|
||||||
|
: real(q)) as typeof window.matchMedia;
|
||||||
|
|
||||||
|
restoreMedia = () => {
|
||||||
|
window.matchMedia = real;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
for (const w of wrappers.splice(0)) w.remove();
|
for (const w of wrappers.splice(0)) w.remove();
|
||||||
|
restoreMedia?.();
|
||||||
|
restoreMedia = null;
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -157,6 +187,52 @@ describe('the queue panel decides whether it can be a column', () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #171 — the scrim is a dismissal target, so it exists only where it
|
||||||
|
* has pixels to be tapped.
|
||||||
|
*
|
||||||
|
* Below 600px `.panel-content` is `width: 100%`, so the scrim is
|
||||||
|
* entirely underneath an opaque panel: measured at 424x439, host,
|
||||||
|
* panel and scrim all 424x318. Drawing it there is a `cursor:
|
||||||
|
* pointer` click target nobody can reach, and the queue is a screen
|
||||||
|
* at that width anyway (#55) — back and the close button are its ways
|
||||||
|
* out. Existence rather than `display: none`, because a hidden scrim
|
||||||
|
* is still an element carrying the handler.
|
||||||
|
*/
|
||||||
|
it('draws no scrim at phone width, where it would have no reachable pixels', async () => {
|
||||||
|
stubPhone(true);
|
||||||
|
|
||||||
|
const el = await panelIn(424);
|
||||||
|
|
||||||
|
expect(el.overlay).toBe(true);
|
||||||
|
expect(el.shadowRoot?.querySelector('.scrim')).toBeNull();
|
||||||
|
|
||||||
|
// The way out a thumb can hit is still there.
|
||||||
|
expect(
|
||||||
|
shadow(el, '[data-testid="queue-close"]')?.getAttribute('aria-label'),
|
||||||
|
).toBe('Close queue');
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The 600–899 band is where the panel is a 320px column of a wider
|
||||||
|
* content area, so the scrim has uncovered pixels and #24's
|
||||||
|
* tap-outside-to-close is real. Same width as the overlay tests
|
||||||
|
* above, with the phone query explicitly answered `false`, so this
|
||||||
|
* fails if the scrim is ever dropped for every overlay.
|
||||||
|
*/
|
||||||
|
it('keeps the scrim above phone width, where it can be tapped', async () => {
|
||||||
|
stubPhone(false);
|
||||||
|
|
||||||
|
const el = await panelIn(700);
|
||||||
|
|
||||||
|
expect(el.overlay).toBe(true);
|
||||||
|
|
||||||
|
shadow<HTMLElement>(el, '.scrim')?.click();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(el.open).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Escape belongs to the overlay, not to the queue. An inline panel is
|
* Escape belongs to the overlay, not to the queue. An inline panel is
|
||||||
* beside the content rather than over it, so there is nothing to
|
* beside the content rather than over it, so there is nothing to
|
||||||
|
|||||||
Reference in New Issue
Block a user