fix(a11y): use native form submission paths - #835
Conversation
|
Warning Review limit reached
Next review available in: 41 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (2)
📝 WalkthroughWalkthrough프로젝트와 연결 생성 UI를 네이티브 form 제출 방식으로 변경했습니다. Enter와 클릭은 단일 guarded 경로를 사용합니다. 다이어그램과 캔버스 검색은 Changes네이티브 폼 제출
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
@coderabbitai review |
|
|
@coderabbitai review Please perform a fresh independent review of unchanged exact head |
|
|
|
@coderabbitai review Please review exact head |
|
|
|
Exact-current-head independent review request for @opencode-agent @cwl-noema-review Please provide a qualifying non-author review of this unchanged accessibility fix. CI |
|
Exact-head accessibility remediation is published at The search forms introduced by this PR exposed anonymous Verification on the published tree:
Local runtime was Node 24.14 while the repository requires Node 26, so exact-head CI is authoritative. The required CodeGraph executable was unavailable in this runner; call-site search and the complete suite were used. @coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Checked-out revision =='
git rev-parse HEAD
git status --short
printf '%s\n' '== PR metadata and checks =='
gh pr view 835 --repo ContextualWisdomLab/pg-erd-cloud \
--json number,headRefOid,baseRefOid,changedFiles,mergeStateStatus,reviews,reviewDecision,statusCheckRollup \
--jq '{number, headRefOid, baseRefOid, changedFiles, mergeStateStatus, reviewDecision, reviews: [.reviews[] | {author: .author.login, state}], checks: [.statusCheckRollup[] | {name, status, conclusion}]}'
printf '%s\n' '== Changed files from merge-base =='
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD HEAD~1)"
git diff --name-status "$base" HEAD
printf '%s\n' '== Commit range =='
git log --oneline --decorate "$base..HEAD"Length of output: 3501 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD HEAD~1)"
printf '%s\n' '== Exact PR diff =='
git diff --find-renames --find-copies --unified=80 "$base" HEAD -- \
CHANGELOG.md \
docs/doctoring/native-form-submission.md \
frontend/src/App.tsx \
frontend/src/App.coverage.test.tsx \
frontend/src/erd/__tests__/App.editTable.test.tsx
printf '%s\n' '== App source structure =='
ast-grep outline frontend/src/App.tsx --items all --view expanded
printf '%s\n' '== Updated test structure =='
ast-grep outline frontend/src/App.coverage.test.tsx --items all --view expanded
ast-grep outline frontend/src/erd/__tests__/App.editTable.test.tsx --items all --view expandedLength of output: 64391 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Form handlers and surrounding state =='
sed -n '112,190p' frontend/src/App.tsx
sed -n '850,985p' frontend/src/App.tsx
printf '%s\n' '== Rendered form containment and search controls =='
sed -n '1010,1275p' frontend/src/App.tsx
sed -n '1275,1445p' frontend/src/App.tsx
printf '%s\n' '== Styles that target changed wrapper classes or element types =='
rg -n -C 4 --glob '*.{css,scss,sass,less}' \
'(\.inlineCreate|\.workspaceSearch|\.canvasToolbar__search|form|label)' \
frontend
printf '%s\n' '== Precise direct references to changed handlers and wrappers =='
rg -n -C 3 \
'\b(onCreateProject|onCreateProjectSubmit|onCreateConnection|onCreateConnectionSubmit|onSearchSubmit)\b|className="(inlineCreate|workspaceSearch|canvasToolbar__search)"|role="search"' \
frontend/src
printf '%s\n' '== Test cases that submit changed forms or query search landmarks =='
rg -n -C 4 \
"(submits the editor project form|project-list creation|connection once|non-navigating search|role\('search'|getByRole\('search'|user\.type\(.*\{enter\})" \
frontend/src/App.coverage.test.tsx frontend/src/erd/__tests__/App.editTable.test.tsxLength of output: 41284
I found no blocking source-level issue. Verified items:
I could not execute the frontend suite, typecheck, or build in this sandbox. I reviewed the test changes and source statically. At review time, GitHub reported frontend, backend, Semgrep, and CodeQL-related checks as You are interacting with an AI system. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/App.coverage.test.tsx (2)
355-360: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win비동기 재진입 guard를 handler 수준에서 직접 검증하세요.
첫 연결 요청이 시작되면
isCreatingConnection때문에 submit button이 disabled 됩니다. 따라서 두 번째user.type(...{enter})는 브라우저의 암시적 제출 단계에서 차단될 수 있습니다. 호출 횟수가 1이라는 결과만으로onCreateConnectionSubmit의 guard가 실행됐다고 볼 수 없습니다. Promise가 pending인 동안dsnInput.closest('form')에fireEvent.submit(...)을 실행해 handler guard를 직접 검증하세요.As per coding guidelines, “Add or update focused tests when changing behavior” 규칙에 따라 비동기 재진입 guard를 직접 검증해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/App.coverage.test.tsx` around lines 355 - 360, Update the connection-creation test to verify the handler-level asynchronous re-entry guard directly: while the first createConnection promise remains pending, submit dsnInput.closest('form') with fireEvent.submit rather than relying on user.type(...{enter}). Keep the assertion that api.createConnection is called only once, ensuring onCreateConnectionSubmit itself rejects the second submission.Source: Coding guidelines
370-373: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win검색 form의
preventDefault()를 직접 검증하세요.현재 테스트는 필터 결과와
role="search"만 확인합니다.onSearchSubmit의preventDefault()가 제거되어도 jsdom은 실제 페이지 이동을 수행하지 않으므로 테스트가 통과할 수 있습니다.fireEvent.submit(searchForm)의 취소 결과 또는defaultPrevented를 확인하고, 다이어그램과 캔버스 form 모두에 적용하세요.As per coding guidelines, “Add or update focused tests when changing behavior” 규칙에 따라 검색 제출 취소 동작을 직접 검증해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/App.coverage.test.tsx` around lines 370 - 373, 검색 제출 테스트에서 결과 필터링이나 ARIA 역할이 아닌 제출 취소 동작을 직접 검증하도록 업데이트하세요. 검색 form과 캔버스 form 각각의 `onSearchSubmit` 경로에 대해 `fireEvent.submit`의 취소 결과 또는 이벤트의 `defaultPrevented`를 확인하고, 기존 다이어그램 검색 검증도 유지하세요.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/App.coverage.test.tsx`:
- Around line 355-360: Update the connection-creation test to verify the
handler-level asynchronous re-entry guard directly: while the first
createConnection promise remains pending, submit dsnInput.closest('form') with
fireEvent.submit rather than relying on user.type(...{enter}). Keep the
assertion that api.createConnection is called only once, ensuring
onCreateConnectionSubmit itself rejects the second submission.
- Around line 370-373: 검색 제출 테스트에서 결과 필터링이나 ARIA 역할이 아닌 제출 취소 동작을 직접 검증하도록
업데이트하세요. 검색 form과 캔버스 form 각각의 `onSearchSubmit` 경로에 대해 `fireEvent.submit`의 취소 결과
또는 이벤트의 `defaultPrevented`를 확인하고, 기존 다이어그램 검색 검증도 유지하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e3a3d636-9476-4d52-8463-09094767cb3b
📒 Files selected for processing (5)
CHANGELOG.mddocs/doctoring/native-form-submission.mdfrontend/src/App.coverage.test.tsxfrontend/src/App.tsxfrontend/src/erd/__tests__/App.editTable.test.tsx
|
Addressed the exact-head CodeRabbit test-coverage findings at
Mutation RED evidence:
GREEN evidence after restoring production behavior:
@coderabbitai review the unchanged exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='e24952fbb025ef0b256b05225c8263486c7a991f'
printf '%s\n' '== Revision =='
git rev-parse HEAD
git status --short
git cat-file -e "${target}^{commit}"
git merge-base --is-ancestor "$target" HEAD && printf '%s\n' 'target_is_ancestor_of_HEAD=yes'
printf '%s\n' '== Commit and incremental diff =='
git show --no-ext-diff --format=fuller --stat "$target"
git diff --no-ext-diff --unified=80 "${target}^" "$target" -- \
frontend/src/App.tsx \
frontend/src/App.coverage.test.tsx \
frontend/src/erd/__tests__/App.editTable.test.tsx
printf '%s\n' '== Relevant source and test structure =='
ast-grep outline frontend/src/App.tsx --items all --view expanded
ast-grep outline frontend/src/App.coverage.test.tsx --items all --view expanded
printf '%s\n' '== Relevant handlers and coverage tests =='
rg -n -C 8 \
'\b(isCreatingConnection|onCreateConnectionSubmit|onSearchSubmit|createConnection)\b|fireEvent\.submit|pending connection|non-navigating search|search.*form' \
frontend/src/App.tsx frontend/src/App.coverage.test.tsx frontend/src/erd/__tests__/App.editTable.test.tsx
printf '%s\n' '== PR checks and head =='
gh pr view 835 --repo ContextualWisdomLab/pg-erd-cloud \
--json headRefOid,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{headRefOid, mergeStateStatus, reviewDecision, checks: [.statusCheckRollup[] | {name, status, conclusion}]}'Length of output: 45260
I found no blocking source-level issue. The revised connection test reaches The revised search tests verify that both named search forms cancel The removed GitHub still reports required checks as If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
|
CI RCA and remediation are published at exact head Failure boundary:
Cause:
Remedy:
Verification:
Local Node remains 24.14 while the repository requires Node 26; this new exact-head CI is authoritative. @coderabbitai review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='b16ab830da661ab32455233060414a4a75b152ef'
repo='ContextualWisdomLab/pg-erd-cloud'
printf '%s\n' '== Exact revision =='
git rev-parse HEAD
git cat-file -e "${target}^{commit}"
git merge-base --is-ancestor "$target" HEAD && echo 'target_is_ancestor_of_HEAD=yes'
git status --short
printf '%s\n' '== Incremental commit and diff =='
git show --no-ext-diff --format=fuller --stat "$target"
git diff --no-ext-diff --unified=70 "${target}^" "$target" -- \
frontend/src/App.coverage.test.tsx \
frontend/src/App.tsx \
frontend/src/erd/__tests__/App.editTable.test.tsx
printf '%s\n' '== Test-file outline =='
ast-grep outline frontend/src/App.coverage.test.tsx --items all --view expanded
printf '%s\n' '== Fake timer and diagram-list test contexts =='
rg -n -C 12 \
"(useFakeTimers|useRealTimers|findAllByRole\('button'.*열기|findAllByRole\(\"button\".*열기|auto-layout|자동|listSnapshots)" \
frontend/src/App.coverage.test.tsx
printf '%s\n' '== Exact-head PR status checks =='
gh pr view 835 --repo "$repo" \
--json headRefOid,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{headRefOid, mergeStateStatus, reviewDecision, checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}]}'
printf '%s\n' '== Referenced workflow run =='
gh run view 31514213291 --repo "$repo" \
--json headSha,status,conclusion,jobs \
--jq '{headSha,status,conclusion,jobs: [.jobs[] | {name,status,conclusion,databaseId}]}'Length of output: 41367
I found no blocking source-level issue. The commit changes only Each affected test now waits for the diagram-list The prior failed workflow run I cannot approve yet. On the reviewed head, ✏️ Learnings added
You are interacting with an AI system. |
Outcome
Keyboard and assistive-technology users can submit project and connection creation from focused inputs with Enter through the same guarded path used by button activation. Diagram and canvas search controls are semantic search forms that prevent navigation while preserving live filtering.
Changes
role="search"formsTest-first evidence
The focused regression initially failed because Enter did not call
createProject. After implementation:App.coverage.test.tsx: 21 passedgit diff --check: passedThe repository-wide coverage report remains at its existing aggregate baseline (99.8% statements / 99.55% branches); this change introduces no uncovered App branch.
Closes #756
Summary by CodeRabbit
개선 사항
문서