fix(accessibility): preserve forced-colors state cues - #151
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough편집기와 툴바에 forced-colors 스타일을 추가했습니다. 시스템 색상, 키보드 포커스, 비활성 상태, 링크, 구조적 경계선, 협업 표시를 검증하는 Vitest 및 Playwright 테스트를 추가했습니다. Changes강제 색상 접근성
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized accessibility change preserves forced-colors state cues without altering normal theme behavior, and no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Browser
participant ForcedColorsSpec
participant LocalHarness
participant Stylesheet
Browser->>ForcedColorsSpec: forced-colors 테스트 실행
ForcedColorsSpec->>LocalHarness: 로컬 fixture와 스타일시트 요청
LocalHarness-->>Browser: 편집기 DOM과 CSS 제공
Browser->>Stylesheet: forced-colors 스타일 적용
ForcedColorsSpec->>Browser: computed style 및 포커스 상태 수집
Browser-->>ForcedColorsSpec: 테두리, 색상, 윤곽선, 표시 상태 반환
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation 변경 사항은 이슈 Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
opencode-agent
Bot
added
area: accessibility
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| expect(rejectedExternalRequests).toEqual([]); | ||
| }); | ||
|
|
||
| test('preserves state and structural cues in forced colors', async ({ page }) => { |
There was a problem hiding this comment.
🟡 New forced-colors browser test never runs
The spec file matches no Playwright project's testMatch (/(?:clipboard|print)\.browser\.spec\.ts/u in playwright.config.ts), so Playwright silently skips it. It is also outside vitest's src/** include, so no runner executes it and the forced-colors browser verification never happens.
Prompt for agents
The new spec tests/browser/specs/forced-colors.browser.spec.ts is never run. In tests/browser/playwright.config.ts, ENGINE_BROWSER_SPECS is defined as /(?:clipboard|print)\.browser\.spec\.ts/u and used as the testMatch for the chromium, firefox and webkit projects; the consensus project matches only clipboard.consensus.spec.ts. Because the filename forced-colors.browser.spec.ts does not contain 'clipboard' or 'print', it matches no project and Playwright skips it without error. Update the ENGINE_BROWSER_SPECS pattern (or add the new file to a project's testMatch) so that forced-colors.browser.spec.ts is discovered and executed, e.g. by extending the alternation to include 'forced-colors'.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Verified still valid on exact #151 head 48fffe12bc596dbf401facc0b05aef99cc060f1c: this branch still inherits the narrow browser selector, so forced-colors.browser.spec.ts is not current execution evidence. The first causal owner for tests/browser/playwright.config.ts is existing Draft #380, not #151. Fresh #380 head fccc4466cb70285822df94bbfb9b0b1f6896f2c3 has ENGINE_BROWSER_SPECS = /\.browser\.spec\.ts$/u, which would discover this spec after non-destructive reconciliation. No duplicate config patch is being added here. Keep this thread unresolved until #151 inherits that owner change and a fresh exact-head browser run proves Chromium/Firefox/WebKit actually discover and execute the forced-colors journey.
# Conflicts: # src/editorFocusStyles.test.ts
|
Conflict resolution at merge head
CI re-verifies at this head. |
- Single screen @media (forced-colors: active) layer after base state rules: editor content focus = CanvasText outline-color (guaranteed canvas contrast), toolbar focus/active = Highlight family, structure = CanvasText - editorFocusStyles contract asserts the unified cascade; designTokens and forcedColors contracts both satisfied (17 tests pass)
|
Cascade reconciliation at |
|
Validation for exact head a10b7c9:
|
|
Current-head governance update for a10b7c9:
|
|
Terminal Strix update for exact head a10b7c9:
|
|
Correction to the earlier validation note:
|
|
Terminal current-head Strix update for eb3618c: the required job failed during Run Strix (quick) (https://github.com/ContextualWisdomLab/inkspan/actions/runs/33146823027/job/98769587278). The uploaded strix-reports artifact records contextual-orchestrator HTTP 400 invalid_tools: each tool.function.description must be at most 1024 characters, followed by STRIX_PROVIDER_UNAVAILABLE; the run emitted no usable scan verdict. Its displayed Vulnerabilities 0 is therefore not vulnerability-clear evidence. The corrective owner-boundary change remains intact; #380 still owns the generalized forced-colors browser matcher. |
1 participant
Closes #150.
Purpose
Implement the bounded forced-colors/high-contrast accessibility slice without changing Inkspan's normal light/dark theme behavior or host authority boundaries.
Protected
mainis the shipped implementation authority. Mutable head/base SHAs, divergence counts, workflow IDs, review counts and mergeability are decision-time evidence only and must be refetched before every lifecycle action.Test-first lineage
d89bd41dabca47a00d486e847eed0d3df8b5456bproved missing forced-colors system-state mappings.bcd993a1d521d3f44fe07eaeaa5acda10dce3d6bproved split forced-colors blocks could be overridden by later equal-specificity screen rules.@media (forced-colors: active)layer without weakening normal light/dark behavior.Tabfocus rather than programmatic.focus()for:focus-visibleevidence. That finding is addressed in the branch; its resolved/outdated review thread is not a remaining blocker.Implemented boundary
Inside the final forced-colors layer, Inkspan uses CSS system colors to preserve editor/container boundaries, toolbar/collaboration chrome, grouping, keyboard focus, active and disabled states, authored links, code/pre/table boundaries, and collaboration cursors. Disabled controls do not rely on opacity alone. Normal light/dark theme declarations remain unchanged outside that media query and Inkspan does not opt out of user-agent forced-color adjustment.
This slice adds no persistence, transport, credentials, model use, authorization, tenancy, network, provider, room, or durable-audit authority.
Browser-discovery dependency
The current #151 branch still inherits a Playwright selector that does not discover
tests/browser/specs/forced-colors.browser.spec.ts, so file presence and aggregate CI success are not execution evidence for that journey.Draft #380 is the existing dedicated writer for the browser harness/spec/config boundary. Its live
tests/browser/playwright.config.tsuses/\.browser\.spec\.ts$/u, which is the correct generalized discovery contract for committed browser journeys. #151 must not race that writer by duplicating the configuration repair locally. Keep the discovery review thread unresolved until #151 inherits/reconciles the #380 browser configuration and exact-current browser evidence proves this spec actually executes.Dependency order / source ownership
This remains the earliest active owner for the forced-colors production/source boundary. Former PR #362 is already integrated on protected main and overlaps historical
src/styles.css/ focus-test work, so #151 must reconcile its net production delta against then-current protected truth non-destructively before further lifecycle advancement.For browser configuration specifically, #380 remains the single writer. Release issue #118 is a downstream stable-release acceptance boundary and does not authorize bypassing this PR's own dependency, browser-execution, review, or merge gates.
Integration boundary
Before any lifecycle transition:
tests/browser/playwright.config.ts; only after its generalized discovery contract is inherited/reconciled may fix(accessibility): preserve forced-colors state cues #151 claim forced-colors browser execution;If the available mutation surface lacks a safe non-destructive branch merge/update operation, classify only that exact reconciliation mutation
TOOL_UNAVAILABLE; do not emulate it by moving refs or copying the browser-config fix into this branch. Pending, queued, skipped-required, cancelled, absent, neutral, failed, stale, predecessor, status-only, model-only, wrong-checkout, undiscovered-test, or synthetic-source-only evidence is non-passing. Do not self-approve, weaken gates, transfer predecessor evidence, bypass governance, or fabricate release identity.