From 099950a100b9198168be84d9a67e1c246d307424 Mon Sep 17 00:00:00 2001 From: fkwp Date: Thu, 17 Sep 2026 22:43:48 +0200 Subject: [PATCH] Update the device menu to the latest design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Each section is headed by its own rule, edge to edge, instead of one inset divider between the sections. Compound draws that rule under a menu heading already, so this removes our Separator, its spacing rule and its import. - Our heading padding override goes too: Compound's menu heading is the design already — 13px/600 on a 19.5px line, #656D77, 16px either side, 7px from the text to the rule. Ours had been moving that 7px above the text. - Two spacings do differ from Compound, both measured off the mock rather than guessed: 20px below the rule before a section's first device (27px rule to control, against Compound's 16px), and 28px above a heading that follows another section (41px from the last control to the next heading's text, against 22px). The first heading keeps Compound's spacing — the menu's own padding sits above it. - Guarded by relationships, not numbers: the first control sits further below the rule than from the menu's edge, and a section stands further from the one above it than a heading does from its own first device. Both fail if either spacing goes back to Compound's 8px. Co-Authored-By: Claude Opus 5 --- .../MediaMuteAndSwitchButton.module.css | 38 ++++++++++------ .../MediaMuteAndSwitchButton.stories.tsx | 43 +++++++++++++++++++ src/components/MediaMuteAndSwitchButton.tsx | 2 - 3 files changed, 67 insertions(+), 16 deletions(-) diff --git a/src/components/MediaMuteAndSwitchButton.module.css b/src/components/MediaMuteAndSwitchButton.module.css index 8c0f38609..4faa52ee1 100644 --- a/src/components/MediaMuteAndSwitchButton.module.css +++ b/src/components/MediaMuteAndSwitchButton.module.css @@ -41,20 +41,6 @@ Please see LICENSE in the repository root for full details. flex-direction: column; } -/* Compound underlines every menu heading. The design has a single line, the - one dividing the speaker list from the microphone list, so the headings - carry none and that divider is drawn explicitly. */ -.menu h3 { - border-block-end: none; - padding-block-end: var(--cpd-space-2x); -} - -/* The menu zeroes separator spacing, but the design gives the heading below - the divider room to breathe. */ -.menu [role="separator"] { - margin-block-end: var(--cpd-space-5x); -} - /* Only the device lists scroll; the level meter stays put beneath them. The bound comes from the measured height of the call area, set by the component: the menu is portalled outside the root, so neither a container @@ -72,6 +58,30 @@ Please see LICENSE in the repository root for full details. var(--device-list-scroll-padding-end, 0); } +/* Each section is headed by its own rule, which Compound draws under a menu + heading already. The heading spans the whole menu — the menu has no padding + of its own — so that rule runs edge to edge, stopping only where the frame + is, rather than inset as a separator between the sections would be. + + Compound's spacing around it is the design's too, bar one thing: the design + leaves a section's first device further below the rule than Compound does — + measured off the mock, 27px from the rule to the top of the control, against + the 16px that Compound's 8px margin and the row's own 8px padding give. The + difference goes below the rule, so the rule stays tight under its own text + where Compound put it. */ +.menu h3 { + margin-block-end: var(--cpd-space-5x); +} + +/* One section is set much further from the one above it than Compound's 8px + heading margin allows: measured off the mock, 41px from the last device's + control to the next heading's text, against the 22px we had. Only between + sections — the first heading keeps Compound's spacing, because the menu's + own padding is already above it. */ +.deviceList [role="group"] + [role="group"] h3 { + margin-block-start: var(--cpd-space-7x); +} + /* Each section's heading stays at the top of the scrollport while any of that section is still in view, so a long list never leaves you wondering which kind of device you are looking at. It leaves with its own section, because diff --git a/src/components/MediaMuteAndSwitchButton.stories.tsx b/src/components/MediaMuteAndSwitchButton.stories.tsx index 67bc3d557..474240cb0 100644 --- a/src/components/MediaMuteAndSwitchButton.stories.tsx +++ b/src/components/MediaMuteAndSwitchButton.stories.tsx @@ -247,6 +247,49 @@ export const SpeakerAndMicrophoneSections: Story = { `.${styles.deviceList}`, )!; await expect(list.scrollHeight).toBe(list.clientHeight); + + // Each section is headed by its own rule, running the full width of the + // menu rather than inset — and nothing divides the sections besides. + const menu = document.body.querySelector("[role='menu']")!; + await expect( + document.body.querySelectorAll("[role='separator']"), + ).toHaveLength(0); + const headings = document.body.querySelectorAll( + `.${styles.sectionHeading}`, + ); + await expect(headings).toHaveLength(2); + const frame = menu.getBoundingClientRect(); + for (const heading of headings) { + const rule = heading.querySelector("h3")!; + await expect( + Number.parseFloat(getComputedStyle(rule).borderBottomWidth), + ).toBeGreaterThan(0); + // Edge to edge, stopping only where the menu's frame is drawn. + const box = rule.getBoundingClientRect(); + await expect(box.left - frame.left).toBeLessThanOrEqual(2); + await expect(frame.right - box.right).toBeLessThanOrEqual(2); + } + + // A section's first device sits further below the rule than it does from + // the menu's edge. Stated as the relationship rather than a number: what + // the design asks for is the asymmetry, and Compound's own heading margin + // alone would make the two equal. + const control = document.body.querySelector("input[type='radio']")!; + const ruleBottom = headings[0] + .querySelector("h3")! + .getBoundingClientRect().bottom; + const box = control.getBoundingClientRect(); + await expect(box.top - ruleBottom).toBeGreaterThan(box.left - frame.left); + + // And one section stands further from the one above it than a heading does + // from its own first device — again the relationship, not a number. + const groups = document.body.querySelectorAll("[role='group']"); + const speakers = groups[0].querySelectorAll("input[type='radio']"); + const lastSpeaker = speakers[speakers.length - 1].getBoundingClientRect(); + const nextHeading = groups[1]!.querySelector("h3")!.getBoundingClientRect(); + await expect(nextHeading.top - lastSpeaker.bottom).toBeGreaterThan( + box.top - ruleBottom, + ); }, }; diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index 69ab69a55..836c76789 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -19,7 +19,6 @@ import { MenuItem, MenuTitle, RadioInput, - Separator, ToggleMenuItem, } from "@vector-im/compound-web"; import { @@ -478,7 +477,6 @@ export const MediaMuteAndSwitchButton: FC = ({ (n) => t("settings.devices.speaker_numbered", { n }), )} - )}