amadan
Fix five review findings against the web call UI
All five came in with the web/native-consistency branch and have been live since 9434395. Two are accessibility. Collapsing chat set display:none on #chat, which takes its aria-live log out of the accessibility tree — so a message arriving while chat was closed announced nothing. The badge is aria-hidden and a changed aria-label does not announce either, so it was silent three ways. A live region outside the panel now carries the count. The count and never the message: someone who closed chat has said they do not want it read out, and the count is what the badge shows. Written only while collapsed, because the panel's own log announces arrivals when open. The room's mobile control bar was reshaping the lobby. Both use .calls-controls, so under 35rem "Join the call" — a primary lg button, the one thing on that screen a person is meant to press — was squeezed into a third of the width at --rst-fs-xs, the same weight as the toggles. Scoped to #controls, which exists only in the room. The pressed-toggle comment described an inset outline that had been deleted, leaving an empty rule. The third non-colour cue is now a ring from estilo's callui.css, which is a dependency on another repository and on the attribute staying put. Said so, rather than describing something that is not there. Help rendered against the brand: [rst-shell-tail] is display:contents at 800px and up, and only [rst-shell-account] carries the auto margin, so Help — first in the tail — was pushed left. The push moves to the dropdown and the account takes an ordinary gap after it. The lobby held 60% of its width open beside an empty black panel when the browser had no getUserMedia, because .calls-device-layout's columns are fixed. The wrap hides with its preview and the column collapses. Gates: make ci, and make browser at 34 scenarios with two new assertions in `layout` — that the collapsed panel announces the count, and that the region announcing it is outside the subtree that collapses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6 files changed,
+127
−6
internal/calls/static/call-layout.js+10 −0internal/calls/static/calls-shell.css+16 −0internal/calls/static/calls.css+42 −6internal/calls/templates/room.html+13 −0internal/callstest/room_test.go+30 −0test/browser/call.mjs+16 −0
CI failed — run details
diff --git a/internal/calls/static/call-layout.js b/internal/calls/static/call-layout.js| index f9b1a0f..f807540 100644 |
| --- a/internal/calls/static/call-layout.js |
| +++ b/internal/calls/static/call-layout.js |
| @@ -11,6 +11,7 @@ |
| const workspace = document.getElementById('call-workspace'); |
| const log = document.getElementById('chat-log'); |
| const unreadLabel = document.getElementById('chat-unread'); |
| + const unreadLive = document.getElementById('chat-unread-live'); |
| if (!room || !tiles || !controls || !chat || !toggle || !workspace) return; |
| let mode = 'focus'; |
| let unread = 0; |
| @@ -36,6 +37,15 @@ |
| toggle.setAttribute('aria-expanded', String(expanded)); |
| toggle.setAttribute('aria-label', unread ? `Chat (${unread} new)` : 'Chat'); |
| unreadLabel.textContent = unread ? ` (${unread})` : ''; |
| + if (unreadLive) { |
| + // Only while collapsed, and only the count. Assigning the same |
| + // string twice would re-announce it, so the guard is on the value |
| + // and not on the event that got us here. |
| + const said = !expanded && unread |
| + ? (unread === 1 ? '1 new chat message' : unread + ' new chat messages') |
| + : ''; |
| + if (unreadLive.textContent !== said) unreadLive.textContent = said; |
| + } |
| } |
| focus.addEventListener('click', () => { mode = 'focus'; layout(); }); |
| grid.addEventListener('click', () => { mode = 'grid'; layout(); }); |
diff --git a/internal/calls/static/calls-shell.css b/internal/calls/static/calls-shell.css| index fc94d8a..764a5d4 100644 |
| --- a/internal/calls/static/calls-shell.css |
| +++ b/internal/calls/static/calls-shell.css |
| @@ -10,3 +10,19 @@ body { background: var(--rst-bg); } |
| .calls-lobby-heading h1 { font-size: 2rem; } |
| [estilo-call-header] h1 { font-size: 1.5rem; } |
| [estilo-call-header] [rst-page-header] { margin: 0; } |
| +/* At 800px and up the framework makes [rst-shell-tail] display:contents, |
| + so its children are laid out by the bar itself and the tail stops |
| + grouping them. [rst-shell-account] carries margin-inline-start:auto and |
| + the Help dropdown does not, so Help — which comes first in the tail — |
| + was pushed hard against the brand while the account went right. Move |
| + the push onto whichever dropdown leads the tail, and give the account |
| + an ordinary gap after it instead of a second auto margin, which would |
| + only split the two apart again. Below 800px the tail is a column and |
| + an auto inline-start margin would right-align Help in the open menu, |
| + so this stays inside the same breakpoint the framework used. */ |
| +@media (min-width: 800px) { |
| + [rst-shell-tail] > [rst-dropdown] { margin-inline-start: auto; } |
| + [rst-shell-tail] > [rst-dropdown] ~ [rst-shell-account] { |
| + margin-inline-start: var(--rst-sp-3); |
| + } |
| +} |
diff --git a/internal/calls/static/calls.css b/internal/calls/static/calls.css| index 9af4f4d..da333b5 100644 |
| --- a/internal/calls/static/calls.css |
| +++ b/internal/calls/static/calls.css |
| @@ -109,6 +109,19 @@ |
| margin-block: var(--rst-sp-3); |
| } |
| +/* Off screen, still in the accessibility tree and still announced. */ |
| +.calls-sr-only { |
| + position: absolute; |
| + inline-size: 1px; |
| + block-size: 1px; |
| + margin: -1px; |
| + padding: 0; |
| + overflow: hidden; |
| + clip-path: inset(50%); |
| + white-space: nowrap; |
| + border: 0; |
| +} |
| + |
| .calls-status { |
| color: var(--rst-text-muted); |
| min-height: 1.2em; |
| @@ -173,10 +186,15 @@ |
| /* The toggles keep one accessible name (aria-label) and move only |
| aria-pressed, so the state is carried three ways and never by colour: |
| the word in .calls-toggle-state, the attribute a screen reader reads, |
| - and this inset outline. */ |
| + and a ring around the control. |
| + The ring is not drawn here. It comes from estilo's callui.css, as |
| + [estilo-call-actions] [aria-pressed='true'], which is why this rule |
| + keeps only the weight. That is a dependency on another repository and |
| + on the attribute staying on the element: a control that loses |
| + estilo-call-actions loses the ring, and then the weight below is the |
| + only thing left that is not the word or the attribute. */ |
| .calls-controls button[aria-pressed='true'] { |
| font-weight: 700; |
| - |
| } |
| .calls-toggle-state { |
| @@ -249,6 +267,15 @@ |
| #call-workspace .chat-log { min-block-size: 10rem; max-block-size: 24rem; } |
| #call-workspace .calls-chat-collapsed { display: none; } |
| .calls-device-layout { display: grid; gap: var(--rst-sp-4); align-items: start; } |
| +/* devices.js hides #preview when the browser cannot reach a camera at |
| + all, and the wrap around it is a black panel with a border radius and |
| + nothing inside. Left alone it holds 60% of the lobby open beside the |
| + pickers. Hide the wrap and collapse the column with it, so a browser |
| + with no media shows the device controls at full width rather than |
| + beside a hole. */ |
| +.calls-device-layout:has(> .calls-preview-wrap > #preview[hidden]) > .calls-preview-wrap { |
| + display: none; |
| +} |
| .calls-preview-wrap { border-radius: 20px; overflow: hidden; background: #000; } |
| .calls-preview { display: block; max-width: none; aspect-ratio: 16 / 9; border: 0; border-radius: 0; } |
| .calls-pickers { margin-block-start: 0; } |
| @@ -272,6 +299,9 @@ |
| .calls-queue li { flex-wrap: wrap; overflow-wrap: anywhere; } |
| @media (min-width: 64rem) { |
| .calls-device-layout { grid-template-columns: minmax(0, 1.5fr) minmax(0, 1fr); } |
| + .calls-device-layout:has(> .calls-preview-wrap > #preview[hidden]) { |
| + grid-template-columns: minmax(0, 1fr); |
| + } |
| } |
| @media (max-width: 35rem) { |
| .calls-grid[data-layout='focus'] { grid-template-columns: repeat(2, minmax(0, 1fr)); } |
| @@ -308,9 +338,15 @@ |
| @media (max-width: 63.99rem) { |
| #call-workspace .chat-log { min-block-size: 4rem; max-block-size: 14rem; } |
| } |
| +/* The room's control bar, and only the room's. The lobby uses |
| + .calls-controls too, and under these rules its "Join the call" — a |
| + primary lg button, the one thing on that screen a person is meant to |
| + press — was squeezed into a third of the width at --rst-fs-xs, the |
| + same weight as the two toggles beside it. #controls exists only in the |
| + room, so it is the narrowest handle that says "the bar under a call". */ |
| @media (max-width: 35rem) { |
| - .calls-controls { display: grid; grid-template-columns: repeat(3, minmax(0, 1fr)); } |
| - .calls-controls button { |
| + #controls.calls-controls { display: grid; grid-template-columns: repeat(3, minmax(0, 1fr)); } |
| + #controls.calls-controls button { |
| display: flex; |
| flex-direction: column; |
| justify-content: center; |
| @@ -321,6 +357,6 @@ |
| font-size: var(--rst-fs-xs); |
| white-space: normal; |
| } |
| - .calls-controls form { display: flex; } |
| - .calls-controls .calls-toggle-state { font-size: inherit; } |
| + #controls.calls-controls form { display: flex; } |
| + #controls.calls-controls .calls-toggle-state { font-size: inherit; } |
| } |
diff --git a/internal/calls/templates/room.html b/internal/calls/templates/room.html| index 56cd2fa..992527f 100644 |
| --- a/internal/calls/templates/room.html |
| +++ b/internal/calls/templates/room.html |
| @@ -40,6 +40,19 @@ |
| <button type="button" id="toggle-chat" rst-btn aria-expanded="true" aria-controls="chat" hidden>{{icon "message-circle"}}Chat<span id="chat-unread" aria-hidden="true"></span></button> |
| <button type="button" id="leave" rst-btn="danger">{{icon "log-out"}}Leave</button> |
| </div> |
| + {{/* Unread chat, for anyone who cannot see the badge on the button. |
| + The panel's own log is aria-live, but collapsing chat sets |
| + display:none on it, which takes the whole region out of the |
| + accessibility tree — so a message arriving while chat is closed |
| + announced nothing at all. The badge is aria-hidden and a changed |
| + aria-label does not announce either, so this was silent three ways. |
| + This region is outside the panel and survives the collapse. |
| + It carries the count and never the message: someone who closed |
| + chat has said they do not want it read out, and the count is |
| + exactly what the badge beside the button shows. call-layout.js |
| + writes it only while collapsed — with the panel open its own log |
| + announces arrivals, and saying it twice is worse than once. */}} |
| + <p id="chat-unread-live" class="calls-sr-only" role="status" aria-live="polite"></p> |
| {{/* The device panel, closed until asked for. A call's own screen is |
| the video and the controls; three selects permanently under them |
| would be furniture the whole call pays for so that someone can |
diff --git a/internal/callstest/room_test.go b/internal/callstest/room_test.go| index 570ebe6..33c23ac 100644 |
| --- a/internal/callstest/room_test.go |
| +++ b/internal/callstest/room_test.go |
| @@ -671,3 +671,33 @@ func TestRoomDeviceNoteIsNotTheStatusLine(t *testing.T) { |
| t.Fatal("the device note and the status line are the same element") |
| } |
| } |
| + |
| +// The unread-chat live region (the accessibility half of collapsing the |
| +// chat panel). Collapsing sets display:none on #chat, which takes its |
| +// aria-live log out of the accessibility tree, so a message arriving |
| +// while chat is closed announced nothing: the badge is aria-hidden, a |
| +// changed aria-label does not announce, and the log was gone. This |
| +// region is the one thing that survives the collapse, so what matters |
| +// in the markup is that the server renders it OUTSIDE #chat. |
| +func TestRoomAnnouncesUnreadChatOutsideTheCollapsingPanel(t *testing.T) { |
| + x, sqid, _ := memberWithMeeting(t) |
| + c := x.browser(t) |
| + c.signIn("paul@example.com") |
| + body := c.get("/m/" + sqid).Body.String() |
| + live := strings.Index(body, `id="chat-unread-live"`) |
| + if live < 0 { |
| + t.Fatalf("room page has no unread live region: %s", body) |
| + } |
| + for _, want := range []string{`role="status"`, `aria-live="polite"`, `class="calls-sr-only"`} { |
| + if !strings.Contains(body[live-200:live+200], want) { |
| + t.Fatalf("the unread live region lacks %q: %s", want, body[live-200:live+200]) |
| + } |
| + } |
| + // Outside the panel, not merely present. A region inside #chat would |
| + // be display:none'd with it and announce nothing, which is the whole |
| + // defect. |
| + panel := strings.Index(body, `<section id="chat"`) |
| + if panel >= 0 && live > panel { |
| + t.Fatal("the unread live region renders inside #chat, where collapsing hides it") |
| + } |
| +} |
diff --git a/test/browser/call.mjs b/test/browser/call.mjs| index cf2cffe..819b624 100644 |
| --- a/test/browser/call.mjs |
| +++ b/test/browser/call.mjs |
| @@ -3740,6 +3740,22 @@ const run = { |
| await pageB.locator('#chat-send').click(); |
| check(await until(async () => (await pageA.locator('#chat-unread').innerText()).includes('1'), 5000), |
| 'hidden chat reports unread arrivals'); |
| + // The badge above is aria-hidden and the toggle's aria-label does not |
| + // announce, and collapsing chat display:none's the aria-live log out |
| + // of the tree entirely — so without this region a screen reader is |
| + // told nothing at all. It has to be OUTSIDE #chat to survive the |
| + // collapse, which is what textContent being readable here proves: |
| + // Playwright reads the DOM, but the assertion below on the live |
| + // region's ancestry is what pins it out of the collapsed subtree. |
| + check(await until(async () => (await pageA.locator('#chat-unread-live') |
| + .textContent()) === '1 new chat message', 5000), |
| + 'collapsed chat announces the unread count to a screen reader (got ' + |
| + JSON.stringify(await pageA.locator('#chat-unread-live').textContent()) + ')'); |
| + check(await pageA.evaluate(() => { |
| + const live = document.getElementById('chat-unread-live'); |
| + const chat = document.getElementById('chat'); |
| + return !!live && !!chat && !chat.contains(live); |
| + }), 'the unread live region is outside the panel that gets collapsed'); |
| await pageA.locator('#toggle-chat').focus(); |
| await pageA.keyboard.press('Enter'); |
| check(await pageA.locator('#chat-text').inputValue() === 'Draft kept while chat is closed', |