Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 Walkthrough개요고객 앱의 로그인 흐름이 재구성되었습니다. 로그인 페이지는 단순 네비게이션 버튼에서 이메일/비밀번호 입력 필드 및 카카오 로그인 옵션을 포함한 전체 로그인 폼으로 전환되었으며, 스플래시 페이지의 리디렉션 대상이 변경 사항
예상 코드 리뷰 난이도🎯 2 (Simple) | ⏱️ ~10분 관련 가능성 있는 PR
축하 시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🧹 Nitpick comments (2)
apps/customer/src/app/login/page.tsx (2)
13-19: 입력 값을 캡처할 상태가 없습니다.현재
handleLogin에서 이메일/비밀번호 값에 접근할 방법이 없습니다. API 연동을 위해useState로 입력 값을 관리하거나,useRef로 입력 요소에 접근하는 방식이 필요합니다.♻️ useState를 사용한 상태 관리 예시
+"use client"; + +import { useState } from "react"; import { useRouter } from "next/navigation"; import { Input, Button, Icon } from "@compasser/design-system"; export default function LoginPage() { const router = useRouter(); + const [email, setEmail] = useState(""); + const [password, setPassword] = useState(""); const handleLogin = () => { - console.log("일반 로그인"); + console.log("일반 로그인", { email, password }); };그리고 Input 컴포넌트에
value와onChangeprops를 추가:<Input type="email" placeholder="이메일을 입력해주세요" value={email} onChange={(e) => setEmail(e.target.value)} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer/src/app/login/page.tsx` around lines 13 - 19, handleLogin and handleKakaoLogin currently have no access to the user's inputs; add local state (e.g., useState hooks like email and password) in the component that renders Input and Password fields, pass those state values into the Input components via value and update them via onChange handlers (or alternatively useRefs to read DOM values), then update handleLogin to read from those state variables (email, password) and use them for the API call; ensure the Input component accepts value and onChange props so the controlled inputs work with the new state.
24-45:<form>요소 사용을 권장합니다.현재 구조에서는 사용자가 Enter 키로 로그인을 제출할 수 없습니다.
<form>으로 감싸고onSubmit을 사용하면 UX가 개선됩니다.♻️ form 요소 적용 예시
-<div className="flex flex-col"> +<form className="flex flex-col" onSubmit={(e) => { e.preventDefault(); handleLogin(); }}> {/* ... inputs ... */} <div className="w-full"> <Button - type="button" + type="submit" size="lg" kind="default" variant="primary" - onClick={handleLogin} > 일반 로그인 </Button> </div> {/* ... rest ... */} -</div> +</form>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer/src/app/login/page.tsx` around lines 24 - 45, Wrap the email/password inputs and login Button inside a <form> and use the form's onSubmit instead of relying on Button onClick so Enter submits; update the Button to type="submit" (or remove its onClick) and change handleLogin to accept (or wrap it with) an event parameter that calls event.preventDefault() then performs the login logic; ensure the form element is placed around the Input components and the Button and that handleLogin is wired as onSubmit on the form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/customer/src/app/login/page.tsx`:
- Around line 25-33: The email and password fields use visual <p> tags as labels
but lack accessible associations; update the markup so each label is an actual
<label> with htmlFor matching an id on the corresponding Input or add
aria-labelledby on the Input. Specifically, for the Email and Password blocks
replace the <p> with <label htmlFor="email"> and <label htmlFor="password"> (or
give the Input an id prop like id="email" / id="password"), or alternatively add
aria-labelledby pointing to a label element; ensure the Input component
instances receive the id prop so Input (the Input component) is associated with
its label.
---
Nitpick comments:
In `@apps/customer/src/app/login/page.tsx`:
- Around line 13-19: handleLogin and handleKakaoLogin currently have no access
to the user's inputs; add local state (e.g., useState hooks like email and
password) in the component that renders Input and Password fields, pass those
state values into the Input components via value and update them via onChange
handlers (or alternatively useRefs to read DOM values), then update handleLogin
to read from those state variables (email, password) and use them for the API
call; ensure the Input component accepts value and onChange props so the
controlled inputs work with the new state.
- Around line 24-45: Wrap the email/password inputs and login Button inside a
<form> and use the form's onSubmit instead of relying on Button onClick so Enter
submits; update the Button to type="submit" (or remove its onClick) and change
handleLogin to accept (or wrap it with) an event parameter that calls
event.preventDefault() then performs the login logic; ensure the form element is
placed around the Input components and the Button and that handleLogin is wired
as onSubmit on the form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30fae1b9-7829-4123-b217-e62451237abc
⛔ Files ignored due to path filters (3)
packages/design-system/src/icons/generated/iconNames.tsis excluded by!**/generated/**packages/design-system/src/icons/generated/spriteSymbols.tsis excluded by!**/generated/**packages/design-system/src/icons/source/KakaoLogo.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
apps/customer/src/app/login/page.tsxapps/customer/src/app/splash/page.tsx
Comment on lines
+25
to
+33
| <div className="w-full"> | ||
| <p className="body2-m pb-[0.2rem] text-default">이메일</p> | ||
| <Input type="email" placeholder="이메일을 입력해주세요" /> | ||
| </div> | ||
|
|
||
| <div className="mt-[2rem] mb-[5.2rem] w-full"> | ||
| <p className="body2-m pb-[0.2rem] text-default">비밀번호</p> | ||
| <Input type="password" placeholder="비밀번호를 입력해주세요" /> | ||
| </div> |
There was a problem hiding this comment.
레이블과 입력 필드의 접근성 연결이 필요합니다.
현재 <p> 태그가 시각적 레이블로만 사용되고 있어, 스크린 리더가 입력 필드와 레이블을 연결하지 못합니다. <label>과 htmlFor/id를 사용하거나 aria-labelledby를 추가해주세요.
♿ 접근성 개선 예시
<div className="w-full">
- <p className="body2-m pb-[0.2rem] text-default">이메일</p>
- <Input type="email" placeholder="이메일을 입력해주세요" />
+ <label htmlFor="email" className="body2-m pb-[0.2rem] text-default block">이메일</label>
+ <Input id="email" type="email" placeholder="이메일을 입력해주세요" />
</div>
<div className="mt-[2rem] mb-[5.2rem] w-full">
- <p className="body2-m pb-[0.2rem] text-default">비밀번호</p>
- <Input type="password" placeholder="비밀번호를 입력해주세요" />
+ <label htmlFor="password" className="body2-m pb-[0.2rem] text-default block">비밀번호</label>
+ <Input id="password" type="password" placeholder="비밀번호를 입력해주세요" />
</div>📝 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.
Suggested change
| <div className="w-full"> | |
| <p className="body2-m pb-[0.2rem] text-default">이메일</p> | |
| <Input type="email" placeholder="이메일을 입력해주세요" /> | |
| </div> | |
| <div className="mt-[2rem] mb-[5.2rem] w-full"> | |
| <p className="body2-m pb-[0.2rem] text-default">비밀번호</p> | |
| <Input type="password" placeholder="비밀번호를 입력해주세요" /> | |
| </div> | |
| <div className="w-full"> | |
| <label htmlFor="email" className="body2-m pb-[0.2rem] text-default block">이메일</label> | |
| <Input id="email" type="email" placeholder="이메일을 입력해주세요" /> | |
| </div> | |
| <div className="mt-[2rem] mb-[5.2rem] w-full"> | |
| <label htmlFor="password" className="body2-m pb-[0.2rem] text-default block">비밀번호</label> | |
| <Input id="password" type="password" placeholder="비밀번호를 입력해주세요" /> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer/src/app/login/page.tsx` around lines 25 - 33, The email and
password fields use visual <p> tags as labels but lack accessible associations;
update the markup so each label is an actual <label> with htmlFor matching an id
on the corresponding Input or add aria-labelledby on the Input. Specifically,
for the Email and Password blocks replace the <p> with <label htmlFor="email">
and <label htmlFor="password"> (or give the Input an id prop like id="email" /
id="password"), or alternatively add aria-labelledby pointing to a label
element; ensure the Input component instances receive the id prop so Input (the
Input component) is associated with its label.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✅ 작업 내용
📝 Description
/role-select이동 연결🚀 설계 의도 및 개선점
📸 스크린샷 (선택)
📎 기타 참고사항
Fixes #40
Summary by CodeRabbit
릴리스 노트
New Features
Chores