Improve theming API - #262
Conversation
| the rows it describes, so they sit on the plain surface colour. */ | ||
| protvista-uniprot .${p}-track-label { | ||
| background-color: var(--protvista-track-label-bg); | ||
| color: var(--protvista-track-label-color); |
There was a problem hiding this comment.
Adding color: var(--protvista-track-label-color) and --protvista-group-label-color pins label text to #222222, so an unthemed viewer no longer inherits the host page's text color, and the nav/credits cells in the same column still do, giving two text colors in one column.
|
|
||
| protvista-uniprot .${p}-nav-track-label, | ||
| protvista-uniprot .${p}-credits { | ||
| background-color: var(--protvista-color-surface); |
There was a problem hiding this comment.
Repointing .nav-track-label and .credits at --protvista-color-surface couples the label column's neutral cells to popovers, tooltips, the customize panel and the datatable, making the documented workaround for a uniform column a page-wide repaint.
| description: 'Background of collapsible group labels.', | ||
| }, | ||
| { | ||
| name: '--protvista-group-label-hover-bg', |
There was a problem hiding this comment.
The new tokens whose defaults are var(--global) are substituted at :where(:root), so a consumer override of the global token on the host or an ancestor no longer reaches the label hover/muted colors, a regression, because the two rules that changed previously read the global token directly at the element.
| * (WCAG 1.4.11), and its hover state, which would otherwise swap in the | ||
| * near-white global hover under text just flipped to white. | ||
| */ | ||
| private applyLabelSurface(surface: 'group' | 'track', bg: Rgb) { |
There was a problem hiding this comment.
The readable-text derivation runs only in applyTheme (the config theme: path), so the CSS-token path, which is the primary documented theming lever, still produces near-black text on a dark label, the exact bug this PR claims to fix.
| // Nothing is derived from the accent, but it goes through the same | ||
| // resolution so every field of `theme` behaves alike: one syntax | ||
| // range, and one answer to an unparseable value. | ||
| const accent = theme.accentColor ? resolveColor(theme.accentColor) : null; |
There was a problem hiding this comment.
A translucent theme.accentColor is now flattened to an opaque colour composited over hardcoded white, changing rendering for a field from which nothing is derived.
| * Translucent colours are composited over {@link SURFACE}. | ||
| */ | ||
| export function resolveColor(value: string): Rgb | null { | ||
| const doc = globalThis.document; |
There was a problem hiding this comment.
resolveColor resolves against globalThis.document rather than the element's own document, and returns null (dropping the entire theme) when document.body is absent.
| // — the only reliable "was that a colour?" test, since a computed style | ||
| // reports inherited black for an invalid value just as it would for a | ||
| // genuine `black`. | ||
| if (!probe.style.color) return null; |
There was a problem hiding this comment.
The probe.style.color truthiness guard accepts CSS-wide keywords and var() references, so those resolve to whatever the probe inherits instead of being rejected.
| )?.default; | ||
| // The fallback only fires with no DOM to resolve against; the | ||
| // registry value is the source of truth in every real environment. | ||
| cachedDefaultText = (declared && resolveColor(declared)) ?? { |
There was a problem hiding this comment.
defaultTextColor() caches permanently on first call, so a DOM-less first call locks in the hardcoded fallb
| if (!doc?.body) return null; | ||
|
|
||
| const probe = doc.createElement('span'); | ||
| // `display: none` keeps the probe out of layout; `color` still resolves. |
There was a problem hiding this comment.
Each resolveColor call builds a throwaway element, appends it to document.body and calls getComputedStyle, up to four forced style flushes per applyTheme, repeated on every config apply.
| ); | ||
| } | ||
|
|
||
| afterEach(() => { |
There was a problem hiding this comment.
Teardown asymmetry: the spec's afterEach restores the real fetch before mount.js's afterEach removes the mounted viewers, so the still-live components can issue real network requests.
2 participants
Addresses #261
Purpose
theme.labelColorpainted group and track labels the same colour, flattening the shipped grey/white hierarchy — and a dark value left near-black text, caret, and hover unreadable on it. This makes one-knob theming keep the hierarchy and stay legible with any colour.Approach
labelColorbecomes two-tone: the colour on group labels, a light tint (25% over white) on track labels; newgroupLabelColor/trackLabelColorfields pin either surface exactly.src/styles/color.tsresolves theme colours torgb()via a browser probe (full CSS syntax in, unparseable values dropped — also an injection guard, and nocolor-mix()so the support matrix holds), then derives text, muted text, caret, and hover per surface by WCAG contrast (best of near-black/white). Results land on new--protvista-*tokens that default to current values.--protvista-color-surfaceinstead of taking the track-label tint (migration note inCHANGELOG).
Testing
src/styles/__spec__/color.spec.ts— colour maths: resolution/rejection (incl. injection attempts), alpha compositing, WCAG reference values.src/__spec__/theme-config.spec.ts— token-level: two-tone mapping, overrides, fallbacks, stale-token clearing, AA sweep across light/dark themes, and the mid-grey (#808080) boundary where AA is unreachable.src/__browser__/theme-chrome.browser.spec.ts— real Chromium: derived values actually reach the cells through the cascade; nav/credits stay on the surface colour.Visual changes
Updated
docs/src/assets/screenshots/theming-comparison.pngandtutorial-themed.pngshow the new two-tone label panel. Unthemed viewers are pixel-identical (all new tokens default to the values they replace).Checklist