[feature] dark_clear_button_icon 통일 및 테두리 추가#1833
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @suhyun113's task in 3m 5s —— View job 🐿️ harry 리뷰전반적으로 깔끔한 리팩터링이에요. 흩어져 있던 포커스 감지·클리어 버튼· 확인만 부탁드리는 2가지를 인라인으로 남겼어요 (룰 위반은 아니에요):
리뷰 요청하신 정리하면 머지 막을 만한 이슈는 없고, 위 2건만 확인해 주시면 좋겠어요. 수고하셨어요! 🙌 |
|
Warning Review limit reached
Next review available in: 39 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough모바일 관리자 입력 UI에 Changes모바일 입력 컴포넌트 통합
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Admin
participant TextField
participant EditField
participant ClearableTextArea
participant FormState
Admin->>TextField: 입력 또는 지우기
TextField->>EditField: 입력 영역 렌더링
EditField->>ClearableTextArea: value, onChange, maxLength 전달
ClearableTextArea->>FormState: 변경된 문자열 전달
ClearableTextArea->>TextField: onClear 호출 및 포커스 복귀
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
suhyun113
changed the title
![reviewer-harry[bot]](https://avatars.githubusercontent.com/in/4158742?s=60&v=4){kind=small}
| <ClearableTextArea | ||
| value={value} | ||
| onChange={onChange} | ||
| onClear={onClear} | ||
| placeholder={placeholder ?? label} | ||
| maxLength={maxLength} | ||
| size='large' | ||
| /> |
There was a problem hiding this comment.
ClearableTextArea로 통합하면서 높이 동작이 바뀐 점을 확인해 주세요. 기존 TextField는 아래처럼 최대 2줄까지만 늘어나고 그 이상은 스크롤되도록 캡을 두고 있었어요.
textarea.style.height = `${Math.min(textarea.scrollHeight, lineHeight * 2)}px`;그런데 ClearableTextArea가 쓰는 useAutoGrow는 el.scrollHeight 그대로 세팅해서 줄 수 제한 없이 무한정 늘어나요. 동아리명·한줄소개처럼 짧게 유지되길 기대하는 필드에서 여러 줄 입력 시 카드가 계속 늘어나게 되는데, 이 2줄 캡 제거가 의도된 UX 변경인지 확인 부탁해요. 유지가 필요하면 ClearableTextArea에 maxRows 같은 옵션을 두는 것도 방법이에요.
|
|
||
| interface ClearableTextAreaProps { | ||
| value: string; | ||
| onChange: (value: string) => void; |
There was a problem hiding this comment.
리뷰 요청하신 onClear 설계 관련해서요. 값 초기화는 내부에서 onChange('')로 처리하고 외부는 사이드이펙트만 담당하는 구조 자체는 괜찮아요 — 호출부에서 setState('')를 매번 안 써도 돼서 실수 여지도 줄고요.
다만 이름이 onClear라 "이걸 넘기면 클리어가 동작한다"로 읽히는데, 실제로는 클리어는 항상 내부에서 되고 이 콜백은 클리어 이후 부가 동작(트래킹) 전용이에요. 의미가 더 드러나도록 onCleared(완료 시점 뉘앙스)로 바꾸거나, 주석 한 줄 달아두면 나중에 헷갈릴 여지가 줄 것 같아요.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a reusable ClearableTextArea component and refactors several mobile admin forms (such as TextField, ClubIntroEditTabMobile, and FAQSection) to use it, simplifying the codebase and removing redundant auto-grow and clear button logic. Additionally, EditField was refactored to use CSS :focus-within for styling instead of an isActive prop. The review feedback suggests enhancing the keyboard accessibility of ClearableTextArea by replacing the state-based isFocused rendering of the clear button with CSS :focus-within to prevent tab-navigation issues. It also recommends removing the unused rows prop from ClearableTextArea since its height is dynamically controlled by the useAutoGrow hook.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const [isFocused, setIsFocused] = useState(false); | ||
| const textareaRef = useAutoGrow(value); | ||
|
|
||
| const handleClear = (e: React.MouseEvent) => { | ||
| e.preventDefault(); | ||
| onChange(''); | ||
| onClear?.(); | ||
| textareaRef.current?.focus(); | ||
| }; | ||
|
|
||
| return ( | ||
| <Styled.Row> | ||
| <Styled.Textarea | ||
| ref={textareaRef} | ||
| value={value} | ||
| onChange={(e) => onChange(e.target.value)} | ||
| placeholder={placeholder} | ||
| maxLength={maxLength} | ||
| rows={rows} | ||
| $size={size} | ||
| onFocus={() => setIsFocused(true)} | ||
| onBlur={() => setIsFocused(false)} | ||
| /> | ||
| {isFocused && value.length > 0 && ( | ||
| <Styled.ClearButton | ||
| type='button' | ||
| onMouseDown={handleClear} | ||
| aria-label='지우기' | ||
| > | ||
| <ClearButtonIcon /> | ||
| </Styled.ClearButton> | ||
| )} | ||
| </Styled.Row> | ||
| ); |
There was a problem hiding this comment.
현재 구현에서는 isFocused 상태를 사용하여 클리어 버튼을 조건부 렌더링하고 있습니다. 이 방식은 마우스 클릭 시 onMouseDown과 e.preventDefault()를 사용해 포커스 이탈을 막아 동작하지만, 키보드 사용자가 Tab 키를 통해 클리어 버튼으로 이동하려고 할 때 textarea가 blur되면서 버튼이 즉시 언마운트되어 **키보드 접근성(Accessibility)**이 차단되는 문제가 있습니다.
이를 해결하기 위해 React의 isFocused 상태를 제거하고, CSS의 :focus-within 가상 클래스를 활용하여 포커스 여부에 따라 클리어 버튼의 노출 여부를 제어하는 방식을 제안합니다. 이렇게 하면:
- 키보드 사용자가
Tab키로 클리어 버튼에 정상적으로 접근하여Enter나Space로 입력값을 지울 수 있습니다. useState,onFocus,onBlur및onMouseDown의preventDefault우회 로직이 필요 없어져 코드가 훨씬 단순해집니다.
주의: 이 제안을 적용하려면 ClearableTextArea.styles.ts 파일의 ClearButton 스타일도 함께 수정해야 합니다.
/* ClearableTextArea.styles.ts */
export const ClearButton = styled.button`
display: none;
/* 기존 스타일 유지 */
${Row}:focus-within & {
display: flex;
}
`; const textareaRef = useAutoGrow(value);
const handleClear = () => {
onChange('');
onClear?.();
textareaRef.current?.focus();
};
return (
<Styled.Row>
<Styled.Textarea
ref={textareaRef}
value={value}
onChange={(e) => onChange(e.target.value)}
placeholder={placeholder}
maxLength={maxLength}
rows={rows}
$size={size}
/>
{value.length > 0 && (
<Styled.ClearButton
type='button'
onClick={handleClear}
aria-label='지우기'
>
<ClearButtonIcon />
</Styled.ClearButton>
)}
</Styled.Row>
);
| onClear?: () => void; | ||
| placeholder?: string; | ||
| maxLength?: number; | ||
| rows?: number; |
There was a problem hiding this comment.
ClearableTextArea 컴포넌트는 내부적으로 useAutoGrow 훅을 사용하여 입력값(value)의 변화에 따라 높이를 동적으로 조절하고 있습니다. useAutoGrow는 내부적으로 style.height를 scrollHeight로 강제 설정하기 때문에, 외부에서 전달하는 rows 프로퍼티는 실제 렌더링 시 무시되고 항상 콘텐츠 크기에 맞게 축소/확장됩니다.
현재 모든 사용처에서 rows를 별도로 지정하지 않고 기본값(1)으로만 사용하고 있으므로, 혼선을 방지하기 위해 rows 프로퍼티를 인터페이스와 컴포넌트 스펙에서 제거하는 것을 권장합니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
frontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.tsx (1)
40-40: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win한국어 입력 시
maxLength초과 방지 로직 추가모바일(특히 안드로이드) 환경에서 한국어 등 조합형 언어를 입력할 때, 브라우저의 기본
maxLength속성이 텍스트 조합 과정의 입력을 완벽히 제어하지 못해 제한된 길이를 초과하는 이슈가 자주 발생합니다.
안정적인 글자 수 제한을 위해onChange핸들러 내부에서 상태 업데이트 전maxLength를 한 번 더 체크하고 자르는 처리를 추가하는 것을 권장합니다.💡 제안하는 수정안
- onChange={(e) => onChange(e.target.value)} + onChange={(e) => { + const newValue = e.target.value; + if (maxLength && newValue.length > maxLength) { + onChange(newValue.slice(0, maxLength)); + } else { + onChange(newValue); + } + }}🤖 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/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.tsx` at line 40, Update the ClearableTextArea onChange handler to enforce maxLength before calling onChange, truncating e.target.value when it exceeds the configured limit while preserving the full value when no limit is set or it is within bounds.frontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.styles.ts (1)
23-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
setTypography의line-height오버라이딩 확인
setTypography믹스인 내부에서 이미 폰트 사이즈와 함께line-height가 지정되고 있습니다. 바로 뒤에line-height: 160%;를 추가하면 테마에서 정의한 줄간격 값이 덮어씌워지게 됩니다.
해당 컴포넌트만의 특별한 디자인 요구사항으로 인한 의도적인 덮어쓰기라면 괜찮지만, 그렇지 않다면line-height: 160%;를 제거하여 디자인 시스템의 일관성을 유지하는 것을 권장합니다.🤖 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/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.styles.ts` around lines 23 - 27, Remove the explicit line-height: 160% declaration following the setTypography mixin in ClearableTextArea styles, allowing the typography theme’s line-height to remain effective for both size variants.frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/TextField/TextField.tsx (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value절대 경로 import 사용 (선택 사항)
코딩 가이드라인에 따라
src/*경로에 매핑되는@/*경로 별칭을 사용하는 것을 권장합니다.💡 제안하는 수정
import ClearableTextArea from '`@/pages/AdminPage/components/ClearableTextArea/ClearableTextArea`'; -import EditField from '../EditField/EditField'; +import EditField from '`@/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField`';🤖 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/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/TextField/TextField.tsx` around lines 1 - 3, Update the import in the TextField component to use the configured `@/`* alias for the ClearableTextArea module instead of its current relative path, while leaving the EditField import unchanged.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/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.styles.ts`:
- Around line 23-27: Remove the explicit line-height: 160% declaration following
the setTypography mixin in ClearableTextArea styles, allowing the typography
theme’s line-height to remain effective for both size variants.
In
`@frontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.tsx`:
- Line 40: Update the ClearableTextArea onChange handler to enforce maxLength
before calling onChange, truncating e.target.value when it exceeds the
configured limit while preserving the full value when no limit is set or it is
within bounds.
In
`@frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/TextField/TextField.tsx`:
- Around line 1-3: Update the import in the TextField component to use the
configured `@/`* alias for the ClearableTextArea module instead of its current
relative path, while leaving the EditField import unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8abd69ac-0a08-419b-b8ff-ee77cb5f31de
⛔ Files ignored due to path filters (1)
frontend/src/assets/images/icons/dark_clear_button_icon.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
frontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.styles.tsfrontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/ClubInfoEditTabMobile.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.stories.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.styles.tsfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/LinkEditPage/LinkField.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/TextField/TextField.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTabMobile.styles.tsfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTabMobile.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/AwardEditPage/AwardSemesterCard.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/FAQSection/FAQSection.styles.tsfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/FAQSection/FAQSection.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/InfoSection/InfoSection.styles.ts
💤 Files with no reviewable changes (2)
- frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.stories.tsx
- frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTabMobile.styles.ts
|
Claude finished @suhyun113's task in 55s —— View job 🐿️ harry 리뷰 (synchronize)이번 푸시(
확인 결과, 문서가 실제 구현과 잘 일치해요.
문서만 바뀐 범위라 룰에 어긋나는 부분은 없어요. 코드 로직에 대한 코멘트(2줄 캡 제거· 문서를 코드와 같이 꼼꼼히 갱신해 주셔서 좋았어요. 통과예요! 🙌
|
4b16705 to
383f1ba
Compare
✅ UI 변경사항 없음
전체 88개 스토리 · 32개 컴포넌트 |
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 `@frontend/docs/features/admin/info/field-components.md`:
- Line 34: Update the EditField + ClearableTextArea documentation to distinguish
responsibilities: document that the focus border comes from the parent Card’s
&:focus-within in EditField.styles.ts, while ClearableTextArea provides only the
X button and useAutoGrow behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02107c32-5f8c-46ac-be6a-e6259a3356a7
⛔ Files ignored due to path filters (1)
frontend/src/assets/images/icons/dark_clear_button_icon.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
frontend/docs/features/admin/info/field-components.mdfrontend/docs/features/admin/intro/mobile.mdfrontend/docs/features/components/ClearableTextArea.mdfrontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.styles.tsfrontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/ClubInfoEditTabMobile.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.stories.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.styles.tsfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/LinkEditPage/LinkField.tsxfrontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/TextField/TextField.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTabMobile.styles.tsfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTabMobile.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/AwardEditPage/AwardSemesterCard.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/FAQSection/FAQSection.styles.tsfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/FAQSection/FAQSection.tsxfrontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/InfoSection/InfoSection.styles.ts
💤 Files with no reviewable changes (2)
- frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.stories.tsx
- frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTabMobile.styles.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/AwardEditPage/AwardSemesterCard.tsx
- frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/InfoSection/InfoSection.styles.ts
- frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/LinkEditPage/LinkField.tsx
- frontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.tsx
- frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.styles.ts
- frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/EditField/EditField.tsx
- frontend/src/pages/AdminPage/components/ClearableTextArea/ClearableTextArea.styles.ts
- frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/mobile/FAQSection/FAQSection.tsx
- frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTabMobile.tsx
- frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/mobile/TextField/TextField.tsx
| <ClearableTextArea | ||
| value={value} | ||
| onChange={onChange} | ||
| onClear={onClear} | ||
| placeholder={placeholder ?? label} | ||
| maxLength={maxLength} | ||
| size='large' | ||
| /> |
There was a problem hiding this comment.
기존 TextField는 Math.min(scrollHeight, lineHeight * 2)로 높이를 최대 2줄까지만 늘리고 나머지는 overflow: hidden으로 잘랐는데, 공통 ClearableTextArea가 쓰는 useAutoGrow는 상한 없이 scrollHeight만큼 계속 늘어나요. 동아리명/소개 필드의 세로 높이 동작이 바뀌는데 의도한 변경인지 확인해 주세요. 2줄 제한이 필요한 필드였다면 ClearableTextArea에 최대 높이(또는 줄 수) 옵션을 주는 방식을 고려해 주세요.
1 participant
#️⃣연관된 이슈
#1831
📝작업 내용
1. 아이콘 통일
field_clear_button_icon.svg제거, 모든 사용처를dark_clear_button_icon.svg로 교체ClearButtonIcon으로 통일 (기능 기준 네이밍)2.
ClearableTextArea공통 컴포넌트 생성isFocused), 클리어 버튼 표시 조건,useAutoGrow, blur 방지(e.preventDefault()) 로직을 단일 컴포넌트로 통합onClear는 Mixpanel 등 사이드이펙트 전용 optional 콜백 — 값 초기화는 내부에서onChange('')처리sizeprop으로 타이포그래피 변형 지원 (default: p6 /large: p3)3. ClubInfoMobile 텍스트 필드
TextField클리어 버튼 조건 수정: 포커스만으로 표시되던 것 → 포커스 + 내용 있을 때만 표시EditFieldisActiveprop 제거 →focus-withinCSS로 대체4. ClubIntroEditTabMobile 텍스트 필드 (4개)
InfoSection.styles.ts에focus-within추가, 각 필드에ClearableTextArea적용useAutoGrow4개,activeField상태, 개별 핸들러,TextAreaRow/ClearButton스타일 제거5. FAQSection 답변 영역
AnswerCard)에만 포커스 테두리 + 클리어 버튼 적용 (질문 입력란 제외)isAnswerActive상태 및AnswerTextarea/AnswerClearButtonstyled component 제거중점적으로 리뷰받고 싶은 부분(선택)
ClearableTextArea의onClear설계 — 값 초기화는 내부에서, 외부는 사이드이펙트만 담당하는 방식이 적절한지sizeprop 네이밍 (variant이 더 적합할 수 있음)🫡 참고사항
focus-withinCSS로 통일 (EditField,InfoSection.Card,AnswerCard)LinkField는input기반이라ClearableTextArea미적용, 동일 패턴 유지Summary by CodeRabbit