Merge branch 'fix/web-call-ui-findings' into main
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', |