From a3af6f1514af3557f6e66fe90d6fa95cd6e6ccff Mon Sep 17 00:00:00 2001 From: fkwp Date: Thu, 24 Sep 2026 12:50:23 +0200 Subject: [PATCH] Hold both device menus at 296px, framed in Safari too - The camera and microphone menus were as wide as their longest device name, so a long one widened the menu over more of the call: 340px for a long speaker name. Both are now 296px, as design sets them, and a long name wraps. - Narrower only where the call area is, so the menu still fits inside it: 268px in a 300px call. Taken from the observation of the call area that already bounds the list's height. - Held only for the dropdown. On Android and iOS Compound renders the menu as a drawer as wide as the screen; menuIsDrawer follows its rule. - Once the list is long enough to scroll, Safari paints it above the outline the menu is framed with, and the frame vanished down both sides of it: 384px of it on the many-devices microphone menu, in WebKit. The list now keeps a border width clear of the frame, so the headings and the meter inside it no longer need their own. - Guarded with a long speaker name, which fails at 340px on main's sizing; a unit test run as a phone, which fails with the width held there; and the many-devices story's inset, which fails on the old stylesheet, since the stories run in Chromium, which never showed the Safari bug. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../MediaMuteAndSwitchButton.module.css | 7 +++- .../MediaMuteAndSwitchButton.stories.tsx | 42 ++++++++++++++++++- .../MediaMuteAndSwitchButton.test.tsx | 30 +++++++++++++ src/components/MediaMuteAndSwitchButton.tsx | 29 +++++++++++-- src/components/menuIsDrawer.ts | 13 ++++++ 5 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 src/components/menuIsDrawer.ts diff --git a/src/components/MediaMuteAndSwitchButton.module.css b/src/components/MediaMuteAndSwitchButton.module.css index 1d5242689..0a242b263 100644 --- a/src/components/MediaMuteAndSwitchButton.module.css +++ b/src/components/MediaMuteAndSwitchButton.module.css @@ -44,6 +44,11 @@ Please see LICENSE in the repository root for full details. /* Only the device lists scroll, bounded by the measured call area: the menu is portalled out of the root, so CSS can't size it against the call. */ .deviceList { + /* A list that scrolls can paint above the menu's outline, hiding it. */ + margin-inline: var(--cpd-border-width-1); + inline-size: calc( + var(--device-list-inline-size) - 2 * var(--cpd-border-width-1) + ); overflow-y: auto; min-block-size: 0; max-block-size: var(--device-list-max-height); @@ -68,7 +73,6 @@ Please see LICENSE in the repository root for full details. position: sticky; inset-block-start: 0; background: var(--cpd-color-bg-canvas-default); - margin-inline: var(--cpd-border-width-1); margin-block-start: var(--cpd-border-width-1); } @@ -77,7 +81,6 @@ Please see LICENSE in the repository root for full details. position: sticky; inset-block-end: 0; background: var(--cpd-color-bg-canvas-default); - margin-inline: var(--cpd-border-width-1); margin-block-end: var(--cpd-border-width-1); } diff --git a/src/components/MediaMuteAndSwitchButton.stories.tsx b/src/components/MediaMuteAndSwitchButton.stories.tsx index d4965892f..653a56418 100644 --- a/src/components/MediaMuteAndSwitchButton.stories.tsx +++ b/src/components/MediaMuteAndSwitchButton.stories.tsx @@ -65,8 +65,10 @@ const WithACallArea: FC<{ children: ReactNode }> = ({ children }) => {
{ + const canvas = within(canvasElement); + await userEvent.click(canvas.getByRole("button", { name: "Microphone" })); + const body = within(document.body); + const long = await body.findByRole("menuitemradio", { name: /Logitech/ }); + const short = await body.findByRole("menuitemradio", { name: "Headset" }); + + const menu = document.body.querySelector("[role='menu']")!; + await expect(Math.round(menu.getBoundingClientRect().width)).toBe(296); + await expect(long.getBoundingClientRect().height).toBeGreaterThan( + short.getBoundingClientRect().height, + ); + }, +}; + export const OutputCannotBeChosen: Story = { args: { ...Default.args, @@ -440,6 +473,13 @@ export const ManyDevices: Story = { // The meter is the menu's one opaque part, so it must stay inside the frame. await expect(pinned.left).toBeGreaterThan(frame.left); await expect(pinned.right).toBeLessThan(frame.right); + + // A list long enough to scroll keeps clear of the frame, which it can + // otherwise paint over. + await expect(list.scrollHeight).toBeGreaterThan(list.clientHeight); + const box = list.getBoundingClientRect(); + await expect(box.left).toBeGreaterThanOrEqual(frame.left + 1); + await expect(box.right).toBeLessThanOrEqual(frame.right - 1); }, }; diff --git a/src/components/MediaMuteAndSwitchButton.test.tsx b/src/components/MediaMuteAndSwitchButton.test.tsx index 9fd67dfdb..4eb04cfe6 100644 --- a/src/components/MediaMuteAndSwitchButton.test.tsx +++ b/src/components/MediaMuteAndSwitchButton.test.tsx @@ -26,6 +26,14 @@ import { MediaDevicesContext } from "../MediaDevicesContext"; import { type MediaDevices } from "../state/MediaDevices"; import { restoreAudioCapture, stubAudioCapture } from "../utils/test"; +const platformMock = vi.hoisted(() => vi.fn(() => "desktop")); +vi.mock("../Platform", () => ({ + get platform(): string { + return platformMock(); + }, + isFirefox: (): boolean => false, +})); + interface RenderOptions { requestDeviceNames: () => void; } @@ -149,6 +157,28 @@ describe("MediaMuteAndSwitchButton", () => { expect(onMute).not.toHaveBeenCalled(); }); + // On a phone the menu is a drawer, not a dropdown. + test("holds the menu's width only where it is a dropdown", async () => { + const width = async (platform: string): Promise => { + platformMock.mockReturnValue(platform); + const user = userEvent.setup(); + const { unmount } = renderComponent( + , + ); + await user.click(screen.getByRole("button", { name: "Microphone" })); + const list = document.body.querySelector( + "[style*='--device-list-max-height']", + )!; + const value = list.style.getPropertyValue("--device-list-inline-size"); + unmount(); + return value; + }; + expect(await width("desktop")).toBe("296px"); + expect(await width("ios")).toBe(""); + expect(await width("android")).toBe(""); + platformMock.mockReturnValue("desktop"); + }); + test("requests device names when opened", async () => { const user = userEvent.setup(); const requestDeviceNames = vi.fn(); diff --git a/src/components/MediaMuteAndSwitchButton.tsx b/src/components/MediaMuteAndSwitchButton.tsx index 25342f001..78da0e12f 100644 --- a/src/components/MediaMuteAndSwitchButton.tsx +++ b/src/components/MediaMuteAndSwitchButton.tsx @@ -40,6 +40,7 @@ import { useMediaDevices } from "../MediaDevicesContext"; import { useRootElement } from "../RootElementContext"; import { observeElementSize$ } from "../utils/elementSize"; import { LiveMicrophoneLevelMeter } from "./MicrophoneLevelMeter"; +import { menuIsDrawer } from "./menuIsDrawer"; export interface MenuOptions { label: DeviceLabel | AudioOutputDeviceLabel; @@ -84,6 +85,12 @@ const LIST_SHARE_OF_CALL = 0.6; /** Smallest device list height in px, so a short call still shows more than one device. */ const MIN_LIST_HEIGHT = 160; +/** The width design sets for the menu; a long device name wraps instead. */ +const MENU_WIDTH = 296; + +/** Space kept between the menu and the call area's sides. */ +const MENU_MARGIN = 16; + export const MediaMuteAndSwitchButton: FC = ({ enabled, busy, @@ -143,17 +150,27 @@ export const MediaMuteAndSwitchButton: FC = ({ // Measured on the call area: CSS can't size the portalled menu against it. const rootElement = useRootElement(); const [listMaxHeight, setListMaxHeight] = useState(); + const [menuWidth, setMenuWidth] = useState(MENU_WIDTH); useEffect(() => { if (!menuOpen) return; // Followed, since a host can resize the call while the menu is open. const subscription = observeElementSize$(rootElement) .pipe( - map(({ height }) => - Math.max(MIN_LIST_HEIGHT, Math.round(height * LIST_SHARE_OF_CALL)), + map(({ width, height }) => ({ + height: Math.max( + MIN_LIST_HEIGHT, + Math.round(height * LIST_SHARE_OF_CALL), + ), + width: Math.min(MENU_WIDTH, Math.round(width - 2 * MENU_MARGIN)), + })), + distinctUntilChanged( + (a, b) => a.height === b.height && a.width === b.width, ), - distinctUntilChanged(), ) - .subscribe(setListMaxHeight); + .subscribe(({ height, width }) => { + setListMaxHeight(height); + setMenuWidth(width); + }); return (): void => subscription.unsubscribe(); }, [menuOpen, rootElement]); @@ -349,6 +366,10 @@ export const MediaMuteAndSwitchButton: FC = ({ { "--device-list-max-height": listMaxHeight === undefined ? undefined : `${listMaxHeight}px`, + // On a phone Compound renders the menu as a drawer, which sets its own width. + "--device-list-inline-size": menuIsDrawer() + ? undefined + : `${menuWidth}px`, "--device-list-scroll-padding-end": meterHeight === undefined ? undefined : `${meterHeight}px`, "--device-list-scroll-padding-start": diff --git a/src/components/menuIsDrawer.ts b/src/components/menuIsDrawer.ts new file mode 100644 index 000000000..4f4561d7d --- /dev/null +++ b/src/components/menuIsDrawer.ts @@ -0,0 +1,13 @@ +/* +Copyright 2026 Element Creations Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { platform } from "../Platform"; + +/** Whether Compound opens a menu as a drawer, as it does on a phone. */ +export function menuIsDrawer(): boolean { + return platform === "android" || platform === "ios"; +}