diff --git a/src/components/MediaMuteAndSwitchButton.module.css b/src/components/MediaMuteAndSwitchButton.module.css index ed7926029..52c432d12 100644 --- a/src/components/MediaMuteAndSwitchButton.module.css +++ b/src/components/MediaMuteAndSwitchButton.module.css @@ -330,27 +330,34 @@ Please see LICENSE in the repository root for full details. tile, so the one destructive action lived where the ordinary one does. */ inset-block-start: 0; inset-inline-end: 0; - translate: 30% -30%; + /* It overhangs the corner by about a fifth of itself, as the design draws + it: enough to read as sitting on the tile rather than in it, little + enough that it never reaches the tile beside it. */ + translate: 20% -20%; display: none; align-items: center; justify-content: center; - inline-size: 22px; - block-size: 22px; + inline-size: 28px; + block-size: 28px; border-radius: 50%; background: var(--cpd-color-bg-canvas-default); - color: var(--cpd-color-icon-primary); - box-shadow: 0 0 0 var(--cpd-border-width-1) - var(--cpd-color-border-interactive-secondary); + /* Red at rest, not only under the pointer. The design says what the button + does before you are on it, and the ground carries the hover instead. */ + color: var(--cpd-color-icon-critical-primary); + /* Raised off the tile rather than outlined against it — Compound's own value + for a small raised surface, there being no shadow token. In a dark theme + it falls on a near-black ground and reads as nothing, which is how the + design draws it there too. */ + box-shadow: 0 1.2px 2.4px 0 rgb(0 0 0 / 15%); cursor: pointer; } -/* Red under the pointer: the corner separates it from choosing the tile, and - this says what it will do once you are there. */ +/* Under the pointer the ground turns critical and the shadow gives way to a + ring, so the button reads as pressable and as destructive at once. */ .effectRemove:hover { - background: var(--cpd-color-bg-critical-subtle); - color: var(--cpd-color-icon-critical-primary); + background: var(--cpd-color-bg-critical-subtle-hovered); box-shadow: 0 0 0 var(--cpd-border-width-1) - var(--cpd-color-border-critical-primary); + var(--cpd-color-border-critical-subtle); } .effectGrid .effectTileWrap:hover .effectRemove, diff --git a/src/components/MediaMuteAndSwitchButton.stories.tsx b/src/components/MediaMuteAndSwitchButton.stories.tsx index c20af34bf..064d8a63a 100644 --- a/src/components/MediaMuteAndSwitchButton.stories.tsx +++ b/src/components/MediaMuteAndSwitchButton.stories.tsx @@ -7,6 +7,7 @@ Please see LICENSE in the repository root for full details. import { fn, userEvent, waitFor, within, expect } from "storybook/test"; import { useEffect, useState, type FC, type JSX, type ReactNode } from "react"; +import { TooltipProvider } from "@vector-im/compound-web"; import type { Meta, StoryObj } from "@storybook/react-vite"; import { MediaMuteAndSwitchButton } from "./MediaMuteAndSwitchButton"; @@ -105,14 +106,18 @@ const WithACallArea: FC<{ children: ReactNode }> = ({ children }) => { const meta = { component: MediaMuteAndSwitchButton, decorators: [ + // The app puts one of these over everything; the remove cross needs it to + // be able to name itself. (Story): JSX.Element => ( - - - - - - - + + + + + + + + + ), ], } satisfies Meta; @@ -993,6 +998,42 @@ export const AddedBackgroundsCanBeRemoved: Story = { await expect(args.onRemoveBackgroundEffect).toHaveBeenCalledWith( "added:two", ); + + // The cross the design drew: critical from the moment it is visible, + // rather than only once the pointer is on it, and sitting over the tile's + // top-right corner rather than inside it. + const wrap = other.closest(`.${styles.effectTileWrap}`)!; + await userEvent.hover(wrap); + const cross = wrap.querySelector(`.${styles.effectRemove}`)!; + // Read the token the way the browser will, so the comparison is against + // the colour itself and not against how the value happens to be spelled. + const token = (name: string): string => { + const probe = document.createElement("span"); + probe.style.color = `var(${name})`; + cross.append(probe); + const value = getComputedStyle(probe).color; + probe.remove(); + return value; + }; + const rest = getComputedStyle(cross); + await expect(rest.color).toBe(token("--cpd-color-icon-critical-primary")); + const tile = other.getBoundingClientRect(); + const box = cross.getBoundingClientRect(); + await expect(box.right).toBeGreaterThan(tile.right); + await expect(box.top).toBeLessThan(tile.top); + + // And it names itself once the pointer is on it. The ground turning + // critical under the pointer is not asserted here: a story drives the + // pointer with synthetic events, which never raise CSS hover — that one is + // measured against the design with a real pointer instead. + await userEvent.hover(cross); + await waitFor(async () => + expect( + [...document.body.querySelectorAll("div")].some( + (d) => d.textContent === "Remove" && d.offsetParent !== null, + ), + ).toBe(true), + ); }, }; diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index 66732d8ff..499bd780d 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -21,6 +21,7 @@ import { MenuItem, MenuTitle, RadioInput, + Tooltip, ToggleMenuItem, } from "@vector-im/compound-web"; import { @@ -582,19 +583,25 @@ export const MediaMuteAndSwitchButton: FC = ({ return canRemove(effect) ? (
{tile} - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, - jsx-a11y/no-static-element-interactions -- - Deliberately not a control: aria-hidden, so assistive technology - never meets it, and the keyboard reaches the same action through - Delete on the tile. A role and a tab stop here would add a stop - the menu's arrow keys know nothing about. */} - onRemoveBackgroundEffect?.(effect.id)} - > - - + {/* The cross carries no words, so hovering it names what it does. + Interactive as far as the tooltip is concerned — that keeps it + from wrapping the cross in a tab stop of its own, which is the + one thing this must not grow. */} + + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, + jsx-a11y/no-static-element-interactions -- + Deliberately not a control: aria-hidden, so assistive technology + never meets it, and the keyboard reaches the same action through + Delete on the tile. A role and a tab stop here would add a stop + the menu's arrow keys know nothing about. */} + onRemoveBackgroundEffect?.(effect.id)} + > + + +
) : ( tile