fix: 지원자 현황 카드 토글 영역 수정#603
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a7f398976
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Link href={resolvedDetailHref} className="min-w-0 flex-1 truncate text-k-900 typo-sb-7"> | ||
| <Link | ||
| href={resolvedDetailHref} | ||
| className="min-w-0 flex-1 truncate text-k-900 typo-sb-7" |
There was a problem hiding this comment.
When a university name is shorter than the row, this <Link> still has flex-1, so it expands across all remaining header space between the icon and arrow. Tapping that blank area will navigate to the detail page and stopPropagation() prevents the new parent toggle from running, which contradicts the intended split where only the school-name text is the link and surrounding header whitespace toggles the applicant list.
Useful? React with 👍 / 👎.
Walkthrough
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@apps/web/src/app/university/application/ScoreSheet.tsx`:
- Around line 78-96: The score sheet header currently toggles only on mouse
click via the clickable div, so add keyboard support in ScoreSheet while keeping
the existing toggle behavior. Prefer refactoring the header container into a
single interactive element that can be focused and activated by keyboard without
nesting the existing Link and button, or at minimum add role, tabIndex, and an
onKeyDown handler for Enter/Space to the header wrapper and ensure
toggleTableOpened is invoked consistently. Use the existing toggleTableOpened,
resolvedDetailHref, and the header Link/button structure in ScoreSheet to locate
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d66e39c7-a531-44f2-a824-920289b9837d
📒 Files selected for processing (1)
apps/web/src/app/university/application/ScoreSheet.tsx
| className={clsx( | ||
| "flex min-h-11 w-full cursor-pointer items-center gap-2.5 px-4", | ||
| tableOpened ? "bg-secondary-100" : "bg-k-50", | ||
| )} | ||
| onClick={toggleTableOpened} | ||
| > | ||
| <span className="size-5 shrink-0 rounded-full bg-white" aria-hidden /> | ||
| <Link href={resolvedDetailHref} className="min-w-0 flex-1 truncate text-k-900 typo-sb-7"> | ||
| <Link | ||
| href={resolvedDetailHref} | ||
| className="min-w-0 flex-1 truncate text-k-900 typo-sb-7" | ||
| onClick={(event) => event.stopPropagation()} | ||
| > | ||
| {scoreSheet.koreanName} ({scoreSheet.applicants.length}/{capacity}) | ||
| </Link> | ||
| <button | ||
| onClick={() => setTableOpened((prev) => !prev)} | ||
| onClick={(event) => { | ||
| event.stopPropagation(); | ||
| toggleTableOpened(); | ||
| }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
1. 헤더 div의 키보드 접근성 보완이 필요해요
헤더 <div>에 onClick와 cursor-pointer를 추가해 마우스 사용자에게는 전체 영역이 토글 영역으로 인식되지만, role="button", tabIndex, onKeyDown 등 키보드 접근성 속성이 없습니다. 키보드 사용자는 내부 작은 버튼으로만 토글할 수 있어, PR 의도인 "헤더 영역 클릭으로 토글" 경험이 마우스 사용자에게만 제공됩니다.
다만 div에 role="button"을 추가하면 내부에 이미 <button>과 <Link>(보조 기술에서 interactive 요소로 인식)이 존재해 중첩 interactive 요소 문제가 발생할 수 있으므로, 아래 두 방안 중 하나를 고려해 보세요.
방안 A (권장): 헤더 전체를 <button>으로 변경하고 Link를 <span> + useRouter 기반 네비게이션으로 대체
- 중첩 interactive 요소를 피하면서 헤더 전체에 키보드 접근성을 부여합니다.
방안 B (최소 변경): div에 키보드 핸들러 추가
role="button",tabIndex={0},onKeyDown에서 Enter/Space 시toggleTableOpened()호출. 단, 스크린 리더 사용자에게 중첩 interactive 요소 경고가 발생할 수 있습니다.
♿ 방안 B 최소 변경 제안
<div
className={clsx(
"flex min-h-11 w-full cursor-pointer items-center gap-2.5 px-4",
tableOpened ? "bg-secondary-100" : "bg-k-50",
)}
onClick={toggleTableOpened}
+ role="button"
+ tabIndex={0}
+ onKeyDown={(event) => {
+ if (event.key === "Enter" || event.key === " ") {
+ event.preventDefault();
+ event.stopPropagation();
+ toggleTableOpened();
+ }
+ }}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={clsx( | |
| "flex min-h-11 w-full cursor-pointer items-center gap-2.5 px-4", | |
| tableOpened ? "bg-secondary-100" : "bg-k-50", | |
| )} | |
| onClick={toggleTableOpened} | |
| > | |
| <span className="size-5 shrink-0 rounded-full bg-white" aria-hidden /> | |
| <Link href={resolvedDetailHref} className="min-w-0 flex-1 truncate text-k-900 typo-sb-7"> | |
| <Link | |
| href={resolvedDetailHref} | |
| className="min-w-0 flex-1 truncate text-k-900 typo-sb-7" | |
| onClick={(event) => event.stopPropagation()} | |
| > | |
| {scoreSheet.koreanName} ({scoreSheet.applicants.length}/{capacity}) | |
| </Link> | |
| <button | |
| onClick={() => setTableOpened((prev) => !prev)} | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| toggleTableOpened(); | |
| }} | |
| className={clsx( | |
| "flex min-h-11 w-full cursor-pointer items-center gap-2.5 px-4", | |
| tableOpened ? "bg-secondary-100" : "bg-k-50", | |
| )} | |
| onClick={toggleTableOpened} | |
| role="button" | |
| tabIndex={0} | |
| onKeyDown={(event) => { | |
| if (event.key === "Enter" || event.key === " ") { | |
| event.preventDefault(); | |
| event.stopPropagation(); | |
| toggleTableOpened(); | |
| } | |
| }} | |
| > | |
| <span className="size-5 shrink-0 rounded-full bg-white" aria-hidden /> | |
| <Link | |
| href={resolvedDetailHref} | |
| className="min-w-0 flex-1 truncate text-k-900 typo-sb-7" | |
| onClick={(event) => event.stopPropagation()} | |
| > | |
| {scoreSheet.koreanName} ({scoreSheet.applicants.length}/{capacity}) | |
| </Link> | |
| <button | |
| onClick={(event) => { | |
| event.stopPropagation(); | |
| toggleTableOpened(); | |
| }} |
🤖 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 `@apps/web/src/app/university/application/ScoreSheet.tsx` around lines 78 - 96,
The score sheet header currently toggles only on mouse click via the clickable
div, so add keyboard support in ScoreSheet while keeping the existing toggle
behavior. Prefer refactoring the header container into a single interactive
element that can be focused and activated by keyboard without nesting the
existing Link and button, or at minimum add role, tabIndex, and an onKeyDown
handler for Enter/Space to the header wrapper and ensure toggleTableOpened is
invoked consistently. Use the existing toggleTableOpened, resolvedDetailHref,
and the header Link/button structure in ScoreSheet to locate the change.
1 participant
관련 이슈
작업 내용
검증
pnpm --filter @solid-connect/web run lint:checkpnpm --filter @solid-connect/web run typecheck:ci@solid-connect/webCI parity checks 통과특이 사항
리뷰 요구사항 (선택)