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.
# 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 |
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.
Current exact authority
Fresh state after the latest protected-main and review changes:
main@057762e003fadad418c38ee748bc3e5b20e711eb;a517183774745040b1a4012e570040d3f92e2580onfix/forced-colors-accessibility;3b38ead2d00f44eb578d0689087b9293b3dabe1e;src/editorFocusStyles.test.ts,src/forcedColorsStyles.test.ts,src/styles.css, andtests/browser/specs/forced-colors.browser.spec.ts;COMMENTED, 0APPROVED;tests/browser/specs/forced-colors.browser.spec.ts.Protected
mainremains the shipped implementation authority. This snapshot is decision-time evidence only and must be refetched before any later 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.Implemented boundary
Inside one final screen
@media (forced-colors: active)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.
Verified false-green browser evidence defect
The current #151 branch contains
tests/browser/specs/forced-colors.browser.spec.ts, but its inheritedtests/browser/playwright.config.tsselects only/(?:clipboard|print)\.browser\.spec\.ts/u. Therefore the forced-colors spec is structurally undiscoverable by Chromium, Firefox, and WebKit on this exact branch. Existing CI/security success must not be represented as execution evidence for that browser spec.The browser harness/spec/config boundary already has an active dedicated Inkspan writer: Draft #380. Its current exact head
b89b38e22c10d01d2462b22361e4bc20b08f9ba0ownstests/browser/playwright.config.tsand currently uses/\.browser\.spec\.ts$/u, which would discover every committed*.browser.spec.ts, including this forced-colors spec after the branches are reconciled. #151 must not race that config writer by duplicating the selector repair locally.The second unresolved review observation notes that programmatic
.focus()may not exercise:focus-visiblereliably. Treat that as an unproven/flakiness risk until the spec is actually discovered and executed; do not resolve the thread or claim GREEN from file-presence/static evidence.Current evidence boundary
Repository workflows recorded for this exact branch head are terminal-success technical evidence only for what those workflows actually executed:
32064781481: completed / success;32064781699: completed / success;32064781459: completed / success.They were generated before the current protected-base divergence was reconciled and do not prove the eventual integrated/live-base result. In particular, they do not prove the undiscovered forced-colors browser spec. No qualifying formal approval is bound to this head. Required central workflows and exact integrated-base browser/Office/coverage/package evidence must be regenerated and inspected after reconciliation. Pending, queued, skipped, cancelled, absent, neutral, failed, stale, predecessor, status-only, model-only, wrong-checkout, undiscovered-test, or synthetic-merge-only evidence is non-passing.
Dependency order / source ownership
This remains the earliest active owner for the forced-colors production/source boundary. However, former Draft #362 was merged into protected main as
057762e003fadad418c38ee748bc3e5b20e711ebdespite previously overlappingsrc/styles.cssandsrc/editorFocusStyles.test.ts. #151 must therefore reconcile its production delta against that protected-main truth before further source work or review advancement; the old instruction that #362 would consume #151 is superseded by the actual protected merge.For browser configuration specifically, Draft #380 is the existing dedicated writer. Reconcile through that owner rather than editing
tests/browser/playwright.config.tsfrom #151.Release issue #118 remains a downstream protected-release acceptance boundary; it does not authorize bypassing this PR's own dependency, browser-execution, review, or merge gates.
Integration boundary
Before any further lifecycle transition:
tests/browser/playwright.config.ts; only after its selector contract is inherited/reconciled may fix(accessibility): preserve forced-colors state cues #151 rely on forced-colors browser discovery;:focus-visiblebehavior if the risk reproduces;If the available mutation surface still lacks a safe non-destructive branch merge/update operation, classify only that reconciliation mutation
TOOL_UNAVAILABLE; do not emulate it by moving refs. Do not self-approve, weaken gates, transfer predecessor evidence, invent approval/evidence, bypass governance, or fabricate release identity.