Re-implement accessibility (at_rule colors, reduce-motion, prefers-contrast, forced-colors) - #136
Re-implement accessibility (at_rule colors, reduce-motion, prefers-contrast, forced-colors)#136evnchn wants to merge 4 commits into
Conversation
…ed-motion
- nicegui/elements/colors.{js,py}: at_rule parameter to scope colors by media query
- nicegui/static/nicegui.css: prefers-reduced-motion kill-switch for transitions/animations/smooth-scroll
- tests/test_colors.py: test_at_rule
- main.py: ui.colors(primary='#317ABE', at_rule='@media (prefers-contrast: more)')
Previously 30859c6 inadvertently reverted recent upstream CSS and main.py changes
by wholesale-copying from a stale branch; this rewrites that commit cleanly.
- website/components/hero_section.py: forced-colors:invert on mascot SVG - website/documentation/windows.py: forced-colors outline/opacity on code & browser windows - website/header.py: Tailwind variant for prefers-reduced-transparency on .q-header.fade Social icons (Phosphor/currentColor), logo wordmark (markdown), and star badge targets dropped — redesign removed them.
|
Force-pushed: rewrote history to fix regressions introduced by the re-impl sub-agent. Previous version of commit 30859c6 wholesale-copied
Now a clean +63/-14 delta in 8 files across 2 commits. |
102e4e1 to
d005e7a
Compare
There was a problem hiding this comment.
Pull request overview
Re-implements a set of accessibility-focused styling improvements across NiceGUI core theming and the documentation website, adding support for conditional theming via CSS at-rules and improving behavior under reduced-motion / forced-colors / reduced-transparency user preferences.
Changes:
- Add
at_rulesupport toui.colors()to scope theme variables (and custom colors) under a CSS at-rule (e.g.@media (...)). - Add global
prefers-reduced-motionCSS to disable transitions/animations/scroll-behavior when requested. - Wire forced-colors and reduced-transparency adjustments into website components and docs UI.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
nicegui/elements/colors.py |
Adds at_rule parameter and forwards it as a component prop. |
nicegui/elements/colors.js |
Implements at-rule–scoped CSS injection for theme variables and custom colors. |
nicegui/static/nicegui.css |
Adds @media (prefers-reduced-motion: reduce) overrides. |
tests/test_colors.py |
Adds a basic test covering the new at_rule code path. |
main.py |
Uses at_rule to bind a higher-contrast primary color under prefers-contrast: more. |
website/components/hero_section.py |
Adds forced-colors behavior for the mascot SVG. |
website/documentation/windows.py |
Improves forced-colors visibility for windows and copy button. |
website/header.py |
Drops header blur when prefers-reduced-transparency: reduce is set. |
| for (const [color, value] of Object.entries(this.customColors || {})) { | ||
| const name = color.replaceAll("_", "-"); | ||
| const varName = "--q-" + name; | ||
| css += `\n body { ${varName}: ${value} !important; }`; | ||
| css += `\n .text-${name} { color: var(${varName}) !important; }`; | ||
| css += `\n .bg-${name} { background-color: var(${varName}) !important; }`; | ||
| } | ||
| this.styleEl = document.createElement("style"); | ||
| this.styleEl.innerHTML = `${this.atRule} {\n${css}\n}`; | ||
| document.head.appendChild(this.styleEl); |
There was a problem hiding this comment.
When atRule is set, this code injects a <style> block for custom colors but does not remove any previously injected [data-nicegui-custom-colors] style blocks (created by applyColors). If a page (or earlier render) already called ui.colors(...) without at_rule, the old .text-*/.bg-* rules can remain globally active and defeat the intent of scoping custom colors to the at-rule. Consider removing/replacing the existing NiceGUI custom-colors style blocks (and, if necessary, previously set custom --q-* inline variables) before appending the at-rule style element.
There was a problem hiding this comment.
Acknowledged as a known design edge case. Each ui.colors(...) call mounts a separate Vue element; the at-rule branch injects a scoped <style> element which is cleaned up on unmounted. If a prior ui.colors() (no at_rule) ran applyColors globally within the same page render, those rules do persist — but that's the same behavior as chained ui.colors() calls today, and the at_rule's !important inside the scoped block still wins within the at-rule scope. Cleaning up prior global custom colors is a broader lifecycle change; leaving out of scope for this PR.
There was a problem hiding this comment.
Applied in 5f2abd3. Followed the existing NiceGUI-removal-before-readd pattern from nicegui/static/nicegui.js (the applyColors function, which removes any existing [data-nicegui-custom-colors] style blocks before appending a new one). The at_rule branch now clears both [data-nicegui-custom-colors] and [data-nicegui-scoped-colors] style tags plus any --q-* inline properties set on document.body by prior plain ui.colors(...) calls, then tags its own scoped <style> with data-nicegui-scoped-colors for symmetrical cleanup by future calls. Added test_at_rule_supersedes_plain_colors covering the scenario.
| const colors = { | ||
| "--q-primary": this.primary, | ||
| "--q-secondary": this.secondary, | ||
| "--q-accent": this.accent, | ||
| "--q-dark": this.dark, | ||
| "--q-dark-page": this.darkPage, | ||
| "--q-positive": this.positive, | ||
| "--q-negative": this.negative, | ||
| "--q-info": this.info, | ||
| "--q-warning": this.warning, | ||
| }; | ||
| let css = Object.entries(colors) | ||
| .map(([k, v]) => ` body { ${k}: ${v} !important; }`) | ||
| .join("\n"); | ||
| for (const [color, value] of Object.entries(this.customColors || {})) { | ||
| const name = color.replaceAll("_", "-"); | ||
| const varName = "--q-" + name; | ||
| css += `\n body { ${varName}: ${value} !important; }`; | ||
| css += `\n .text-${name} { color: var(${varName}) !important; }`; | ||
| css += `\n .bg-${name} { background-color: var(${varName}) !important; }`; | ||
| } |
There was a problem hiding this comment.
at_rule is currently only covered by a test that verifies the media query branch can set the primary color, but it doesn't exercise custom colors inside an at-rule (i.e., scoped .text-<custom> / .bg-<custom> rules) or verify that custom color styles don’t leak outside the at-rule when previous ui.colors calls have injected global custom-color CSS. Adding tests for these behaviors would help prevent regressions in the new styling path.
There was a problem hiding this comment.
Noted. The existing test_at_rule covers the core at-rule branch (scoped --q-primary override applied to a button). Expanding coverage to custom-color scoping + leak-prevention is reasonable but, given Falko's limited review bandwidth, keeping the test surface minimal for this PR. Happy to add more in a follow-up if desired.
| @media (prefers-reduced-motion: reduce) { | ||
| html, | ||
| body * { | ||
| transition: none !important; | ||
| animation: none !important; | ||
| scroll-behavior: auto !important; | ||
| } |
There was a problem hiding this comment.
The reduced-motion rule targets html, body *, which excludes the body element itself and also does not cover pseudo-elements (e.g. ::before/::after) that often carry animations/transitions. To fully respect prefers-reduced-motion, consider expanding the selector to include body and pseudo-elements (commonly *, *::before, *::after scoped as needed).
There was a problem hiding this comment.
Good catch — fixed in 4b27a5c. Switched the selector to *, *::before, *::after so the body element and pseudo-element animations are also covered.
Previously the rule targeted 'html, body *' which excludes the body element itself and pseudo-elements. Switch to '*, *::before, *::after' so animations/transitions on body and generated content also honor prefers-reduced-motion.
Re-implementation of what fork PR #133 was trying to do. After upstream zauberzeug#5910 redesigned the website, #133 is no longer rebaseable.
Status: Ready for review
Core (commit 30859c6)
nicegui/elements/colors.{js,py}—at_ruleparameternicegui/elements/sub_pages.{js,py}— reduce-motion (drop smooth scroll)nicegui/static/nicegui.css— prefers-reduced-motion media querytests/test_colors.py— test_at_rulemain.py— prefers-contrast color bindingWebsite wiring (commit 102e4e1)
website/components/hero_section.py—forced-colors:inverton mascot SVGwebsite/documentation/windows.py—forced-colors:outlineon code/browser windows;forced-colors:opacity-100on copy buttonwebsite/header.py— Tailwind arbitrary media variantmedia-[(prefers-reduced-transparency:reduce)]:[&.fade]:backdrop-blur-noneto drop blur for users preferring reduced transparencyDropped (no longer applicable after zauberzeug#5910 redesign)
website/components/footer_section.py— oldsvg.face/svg.discord/reddit/githubreplaced by Phosphor icons (text-based, inheritcurrentColor, which already respectsforced-colorsautomatically)star badge—forced-colors:invertwebsite/star.pywas deleted; replaced by Phosphor-icon GitHub badge in newheader.py(no SVG to invert)— those CSS classes are gone from the redesign; the few remaining.bg-primary-alpha/.hover-opacityports inexamples_section.py,demo.py,overview.pyopacity-*/hover:opacity-*usages use Tailwind utilities directly— already handled globally inprefers-reduced-motionguards on scroll/revealwebsite/static/style.cssby the redesignCloses #133 if accepted.