-
Notifications
You must be signed in to change notification settings - Fork 4
[Feat] 디자인 QA 반영 #150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feat] 디자인 QA 반영 #150
Conversation
qowjdals23
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
야호야호
📝 Walkthrough개요로딩 상태 시각화를 위한 새로운 애니메이션 자산(CAT_SPINNER) 도입, 모달 버튼의 액션 및 텍스트 재배치, 주요 기업 섹션 및 로그인 페이지의 레이아웃·스타일링 개선이 포함되어 있습니다. 변경 사항
코드 리뷰 난이도🎯 2 (Simple) | ⏱️ ~12분 제안 라벨
제안 리뷰어
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
🚀 빌드 결과✅ 린트 검사 완료 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/home/major-company-section/major-company-section.tsx (1)
6-47: 로딩 UI 개선 👍 — 시멘틱/접근성 보완 권장
현재 구조도 동작은 문제없지만, 로딩 상태는 시멘틱 태그와aria-live를 부여하면 스크린리더 친화성이 좋아집니다.♿ 제안 변경
- <div className={styles.emptyWrapper}> - <img src={CAT_SPINNER} className={styles.spinner} alt="로딩중" /> - <p className={styles.spinnerText}>기업 정보를 불러오고 있어요</p> - </div> + <figure className={styles.emptyWrapper} role="status" aria-live="polite"> + <img src={CAT_SPINNER} className={styles.spinner} alt="로딩중" /> + <figcaption className={styles.spinnerText}> + 기업 정보를 불러오고 있어요 + </figcaption> + </figure>
🤖 Fix all issues with AI agents
In `@src/pages/login/login-page.css.ts`:
- Around line 14-21: 현재 CSS 객체에서 하드코딩된 border 값 "1px solid red"를 제거하거나 디자인 토큰으로
교체하세요: locate the style block that uses themeVars (the object containing
paddingTop, display, etc.) and replace the literal with an appropriate theme
token such as themeVars.color.border (or themeVars.palette.error if it was
intended as a debug/error indicator) or remove the border entirely if not
needed; ensure you reference the same style object where paddingTop:
`calc(${themeVars.height.header})` is defined so the change follows the
project's theme token conventions.
| paddingTop: `calc(${themeVars.height.header})`, | ||
| display: "flex", | ||
| flexDirection: "column", | ||
| alignItems: "center", | ||
| justifyContent: "center", | ||
| gap: "8rem", | ||
| border: "1px solid red", | ||
| height: "100vh", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
하드코딩된 border 색상은 디자인 토큰으로 교체 필요
Line 20의 "1px solid red"는 테마 토큰 규칙과 충돌합니다. 디버그 목적이 아니라면 제거하거나 themeVars 기반 색상으로 교체해주세요.
🛠️ 예시 수정
- border: "1px solid red",코딩 가이드라인에 따라.
📝 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.
| paddingTop: `calc(${themeVars.height.header})`, | |
| display: "flex", | |
| flexDirection: "column", | |
| alignItems: "center", | |
| justifyContent: "center", | |
| gap: "8rem", | |
| border: "1px solid red", | |
| height: "100vh", | |
| paddingTop: `calc(${themeVars.height.header})`, | |
| display: "flex", | |
| flexDirection: "column", | |
| alignItems: "center", | |
| justifyContent: "center", | |
| gap: "8rem", | |
| height: "100vh", |
🤖 Prompt for AI Agents
In `@src/pages/login/login-page.css.ts` around lines 14 - 21, 현재 CSS 객체에서 하드코딩된
border 값 "1px solid red"를 제거하거나 디자인 토큰으로 교체하세요: locate the style block that uses
themeVars (the object containing paddingTop, display, etc.) and replace the
literal with an appropriate theme token such as themeVars.color.border (or
themeVars.palette.error if it was intended as a debug/error indicator) or remove
the border entirely if not needed; ensure you reference the same style object
where paddingTop: `calc(${themeVars.height.header})` is defined so the change
follows the project's theme token conventions.
✏️ Summary
부스 운영 준비를 하며 발견한 몇 가지 수정 사항을 반영했습니다.
📑 Tasks
Summary by CodeRabbit
릴리스 노트
버그 수정
스타일
✏️ Tip: You can customize this high-level summary in your review settings.