fix: brighten dark-mode input text + add axe-core contrast tests#34
Merged
Conversation
Inputs in the Edit Profile modal rendered typed values in Chrome's UA-default mid-gray (contrast ~3.5:1) against the dark surface — form controls don't inherit `color` from `body`. Placeholders used `--text-tertiary` (#48484A), nearly invisible against the `#1C1C1E` surface. - Set `color: var(--text-primary)` on `.form-input/.form-select/.form-textarea`. - Extend placeholder rule to all form inputs (was textarea-only). - Override placeholder to `--text-secondary` (#8E8E93) in dark mode only; light-mode placeholders stay at the existing 0.3-opacity subtle tone.
Adds `@axe-core/playwright` and a new `e2e/a11y.spec.ts` suite that scans the popup and options page (in both light and dark modes) plus the Edit Profile modal (dark, with inputs populated) for WCAG color-contrast violations. This is the kind of check that would have caught the dim-input regression automatically: axe measures computed fg/bg and fails anything below AA 4.5:1 without needing a baseline screenshot. Surfaces several preexisting violations (primary button 4.01:1, sidebar nav `.text-secondary` on translucent bg, section descriptions, empty-state copy). Each is annotated in `KNOWN_CONTRAST_EXCEPTIONS` with the token-level root cause so the suite passes today and future regressions are caught, while the known issues stay tracked for a separate design decision.
Owner
Author
Code Review — PR #34SummaryReviewed the CSS fix for dark-mode input contrast and the new 1. PR Description vs. Implementation Consistency ✅The description accurately reflects the code changes:
2. Multi-Dimensional Analysis
3. Test Coverage ✅The new
4. Regression Verification ✅
5. CI/CD Pipeline ✅All 10 check runs completed successfully:
Merging now. Nice work on pairing the fix with automated contrast testing — this will catch similar issues early in the future. Generated by Claude Code |
6 tasks
helebest
added a commit
that referenced
this pull request
Apr 23, 2026
* docs: sync v1.6.1 references and formalize release checklist
About panel, README roadmap, docs/index.html JSON-LD softwareVersion,
STORE_LISTING Current Version block, SEO_GUIDE header, and the hardcoded
version assertion in e2e/options.spec.ts were all stuck at v1.6.0 (or
older) after the 1.6.1 cut — evidence that the release process had no
single source of truth for "where does the version string appear."
CONTRIBUTING.md rewritten to match the real project (plain JS + three-
component Vite build, no TS source, no React) and gains a three-tier
Release Checklist so future bumps can't silently miss JSON-LD schema,
store listing copy, or the e2e assertion that asserts the About text.
* test(e2e): tighten visual regression tolerance and refresh stale baselines
playwright.config.ts maxDiffPixelRatio was 0.05 — a ~46,000-pixel budget
on a 1280x720 frame. That band was wide enough to silently absorb the
entire v1.5.1 -> v1.6.1 text change in options-about.png, so the baseline
quietly stayed three versions out of date while CI kept passing. Drop
the global threshold to 0.01 and remove the per-test 0.02 overrides in
popup-visual.spec.ts so there is a single rule.
Note: content drift below ~1% of pixels (single-character diffs like the
version number itself) is still invisible to toHaveScreenshot even at
0.01. That class of drift is caught by text assertions such as
options.spec.ts's toContainText('X-Proxy v1.6.1'), not by pixel diff.
Regenerate all nine baselines via --update-snapshots=all so screenshots
reflect the shipped UI, and add a discoverable `npm run test:e2e:update`
script so contributors stop having to remember the raw playwright flag.
* fix(test): eliminate CSS-transition race in axe color-contrast scans
The dark-mode axe scans in a11y.spec.ts were failing spuriously because
page.emulateMedia({colorScheme:'dark'}) flips :root custom properties
synchronously, but elements carrying `transition: all var(--transition-
base)` animate their resolved color between the old and new var() values
over 250ms. Axe, injected immediately after the media flip, samples the
mid-transition rgba value — empirically rgba(120,120,125,0.87) at 100ms
and rgba(170,170,174,0.99) at 200ms — and reports it as insufficient
contrast against the (already-final) dark background.
Fix: disable CSS transitions and animations via addStyleTag BEFORE
emulateMedia fires, so the color change is instantaneous instead of
interpolated. playwright.config.ts's toHaveScreenshot.animations:
'disabled' only applies to screenshots; axe runs page JS and needs
CSS-level suppression.
Ordering note documented in-file: disableTransitions must precede
emulateMedia, otherwise transitions are already armed at the moment of
the flip and !important on transition-property can't unwind them.
Three pre-existing "failures" cleared by this fix:
- li[data-section=about] > span inactive nav label (reported 2.35)
- #importProfilesBtn .btn-secondary (reported 3.67)
- #exportProfilesBtn .btn-secondary (reported 3.67)
All three are in fact above AA in the shipped CSS; they just weren't
observable through the racing transition.
* test(e2e): add modal and dark-mode visual baselines (Phase 2)
Previously only the popup had visual regression coverage for dark mode
and nothing guarded the Add/Edit Profile modal in either theme. A revert
of #34 / #35's contrast fixes, or any drift in iOS design tokens
(--border-radius, --text-secondary, etc.), would ship without any test
failing. Six baselines close the gap:
modal-add-light / modal-add-dark — Add Profile form
modal-edit-light / modal-edit-dark — Edit form with a seeded profile
options-profiles-dark — Profiles page dark (was uncovered)
options-about-dark — About page dark
Reuses the disableTransitions() helper approach from a11y.spec.ts —
addStyleTag must precede emulateMedia, otherwise CSS transitions are
already armed at the moment of the media flip and screenshots capture
mid-interpolation rgba values. Same race as the axe scans.
Seeded profile uses the v2 storage shape (lib/storage-migration.js) so
no migration step runs mid-test.
* docs(contributing): add visual baseline refresh + caveats to Release Checklist
Phase 1 and Phase 2 uncovered three things that were not institutionalized:
1. After any CSS / HTML / design-token change, baselines under
e2e/__screenshots__/ must be regenerated via npm run test:e2e:update
and the PNG diffs reviewed in the PR. Tier 2 now calls this out as
its top bullet — explicit review, not just file-count confirmation.
2. The About-section baseline in visual.spec.ts is effectively a second
version string and must be regenerated whenever the version bumps.
Added it to the Tier 1 table so it can't be missed.
3. Gaps that the automation does NOT catch are now documented so
reviewers don't over-trust a green check:
- single-character text drift below the 1% threshold (content
assertions handle this, not pixel diff)
- CSS transition mid-flight values fooling axe after emulateMedia
- worker-parallelism flakes vs real test failures
Sanity-check section also gains the build + full-e2e + serial-e2e
commands that Phase 1 / Phase 2 executions leaned on.
* test(e2e): add storage migration integration guard (Phase 4a)
tests/mode-migration.test.js already covers every branch of migrateData()
as a pure function. This spec catches a different regression class:
someone disconnecting migrateData() from the actual read or write path.
Unit tests would not detect that.
Three integration invariants pinned down:
1. Read path is non-destructive — reading v1-shaped storage through the
popup must not silently rewrite it to v2. If someone adds an auto-
persist "convenience" to readData(), this fails.
2. Write path upgrades to v2 — first user action (click System) after
the popup loads v1 data must persist back in v2 shape with the
correct `mode` field. If someone forgets version: SCHEMA_VERSION in
writeData() or bypasses migrateData(), this fails.
3. Direct mode is preserved — same as #2 but targets the Direct button,
since v2's raison d'être was to distinguish Direct from System. A
regression that collapsed them would slip past test #2.
Scope note: UI-level "seed v1 → popup shows migrated mode" assertions
would be complementary but require forcing the background service
worker to re-initialize against seeded data. chrome.runtime.reload()
inside a persistent Playwright context leaves the extension in a
transient ERR_BLOCKED_BY_CLIENT state that does not reliably clear.
Deferred — unit coverage already exhaustive, and write-path tests here
detect real regressions.
TDD red-check performed during development: flipping the version
assertion from 2 to 1 in test #2 produces "Expected: 1, Received: 2",
confirming the test actually exercises the migration write path rather
than reading back its own seeded data.
* feat(a11y): keyboard navigation for profile modal (Phase 4b)
Pre-v1.6.1 options.js shipped with no keyboard handling on #profileModal:
no Escape-to-close, no focus return on close, no auto-focus on open.
The only exit was Tab-hunting for Cancel or the X button, which fails
WCAG 2.1.2 ("No Keyboard Trap") for keyboard-only users.
This lands three behaviors driven red-green by e2e/keyboard-nav.spec.ts:
1. Escape closes the modal when .show is present. Document-level
keydown listener, guarded by the class so it's a no-op otherwise.
2. Focus is saved at showProfileModal() (document.activeElement before
open) and restored at hideProfileModal() so Add/Edit Profile
buttons regain focus on close. Defensive: skips restore if the
saved node was removed from the DOM mid-session.
3. On open, #profileName receives focus synchronously so keyboard
users can type immediately instead of being stranded behind the
modal.
TDD flow: four tests in keyboard-nav.spec.ts failed against the prior
codebase (no handlers), then all four passed after the three changes
above. Unit + full serial E2E (96 tests) stay green — no regressions
in existing modal-open/close paths.
Scope note: focus trap (Tab cycling within the modal, blocking Tab
from escaping to background content) is deferred. The current Cancel
/Save button pair is the expected Tab-end and browsers already return
to page top after Tab past the last focusable element, which is a
survivable degradation for keyboard users given Escape always works.
* feat(a11y): trap Tab focus inside profile modal (Phase 4b+)
Phase 4b (ba16edc) made the profile modal keyboard-operable: Escape
closes it, focus saved on open and restored on close, first field
auto-focused. But Tab from the Save button still bled focus into the
background page (footer Save All, sidebar nav), so a keyboard user
would see the modal visually open while their keyboard was driving
the content behind it. WAI-ARIA's dialog pattern requires focus be
contained.
This change adds a trap:
- Tab on the last focusable inside the modal wraps to the first.
- Shift+Tab on the first focusable wraps to the last.
- "Focusable" enumerates buttons / inputs / selects / textareas /
tabindex≠-1, excluding disabled and display:none nodes — the last
point matters because #pacDetails, #proxyDetails, #routingRulesPanel
and auth inputs toggle visibility based on proxy type.
Extracted getModalFocusables(modal) as an instance method so future
modals (if any are added) can reuse the same visibility-aware
enumeration. No shared lib/focus-trap.js yet — one caller doesn't
justify that indirection.
TDD: two new tests in keyboard-nav.spec.ts failed against Phase 4b
(focus escaped to page elements outside #profileModal), pass after
this change. Full regression: 91 unit + 98 E2E green, no changes to
prior Escape / focus-return / auto-focus behavior.
* docs(changelog): populate [Unreleased] with the E2E-coverage overhaul
[Unreleased] had placeholder "Future improvements planned" text rather
than actual unreleased work. Replaced with concrete Added / Changed /
Fixed entries covering:
- Keyboard navigation for the profile modal (Escape, focus return,
auto-focus, Tab trap).
- Three new E2E spec files (modal-visual, migration, keyboard-nav)
and the test:e2e:update convenience script.
- Visual regression threshold tightened from 5% to 1%.
- CONTRIBUTING.md rewritten with a three-tier Release Checklist.
- CSS-transition race in axe color-contrast scans eliminated.
- Stale v1.6.0 references across options.html, README, docs/*,
e2e/options.spec.ts synced to v1.6.1.
"Future improvements planned" preserved under a labeled subsection so
the aspirational list is kept but no longer conflated with shipped work.
* test(e2e): fix CI Linux baseline drift + address code review feedback
Three follow-up fixes from PR #39 review:
1. Linux CI visual baseline for options-about-dark.png was failing at
9636 px (~2.08%) against a 1% global threshold. Cause is font-hinting
divergence between macOS (baseline authoring) and Xvfb+Linux (CI)
amplified by the About page being the text-heaviest screen in the
UI — version line, description paragraph, nine-item feature list,
link row.
Narrowly scoped per-test override `maxDiffPixelRatio: 0.03` on that
single screenshot, with an inline justification comment. 0.03 still
leaves real token / layout regressions detectable (off-by-color-
button is tens of thousands of pixels, an order of magnitude above
this floor). No other baseline approached 1% on CI, so no other
override needed.
2. disableTransitions() helper was duplicated verbatim in a11y.spec.ts
and modal-visual.spec.ts. Extracted to e2e/helpers.ts with a single
authoritative JSDoc documenting the "run before emulateMedia"
ordering constraint. Both specs now import it.
3. migration.spec.ts header said "Why only two tests here" but the
file contains three. Rewrote the scope note to enumerate all three
invariants the tests pin down (non-destructive read, System-click
upgrade, Direct-click upgrade) and dropped the misleading count.
Local regression (serial, --workers=1): 98 E2E + 91 unit, green.
---------
Co-authored-by: hele.hl <hele.hl@bytedance.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.form-input/.form-select/.form-textarea) rendered in Chrome's UA-default mid-gray on the dark#1C1C1Esurface because form controls don't inheritcolorfrombody. Added explicitcolor: var(--text-primary)so typed values match label brightness. Placeholders in dark mode went from invisible#48484Ato readable#8E8E93.KNOWN_CONTRAST_EXCEPTIONSallowlist; future regressions fail immediately.@axe-core/playwrightande2e/a11y.spec.ts. Covers popup + options page in both color schemes and the Edit Profile modal with populated inputs. This catches contrast issues without baseline screenshots.Token changes
Mirrored in both
options.cssandpopup.css.--primary-color#007AFF#0062CC--primary-light/--primary-lighterrgba(0, 122, 255, …)rgba(0, 98, 204, …)--text-secondary(light)rgba(60, 60, 67, 0.6)rgba(60, 60, 67, 0.76).section-description, empty-state copy all below AA on white--text-secondary(dark)#8E8E93#AEAEB2.nav-item.active(dark only)color: var(--primary-color)color: var(--text-primary)#0062CCon--primary-lighttint over dark sidebar is unreadable; iOS convention is white text on tinted bgTest plan
npm test— 83 unit tests passnpm run test:e2e— all e2e tests pass (including 5 new a11y tests; 0 exceptions)npm run type-check— cleannpm run build— bundles updateddist/, loaded unpacked, toggledprefers-color-scheme: darkvia DevTools Rendering panel, confirmed Edit Profile modal input values render bright white and placeholders readable gray; sidebar nav / section description / button contrast all improved; visual regression baselines still within 5% tolerance🤖 Generated with Claude Code