feat(pad): add theme-color meta to match toolbar on mobile (#7606)#7636
feat(pad): add theme-color meta to match toolbar on mobile (#7606)#7636JohnMcLear wants to merge 5 commits intoether:developfrom
Conversation
Mobile browsers paint the address-bar / status-bar area above the viewport. Without theme-color this is a system color that does not match the Etherpad toolbar, leaving a visible gap above the pad. Render <meta name="theme-color"> server-side so the bar matches the configured toolbar on first paint. Light + dark variants are emitted with prefers-color-scheme media queries when dark mode is enabled. Colors are derived from settings.skinVariants via a new SkinColors helper (mirrors --bg-color in the colibris pad-variants.css). Closes ether#7606 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
/review |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
Code Review by Qodo
Context used 1. theme-color skipped for non-colibris
|
Review Summary by QodoAdd theme-color meta tags matching toolbar on mobile
WalkthroughsDescription• Add server-side theme-color meta tags matching toolbar colors • Create SkinColors utility mapping skin variants to toolbar colors • Support light and dark theme variants with prefers-color-scheme media queries • Add comprehensive unit and integration tests for theme-color rendering Diagramflowchart LR
A["SkinColors.ts<br/>Toolbar color mapping"] --> B["pad.html<br/>Render theme-color meta"]
A --> C["timeslider.html<br/>Render theme-color meta"]
B --> D["Mobile browser<br/>Matching address bar"]
C --> D
E["settings.skinVariants<br/>settings.enableDarkMode"] --> A
File Changes1. src/node/utils/SkinColors.ts
|
Code Review by Qodo
Context used 1. Dark meta mismatches timeslider
|
Qodo flagged a mismatch: timeslider does not switch skin variants on prefers-color-scheme, so emitting a dark theme-color via media query would leave dark-mode devices with a dark address bar over a light toolbar. Drop the media-query metas on timeslider and emit one unconditional theme-color resolved from settings.skinVariants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/review |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Persistent review updated to latest commit 7b41829 |
Qodo flagged that gating the light theme-color on prefers-color-scheme: light leaves no applicable meta on dark-OS devices when enableDarkMode is false — the address bar then uses a system color while the toolbar stays light. Drop the light media query so the light theme-color is the baseline, and let the prefers-color-scheme: dark meta override it when dark mode is enabled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/review |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Persistent review updated to latest commit dcedf72 |
Two related Qodo findings on the SkinColors helper: - The pad client's dark-mode auto-switch (pad.ts L650) forces super-dark-toolbar regardless of the configured skinVariants, so the prefers-color-scheme: dark meta must always be #485365 — not whichever dark variant the operator configured. - When skinVariants only carries a dark token (e.g. dark-toolbar), the previous helper left the baseline meta at #ffffff, so light-OS users would see white above a dark toolbar. Replace toolbarThemeColors() with configuredToolbarColor() (used as the unconditional baseline) and a fixed DARK_MODE_TOOLBAR_COLOR constant (used in the prefers-color-scheme: dark meta). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/review |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Persistent review updated to latest commit d6753cc |
|
On Qodo finding #4 ("theme-color lacks feature flag"): leaving this as-is and not gating it.
|
Address remaining Qodo findings on the theme-color rollout: - (#1) Skip emitting the meta entirely when settings.skinName is not colibris — the helper only knows colibris's --bg-color values, so on no-skin or third-party skins the previous code would emit a white meta over a non-white toolbar. - (ether#4) Drop the prefers-color-scheme: dark variant. The pad's client-side dark mode is also gated on a localStorage white-mode override that no media query can express, so the dark meta could paint a dark address bar over a still-light toolbar. The single baseline meta always matches what the user sees on first paint. - (ether#8) Remove the redundant module.exports assignment; rely on the ES named export only (tsx handles the require() interop). - (ether#9) Iterate the toolbar variants in CSS source order and let the last match win, matching the cascade in pad-variants.css when multiple *-toolbar tokens are present. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
/review |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Persistent review updated to latest commit 51115af |
| <meta name="robots" content="noindex, nofollow"> | ||
| <meta name="referrer" content="no-referrer"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=0"> | ||
| <% if (configuredColor) { %><meta name="theme-color" content="<%=configuredColor%>"><% } %> |
There was a problem hiding this comment.
1. theme-color skipped for non-colibris 📎 Requirement gap ≡ Correctness
pad.html only emits <meta name="theme-color"> when configuredToolbarColor() returns a value, but that helper returns null for any skinName other than colibris. This means pads using no-skin or third-party skins will not include the meta tag, failing the requirement that pad HTML output includes a theme-color meta matching the active theme.
Agent Prompt
## Issue description
`pad.html` conditionally omits `<meta name="theme-color">` for non-colibris skins because `configuredToolbarColor()` returns `null` unless `skinName === 'colibris'`. This violates the requirement that pad HTML output includes a theme-color meta whose content matches the active theme's toolbar color.
## Issue Context
The current implementation avoids emitting a potentially wrong color for unknown skins, but the compliance requirement is explicit about always including the meta and matching the active theme.
## Fix Focus Areas
- src/templates/pad.html[9-14]
- src/templates/pad.html[51-51]
- src/node/utils/SkinColors.ts[23-33]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Summary
Closes #7606.
Mobile browsers paint the address-bar / status-bar strip above the viewport using a system color when
<meta name="theme-color">is absent, leaving a visible gap above the Etherpad toolbar. This PR renderstheme-colorserver-side so the bar blends into the configured toolbar from the first paint.src/node/utils/SkinColors.tsresolves the colibris*-toolbarskin variants to the--bg-colorvalues declared incolibris/src/pad-variants.css. Iteration order matches the CSS source so the last applicable token wins, mirroring the cascade.nullfor non-colibris skins (no-skin, third-party skins) since their toolbar colors aren't known server-side; the templates then omit the meta rather than emit a misleading value.pad.htmlandtimeslider.htmlemit a single unconditional<meta name="theme-color">when the helper returns a color. Noprefers-color-schememedia-query variants — the runtime dark-mode auto-switch inpad.tsis also gated on a localStorage white-mode override that a media query cannot express, so any media-query variant would risk painting a dark address bar over a still-light toolbar.specialpages.tscover the rendered metas and the non-colibris skip path; vitest unit tests cover the helper.Semver
patch— additive HTML metadata, no behavior change for existing clients.Test plan
skinVariantswithdark-toolbar; confirm the address bar matches the dark toolbar.skinName: "no-skin"; confirm the page renders with notheme-colormeta.Generated with Claude Code