diff --git a/src/components/MediaMuteAndSwitchButton.module.css b/src/components/MediaMuteAndSwitchButton.module.css index f94d67a5d..e620a180d 100644 --- a/src/components/MediaMuteAndSwitchButton.module.css +++ b/src/components/MediaMuteAndSwitchButton.module.css @@ -106,6 +106,11 @@ Please see LICENSE in the repository root for full details. .sectionHeading { position: sticky; inset-block-start: 0; + /* Over what scrolls beneath it. Without a layer of its own it is painted in + order with everything else positioned, and the effect tiles — positioned, + to carry their tick and the remove cross — come after it, so the grid + slid over the heading instead of under it. */ + z-index: 1; background: var(--cpd-color-bg-canvas-default); /* Clear of the frame's top edge; the list it sits in already keeps it clear of the sides. */ @@ -472,10 +477,6 @@ Please see LICENSE in the repository root for full details. display: flex; align-items: center; justify-content: center; - margin-block-start: calc( - -1 * var(--cpd-space-5x) + var(--cpd-border-width-1) - ); - margin-inline: var(--cpd-border-width-1); margin-block-end: var(--cpd-space-2x); block-size: 166px; overflow: hidden; @@ -489,6 +490,33 @@ Please see LICENSE in the repository root for full details. color: var(--cpd-color-icon-secondary); } +/* Above the list: it takes the menu's own top padding, which is why it pulls + itself up over it, and it keeps clear of the frame's sides itself, since it + is not inside the list that otherwise does. */ +.selfPreviewPinned { + margin-block-start: calc( + -1 * var(--cpd-space-5x) + var(--cpd-border-width-1) + ); + margin-inline: var(--cpd-border-width-1); +} + +/* Inside the list, where there was no room to pin it. A scrolling container + cuts off whatever is pulled above its top, so it is the list that moves up + over the menu's padding instead, to bring the preview flush with the top. + That puts the list's corners where the frame's are rounded, so they are + rounded too: square, a stuck heading would show past the curve. */ +.deviceListLeadsWithPreview { + margin-block-start: calc( + -1 * var(--cpd-space-5x) + var(--cpd-border-width-1) + ); + border-start-start-radius: calc( + var(--cpd-space-3x) - var(--cpd-border-width-1) + ); + border-start-end-radius: calc( + var(--cpd-space-3x) - var(--cpd-border-width-1) + ); +} + /* Laid over the box rather than in it. In flow, a sixteen-by-nine picture 166 pixels tall asks for 295 of width, and the menu, sized to its widest content, grew by the pixel that asked for. Out of flow it only fills what diff --git a/src/components/MediaMuteAndSwitchButton.stories.tsx b/src/components/MediaMuteAndSwitchButton.stories.tsx index df73a80e3..f5f17a4f1 100644 --- a/src/components/MediaMuteAndSwitchButton.stories.tsx +++ b/src/components/MediaMuteAndSwitchButton.stories.tsx @@ -986,11 +986,12 @@ export const BackgroundEffectsWithPreview: Story = { }; /** - * No room: a call area as short as a laptop browser often leaves. The list - * could not keep its floor after paying for the preview, so there is none — - * the backgrounds and the devices matter more than a picture of the user. + * No room to pin the preview: a call area as short as a laptop browser often + * leaves. It is still there, but first in the list rather than above it, and + * it scrolls away with the devices and the effects — seen on opening, out of + * the way once the user is choosing. */ -export const BackgroundEffectsNoRoomForPreview: Story = { +export const BackgroundEffectsPreviewScrollsWithTheList: Story = { args: BackgroundEffectsWithPreview.args, parameters: { callAreaHeight: 470 }, play: async ({ canvasElement }) => { @@ -999,13 +1000,77 @@ export const BackgroundEffectsNoRoomForPreview: Story = { await within(document.body).findByRole("menuitemradio", { name: "Blur" }); const menu = document.body.querySelector("[role='menu']")!; - await expect( - menu.querySelector(`.${styles.selfPreview}`), - ).not.toBeInTheDocument(); const list = menu.querySelector(`.${styles.deviceList}`)!; + const preview = menu.querySelector(`.${styles.selfPreview}`)!; + await expect(list.contains(preview)).toBe(true); + + // Flush with the top and the sides, the same as when it is pinned. + const frame = menu.getBoundingClientRect(); + const box = preview.getBoundingClientRect(); + await expect(box.top - frame.top).toBeLessThanOrEqual(2); + await expect(box.left - frame.left).toBeLessThanOrEqual(2); + await expect(frame.right - box.right).toBeLessThanOrEqual(2); + + // The list keeps its whole share, the preview being inside it. await expect(getComputedStyle(list).maxBlockSize).toBe( `${Math.round(470 * 0.6)}px`, ); + + // And it scrolls away. + list.scrollTop = 200; + await waitFor(async () => + expect(preview.getBoundingClientRect().top).toBeLessThan(box.top - 150), + ); + }, +}; + +/** + * A heading stays over what scrolls beneath it. The tiles are positioned, to + * carry their tick and the remove cross, and positioned content painted after + * the heading slid over it: the grid ran across the "Background effects" + * title instead of under it. + */ +export const BackgroundEffectsHeadingStaysOverTheGrid: Story = { + args: { + ...BackgroundEffectsWithPreview.args, + backgroundEffects: [ + ...backgroundEffects, + ...[1, 2, 3, 4].map((n) => ({ + id: `added:${n}`, + label: `Background ${n + 2}`, + kind: "image" as const, + imageUrl: swatch("#1d4ed8", "#172554"), + removable: true, + })), + ], + onRemoveBackgroundEffect: fn(), + }, + parameters: { callAreaHeight: 400 }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Camera" })); + await within(document.body).findByRole("menuitemradio", { name: "Blur" }); + + const menu = document.body.querySelector("[role='menu']")!; + const list = menu.querySelector(`.${styles.deviceList}`)!; + const headings = menu.querySelectorAll( + `.${styles.sectionHeading}`, + ); + const heading = headings[headings.length - 1]; + const grid = menu.querySelector(`.${styles.effectGrid}`)!; + + // Scroll the grid's first row up under the stuck heading. + list.scrollTop = grid.offsetTop - list.offsetTop + 24; + await waitFor(async () => { + const h = heading.getBoundingClientRect(); + const row = grid.getBoundingClientRect(); + await expect(row.top).toBeLessThan(h.bottom); + }); + + // What is on top where the heading is, is the heading. + const h = heading.getBoundingClientRect(); + const hit = document.elementFromPoint(h.left + 40, h.top + h.height / 2); + await expect(heading.contains(hit)).toBe(true); }, }; diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index e4de48378..1e7ef4f48 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -717,21 +717,43 @@ export const MediaMuteAndSwitchButton: FC = ({ return tiles; }; - // The preview is paid for out of the list's share, and only where the list - // keeps its floor after paying, so a menu with one is no taller than a menu - // without — which is what keeps it inside the call area at any size. - const showPreview = + // The preview is always there in the camera menu; what the call's height + // decides is whether it stays put. Pinned above the list, it is paid for out + // of the list's share, and only where the list keeps its floor after paying, + // so a menu with one is no taller than a menu without. Where the list could + // not afford it, it goes into the list instead, first, and scrolls away with + // everything else: seen on opening, out of the way once the user is choosing. + const hasPreview = iconsAndLabels === "video" && selfPreview !== undefined && - listShare !== undefined && - listShare - PREVIEW_BLOCK >= MIN_LIST_HEIGHT; + listShare !== undefined; + const previewPinned = + hasPreview && listShare - PREVIEW_BLOCK >= MIN_LIST_HEIGHT; const listMaxHeight = listShare === undefined ? undefined : Math.max( MIN_LIST_HEIGHT, - listShare - (showPreview ? PREVIEW_BLOCK : 0), + listShare - (previewPinned ? PREVIEW_BLOCK : 0), ); + const preview = hasPreview && ( + // Decoration to assistive technology — the choice is announced by the + // items, and a picture of the user tells them nothing new. +
+ {backgroundEffectSettling ? ( + // The same wait the pressed tile shows, where the picture will be, so + // the eye does not have to go looking for why it is not. + + ) : ( + selfPreview + )} +
+ ); return (
= ({ /> } > - {showPreview && ( - // Pinned above the list rather than inside it: the devices and the - // effects scroll beneath it, because it is what the choosing below is - // for. Decoration to assistive technology — the choice is announced - // by the items, and a picture of the user tells them nothing new. -
- {backgroundEffectSettling ? ( - // The same wait the pressed tile shows, where the picture will - // be, so the eye does not have to go looking for why it is not. - - ) : ( - selfPreview - )} -
- )} + {/* Pinned where there is room: the devices and the effects scroll + beneath it, because it is what the choosing below is for. */} + {previewPinned && preview}
= ({ } as CSSProperties } > + {hasPreview && !previewPinned && preview} {iconsAndLabels === "audio" && speakerOptions && ( <> {/* A menu may only contain items, separators and groups, so each