mirror of
https://github.com/vector-im/element-call.git
synced 2026-09-22 22:29:30 +00:00
Update the device menu to the latest design
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<HTMLElement>(
|
||||
`.${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,
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -19,7 +19,6 @@ import {
|
||||
MenuItem,
|
||||
MenuTitle,
|
||||
RadioInput,
|
||||
Separator,
|
||||
ToggleMenuItem,
|
||||
} from "@vector-im/compound-web";
|
||||
import {
|
||||
@@ -478,7 +477,6 @@ export const MediaMuteAndSwitchButton: FC<MediaMuteAndSwitchButtonProps> = ({
|
||||
(n) => t("settings.devices.speaker_numbered", { n }),
|
||||
)}
|
||||
</div>
|
||||
<Separator />
|
||||
</>
|
||||
)}
|
||||
<div role="group" aria-label={optionsButtonLabel}>
|
||||
|
||||
Reference in New Issue
Block a user