a11y(frontend): expose unavailable table-save state to keyboard users - #704
a11y(frontend): expose unavailable table-save state to keyboard users#704seonghobae wants to merge 29 commits into
Conversation
- 네이티브 disabled 속성을 aria-disabled로 대체하여 스크린 리더 포커스 유지 - 버튼 비활성화 시 opacity 및 cursor 스타일링 추가 - 비활성화 상태에서 클릭 시 e.preventDefault()로 폼 제출 차단
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Comment |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headb2c9117777e2552eb79e39b2ffcd4aeecee6e0cc. -
Head SHA:
b2c9117777e2552eb79e39b2ffcd4aeecee6e0cc -
Workflow run: 30766336583
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Frontend (2 files)"]
S1 --> I1["browser runtime and bundle"]
I1 --> R1["Review risk: Frontend (2 files)"]
R1 --> V1["frontend tests"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Frontend (2 files)"]
S1 --> I1["browser runtime and bundle"]
I1 --> R1["Review risk: Frontend (2 files)"]
R1 --> V1["frontend tests"]
|
|
@jules Please add a focused regression-test commit before merge:
|
Regression tests have been added to ModalCoverage.test.tsx as requested. The tests confirm that the empty/whitespace-only Save button has |
- 빈 입력값일 때 `aria-disabled="true"` 및 시각적 비활성화 스타일 확인 테스트 추가 - 비활성화 상태에서 클릭, Enter, Space 및 직접 폼 제출 시 콜백이 실행되지 않음을 검증 - 유효한 입력값일 때 정상적으로 활성화 상태를 갖추고 제출 이벤트가 실행되는지 테스트 검증
|
@jules The form-level guard is correct, but |
Test coverage for keyboard interactions has been updated using |
- `fireEvent.keyDown` 대신 `@testing-library/user-event`를 사용하여 실제 네이티브 키보드 활성화(Enter, Space) 동작 모의 구현 - 유효한 이름(활성화 상태) 시 키보드 입력 각각이 정확히 이벤트를 호출하는지 검증하는 로직 추가 - 비활성화 시 콜백이 차단되는 기존 검증을 userEvent 기반으로 수정하여 엄격성 확보
- `App.coverage.test.tsx`에서 stale snapshot 발생을 막기 위해 `act()` 렌더링 스케줄링을 적절히 `await`하여 안정화 - `vi.useFakeTimers()`를 비동기 엘리먼트 쿼리(`findAllByRole`) 이후로 재배치하여 무한 대기(Timeout)가 발생하는 문제를 수정
seonghobae
changed the title
Pull request was converted to draft
|
@coderabbitai review |
✅ Action performedReview finished.
|
- `App.coverage.test.tsx`에서 stale snapshot 발생을 막기 위해 `act()` 렌더링 스케줄링을 적절히 `await`하여 안정화 - `vi.useFakeTimers()`를 비동기 엘리먼트 쿼리(`findAllByRole`) 이후로 재배치하여 무한 대기(Timeout)가 발생하는 문제를 수정
Pull request was converted to draft
|
@opencode-agent @cwl-noema-review Please independently review exact current head |
- `App.coverage.test.tsx`에서 stale snapshot 발생을 막기 위해 `act()` 렌더링 스케줄링을 적절히 `await`하여 안정화 - `vi.useFakeTimers()`를 비동기 엘리먼트 쿼리(`findAllByRole`) 이후로 재배치하여 무한 대기(Timeout)가 발생하는 문제를 수정
|
@opencode-agent Please review exact current head |
- `App.coverage.test.tsx`에서 stale snapshot 발생을 막기 위해 `act()` 렌더링 스케줄링을 적절히 `await`하여 안정화 - `vi.useFakeTimers()`를 비동기 엘리먼트 쿼리(`findAllByRole`) 이후로 재배치하여 무한 대기(Timeout)가 발생하는 문제를 수정
- 프로젝트 내 패키지 매니저가 pnpm으로 강제되나, 저장소 내에 잔존하던 `package-lock.json`으로 인해 CI의 OSV 스캐너가 오래된 종속성(`undici`)의 취약점을 탐지하고 실패하는 문제를 해결 - `frontend/package-lock.json` 파일을 삭제하여 의도치 않은 패키지 추적 방지 - `frontend/package.json`에 `"packageManager": "pnpm@10.30.3"` 필드를 명시하여 CI `coverage-evidence` 스크립트 등 pnpm 엄격성 제약을 준수하도록 수정
- `.github/workflows/ci.yml`이 `pnpm` 캐시를 위해 의존하는 `frontend/pnpm-lock.yaml` 파일 누락 오류를 수정 - `pnpm install`을 실행하여 락파일을 생성 및 커밋에 포함하여 CI 의존성 설치 캐시 에러가 발생하지 않도록 해결
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- `.github/workflows/ci.yml`에서 npm 설치 명령(`npm ci`)을 `pnpm install --frozen-lockfile`로 변경 - Node 캐시 액션의 의존성 관리 도구를 `pnpm`으로 변경 - 스크립트 실행 명령어(`npm run ...`)를 모두 `pnpm run` 기반으로 수정
- PR 리뷰 과정에서 발생한 의도치 않은 package.json/package-lock.json (락파일 등 종속성 버전 업그레이드) 변경 사항 롤백 - Strix CI에서 실패한 UI 검증 테스트 렌더링 에러 해결을 위해 `AddTableModal.a11y.test.tsx` 테스트 스크립트에 `cleanup` 적용 추가 - Strix가 테스트 환경에 강제한 `aria-describedby` 등의 상태가 올바르게 검증될 수 있도록 보장 - `CHANGELOG.md`의 줄바꿈 규칙 복구
|
@opencode-agent @cwl-noema-review Please perform a fresh independent review of current head |
- `AddTableModal.a11y.test.tsx`에서 다중 컴포넌트 렌더링 충돌로 인한 `getByRole` 에러를 수정하기 위해 `afterEach(cleanup)` 명시적 추가 - 스코프 밖의 package.json, package-lock.json 등 패키지 종속성 자동 업그레이드 내용을 롤백하여 Strix 보안 게이트 통과 유지
2 participants
What
Keep the Add Table modal's save action in the tab order when the table name is blank. The button now exposes
aria-disabled, retains a clear unavailable visual state, and blocks click and submit behavior until a non-blank name is present.Why
A natively disabled button cannot receive keyboard focus, which prevents assistive-technology users from discovering the action and its current state.
aria-disabledpreserves discoverability while explicit event guards preserve the business rule.Regression coverage
aria-disabled="true".aria-disabled="false"and submits exactly once.The required current-head frontend workflow must pass
npm ci, typecheck, the complete 100%-coverage suite, and the production build before merge.Originally created by Jules for task 5390012025943726862.