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 }), )} - )}