Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
텍스트 입력 필드 클리어 기능 frontend/src/components/application/questionTypes/ShortText.tsx, frontend/src/components/application/questionTypes/LongText.tsx |
ShortText와 LongText에 onClear 핸들러를 추가하여 답변을 비울 수 있도록 하고, 답변 모드에서만 클리어 버튼을 표시하도록 showClearButton 조건을 적용했습니다. |
CustomTextArea 컴포넌트 확장 frontend/src/components/common/CustomTextArea/CustomTextArea.tsx, frontend/src/components/common/CustomTextArea/CustomTextArea.styles.ts |
showClearButton, variant('outlined'|'filled'), onClear 속성을 추가하여 클리어 버튼과 스타일 변형을 지원하고, 새로운 ClearButton 스타일 컴포넌트를 추가했습니다. |
CustomTextArea 스토리 frontend/src/components/common/CustomTextArea/CustomTextArea.stories.tsx |
TextAreaRender 헬퍼 함수를 도입하여 내부 상태 관리를 중앙집중식으로 처리하고, FilledVariant와 WithClearButton 스토리를 추가했습니다. |
InputField 컴포넌트 확장 frontend/src/components/common/InputField/InputField.tsx, frontend/src/components/common/InputField/InputField.styles.ts |
variant 기반 동적 패딩 계산, 클리어 버튼 조건부 렌더링, 아이콘 변경(clearIcon), 레이아웃 조정을 통해 스타일을 개선했습니다. |
InputField 스토리 frontend/src/components/common/InputField/InputField.stories.tsx |
InputRender 래퍼 컴포넌트를 도입하여 각 스토리의 상태 관리 로직을 통합했습니다. |
ApplicationFormPage 스타일 및 레이아웃 frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts, frontend/src/pages/ApplicationFormPage/ApplicationFormPage.tsx, frontend/src/pages/ApplicationFormPage/components/QuestionContainer/QuestionContainer.tsx |
FormTitle, FormDescription, SubmitButton의 타이포그래피, 간격, 색상을 테마 기반으로 업데이트하고, Header 표시 범위를 확대하며, Footer를 추가했습니다. |
관리자 페이지 클리어 버튼 아이콘 통일 frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/components/MakeTags/MakeTags.tsx, frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/AwardEditor/AwardEditor.tsx, frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/components/FAQEditor/FAQEditor.tsx, frontend/src/pages/AdminPage/tabs/PhotoEditTab/components/ImagePreview/ImagePreview.tsx, frontend/src/pages/AdminPage/tabs/PhotoEditTab/components/ImagePreview/ImagePreview.styles.ts |
delete_button_icon.svg에서 input_clear_button_icon.svg로 아이콘을 변경하고, DeleteButton을 ClearButton으로 명칭 통일하여 UI 일관성을 개선했습니다. |
관리자 폼 필드 스타일 업데이트 frontend/src/pages/AdminPage/tabs/ApplicationEditTab/ApplicationEditTab.tsx, frontend/src/pages/AdminPage/tabs/ClubIntroEditTab/ClubIntroEditTab.tsx, frontend/src/pages/AdminPage/tabs/ClubInfoEditTab/ClubInfoEditTab.tsx |
CustomTextArea에 variant='filled'를 추가하고, 입력 필드 너비를 조정하여 관리자 페이지의 폼 스타일을 개선했습니다. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- [release] FE v1.1.19 #1106: CustomTextArea와 InputField의 클리어 버튼 속성(onClear, showClearButton)과 variant 스타일 변형이 이전 버전에서 도입된 기능과 동일한 컴포넌트를 수정합니다.
- [release] FE v1.1.12 #1025: ShortText, LongText, CustomTextArea, InputField 등 동일한 폼 입력 컴포넌트에 클리어 버튼과 variant 스타일 속성을 추가/사용합니다.
- Revert "Revert " [release] FE v1.1.19"" #1115: ApplicationFormPage에서 Header의 showOn 속성 변경으로 Header 표시 범위를 수정하는 코드 경로가 겹칩니다.
Suggested labels
🎨 Design
Suggested reviewers
- lepitaaar
- seongwon030
- oesnuj
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | PR 제목 '[refactor] 지원서 페이지 UI를 개선한다'는 변경사항의 주요 목적을 명확하게 요약하고 있으며, 지원서 페이지 UI 개선이라는 핵심 변화를 정확히 반영합니다. |
| Linked Issues check | ✅ Passed | PR은 MOA-608의 모든 주요 목표를 구현합니다: (1) 입력 필드의 border radius 통일 [InputField, CustomTextArea], (2) 텍스트 필드 클리어 버튼 추가 및 동작 보장 [ShortText, LongText, InputField, CustomTextArea], (3) 단일행/다중행 필드 디자인 통일 [variant props 도입], (4) 제출 버튼 스타일 및 반응형 개선 [ApplicationFormPage], (5) 컴포넌트 구조/네이밍 개선 [variant props, PascalCase], (6) 레이아웃 조정 [Header, Footer, 여백 수정]. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 지원서 페이지 UI 개선과 관련된 범위 내에 있습니다. MakeTags, AwardEditor, FAQEditor, ImagePreview 등의 관리자 페이지 아이콘 변경은 통일된 clear button 아이콘 사용으로 UI 일관성을 강화하는 범위 내 변경입니다. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
refactor/#1146-application-form-ui-improvement-MOA-608
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 help to get the list of available commands and usage tips.
![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
🤖 Fix all issues with AI agents
In `@frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts`:
- Around line 51-77: Rename the styled-component export submitButton to
PascalCase SubmitButton to match other components and update any imports/usages
accordingly; also fix the hover animation by either importing the pulse
keyframes from frontend/src/components/common/Button/Button.tsx and changing the
hover rule to use animation: ${pulse} 0.4s ease-in-out;, or define a local
keyframes('pulse') in ApplicationFormPage.styles.ts and reference it as
animation: ${pulse} 0.4s ease-in-out; so the animation is a keyframes reference
rather than a plain string.
🧹 Nitpick comments (7)
frontend/src/pages/ApplicationFormPage/components/QuestionContainer/QuestionContainer.tsx (1)
7-7: 에러 상태 전환 시 레이아웃 시프트 발생 가능
hasError가 토글될 때 패딩이26px 4px↔10px으로 변경되어 수직 방향으로 16px 차이가 발생합니다. 여기에ErrorText요소의 추가/제거까지 합쳐지면 컨테이너 내부 콘텐츠가 눈에 띄게 점프할 수 있습니다.에러 상태에서도 수직 패딩을 유사하게 유지하거나,
ErrorText에 대한 공간을 미리 확보해두면 UX가 더 매끄러워질 수 있습니다.frontend/src/components/common/InputField/InputField.styles.ts (1)
84-104: ClearButton 위치 지정이 Input 높이에 강하게 결합되어 있습니다.
top: 13px과bottom: 13px은 Inputheight: 45px에 맞춘 고정값입니다. Input 높이가 변경되면 버튼 위치가 깨질 수 있으므로, 기존의top: 50%; transform: translateY(-50%)방식이 더 유연합니다. 다만 현재 동작에 문제는 없으므로 참고사항입니다.frontend/src/components/common/InputField/InputField.tsx (2)
79-83: 입력값이 비어있을 때도 클리어 버튼이 표시됩니다.일반적으로 클리어 버튼은 입력값이 있을 때만 표시하는 것이 UX 관점에서 자연스럽습니다. 빈 입력 필드에 클리어 버튼이 있으면 사용자에게 혼란을 줄 수 있습니다.
💡 입력값이 있을 때만 클리어 버튼 표시
- {showClearButton && !disabled && ( + {showClearButton && !disabled && value && (
89-94:CharCount가InputWrapper밖으로 이동했지만 들여쓰기가 불일치합니다.Line 89에서
InputWrapper가 닫히고, Lines 90-94의CharCount블록이InputWrapper외부(InputContainer내부)에 위치하는데, 들여쓰기가 마치InputWrapper내부처럼 되어 있습니다.💅 들여쓰기 수정
</Styled.InputWrapper> - {showMaxChar && maxLength !== undefined && ( - <Styled.CharCount> - {value.length}/{maxLength} - </Styled.CharCount> - )} + {showMaxChar && maxLength !== undefined && ( + <Styled.CharCount> + {value.length}/{maxLength} + </Styled.CharCount> + )} {isError && helperText && (frontend/src/components/common/CustomTextArea/CustomTextArea.styles.ts (1)
48-77:ClearButton과CharCount가InputField.styles.ts와 거의 동일하게 중복되어 있습니다.Line 4의 TODO 주석에서 이미 인지하고 있는 것으로 보이지만,
ClearButton과CharCount가 두 파일에서 거의 동일한 스타일을 가지고 있습니다. 공통 스타일을 별도 파일로 추출하면 유지보수성이 향상됩니다.이 공통 컴포넌트 추출 작업을 위한 이슈를 생성해 드릴까요?
frontend/src/components/common/CustomTextArea/CustomTextArea.tsx (2)
76-80: 값이 비어있을 때도 클리어 버튼이 노출됩니다.
value가 빈 문자열일 때 클리어 버튼을 노출할 필요가 없습니다. 입력된 텍스트가 있을 때만 버튼을 표시하는 것이 일반적인 UX 패턴입니다.🛠️ 수정 제안
- {showClearButton && !disabled && ( + {showClearButton && !disabled && value && (
57-61:handleClear내!disabled체크가 중복됩니다.Line 76에서 이미
!disabled조건으로 버튼 렌더링을 제어하고 있으므로,handleClear내부의!disabled체크는 불필요합니다. 방어적 코딩 관점에서 유지해도 무방하지만, 단순화할 수 있습니다.♻️ 선택적 수정 제안
const handleClear = () => { - if (!disabled && onClear) { - onClear(); - } + onClear?.(); };
| export const submitButton = styled.button` | ||
| padding: 10px 56px; | ||
| background-color: #ff5414; | ||
| padding: 10px 40px; | ||
| background-color: ${colors.gray[900]}; | ||
| border-radius: 10px; | ||
| border: none; | ||
| color: #fff; | ||
| font-size: 1.25rem; | ||
| color: ${colors.base.white}; | ||
| font-size: 20px; | ||
| font-weight: 600; | ||
| letter-spacing: -0.4px; | ||
| transition: background-color 0.2s; | ||
| margin: 50px 0; | ||
| margin: 50px 10px; | ||
|
|
||
| &:hover { | ||
| background-color: #ffad8e; | ||
| background-color: ${colors.gray[800]}; | ||
| animation: pulse 0.4s ease-in-out; | ||
| } | ||
|
|
||
| &:active { | ||
| transform: scale(0.95); | ||
| } | ||
|
|
||
| ${media.tablet} { | ||
| width: 100%; | ||
| max-width: 100%; | ||
| height: 44px; | ||
| } | ||
| `; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for pulse animation definition
rg -n "pulse" --type=ts --type=tsx --type=css -g '!node_modules/**'Repository: Moadong/moadong
Length of output: 86
🏁 Script executed:
# Check ApplicationFormPage.tsx to see how submitButton is used
rg -A 5 -B 5 "submitButton" frontend/src/pages/ApplicationFormPage/ApplicationFormPage.tsxRepository: Moadong/moadong
Length of output: 362
🏁 Script executed:
# Check the styles file and see if there are keyframes defined
cat -n frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.tsRepository: Moadong/moadong
Length of output: 2073
🏁 Script executed:
# Check naming conventions used for styled-components in the codebase
rg "export const [A-Z]" frontend/src --type=ts --type=tsx -A 2 | head -50Repository: Moadong/moadong
Length of output: 86
🏁 Script executed:
# Search for pulse animation definition with basic rg
rg "pulse|@keyframes" --type=tsRepository: Moadong/moadong
Length of output: 537
🏁 Script executed:
# Check for keyframes in all style files
rg "keyframes" frontend/src --type=tsRepository: Moadong/moadong
Length of output: 503
🏁 Script executed:
# Look for global styles or theme file that might define animations
find frontend/src -name "*.ts" -o -name "*.js" | grep -E "(global|theme|styles|animation)" | head -20Repository: Moadong/moadong
Length of output: 1458
🏁 Script executed:
# Check other styled-components files to confirm PascalCase convention
rg "export const [A-Za-z]" frontend/src --type=ts | grep -E "styled\.(button|div|h1)" | head -30Repository: Moadong/moadong
Length of output: 3689
🏁 Script executed:
cat -n frontend/src/components/common/Button/Button.tsx | head -60Repository: Moadong/moadong
Length of output: 1762
submitButton는 PascalCase 네이밍 컨벤션을 따라야 합니다.
styled-components로 생성되는 컴포넌트는 React 컴포넌트이므로 SubmitButton으로 이름을 변경하세요. 현재 코드베이스의 모든 styled-components (Container, CoverImageWrapper, LogoWrapper, Header 등)가 PascalCase를 사용하고 있으며, submitButton만 예외입니다.
Line 65의 pulse 애니메이션이 정의되지 않았습니다.
animation: pulse 0.4s ease-in-out;은 string으로 작성되어 있어 CSS가 인식할 수 없습니다. pulse keyframes는 frontend/src/components/common/Button/Button.tsx에만 정의되어 있으므로, 다음 중 하나를 선택하세요:
- Button.tsx에서
pulsekeyframes를 import하여animation: ${pulse} 0.4s ease-in-out;로 변경 - 또는 ApplicationFormPage.styles.ts에 keyframes를 직접 정의
현재 상태에서는 호버 애니메이션이 작동하지 않습니다.
🤖 Prompt for AI Agents
In `@frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts` around
lines 51 - 77, Rename the styled-component export submitButton to PascalCase
SubmitButton to match other components and update any imports/usages
accordingly; also fix the hover animation by either importing the pulse
keyframes from frontend/src/components/common/Button/Button.tsx and changing the
hover rule to use animation: ${pulse} 0.4s ease-in-out;, or define a local
keyframes('pulse') in ApplicationFormPage.styles.ts and reference it as
animation: ${pulse} 0.4s ease-in-out; so the animation is a keyframes reference
rather than a plain string.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts`:
- Around line 61-63: The transition declaration in ApplicationFormPage.styles.ts
is broken because the semicolon ends the declaration so the second value is
treated as invalid; update the transition property to include both transitions
in one line separated by a comma (e.g., combine "background-color 0.15s ease"
and "transform 0.15s ease" into a single comma-separated value) so the transform
transition takes effect.
🧹 Nitpick comments (3)
frontend/src/pages/ApplicationFormPage/ApplicationFormPage.tsx (1)
31-39: 기존 코드의 React Hooks 규칙 위반이 있습니다 (이번 PR 변경 사항은 아님).Line 31의 조건부
return null이 Line 33 이후의useGetClubDetail,useGetApplication,useTrackPageView,useAnswers,useEffect호출보다 앞에 위치하여 Rules of Hooks를 위반합니다. 훅은 조건부 반환 전에 항상 호출되어야 합니다. 이번 PR 범위는 아니지만 추후 수정을 권장합니다.frontend/src/components/common/CustomTextArea/CustomTextArea.styles.ts (2)
30-33:variant="filled"일 때hasError상태가 시각적으로 표시되지 않습니다.
$variant === 'filled'이면border: none이 무조건 적용되어,hasError={true}여도 에러 표시(빨간 border)가 나타나지 않습니다. 현재 filled variant가 Admin 페이지에서만 사용되고 에러 상태가 전달되지 않는다면 당장 문제는 아니지만, 향후 filled variant에서도 에러를 표시해야 할 경우를 대비해 참고하시기 바랍니다.
55-75:ClearButton이InputField.styles.ts의ClearButton과 거의 동일합니다.Line 4의 TODO 주석에서 이미 인지하고 계신 부분입니다. 추후 리팩토링 시 공통 컴포넌트로 추출하면 중복을 줄일 수 있습니다.
| transition: | ||
| background-color 0.15s ease; | ||
| transform 0.15s ease; |
There was a problem hiding this comment.
CSS transition 속성에 구문 오류가 있습니다 — transform 전환이 동작하지 않습니다.
Line 62의 세미콜론이 transition 선언을 종료시키므로, Line 63의 transform 0.15s ease;는 별도의 (무효한) CSS 선언으로 처리됩니다. 여러 전환을 지정하려면 쉼표로 구분해야 합니다.
🐛 수정 제안
transition:
- background-color 0.15s ease;
- transform 0.15s ease;
+ background-color 0.15s ease,
+ transform 0.15s ease;📝 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.
| transition: | |
| background-color 0.15s ease; | |
| transform 0.15s ease; | |
| transition: | |
| background-color 0.15s ease, | |
| transform 0.15s ease; |
🤖 Prompt for AI Agents
In `@frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts` around
lines 61 - 63, The transition declaration in ApplicationFormPage.styles.ts is
broken because the semicolon ends the declaration so the second value is treated
as invalid; update the transition property to include both transitions in one
line separated by a comma (e.g., combine "background-color 0.15s ease" and
"transform 0.15s ease" into a single comma-separated value) so the transform
transition takes effect.
There was a problem hiding this comment.
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)
frontend/src/components/common/CustomTextArea/CustomTextArea.stories.tsx (1)
12-62:⚠️ Potential issue | 🟡 Minor
onClearargType이 누락되었습니다.
InputField.stories.tsx에는onClearargType이 정의되어 있지만(action: 'cleared'), 이 파일에는 빠져 있습니다.TextAreaRender에서args.onClear?.()를 호출하고 있으므로, Storybook Actions 패널에서 로깅되려면 argType 정의가 필요합니다.누락된 argType 추가 제안
showClearButton: { control: 'boolean', description: '삭제 버튼 표시 여부입니다.', }, + onClear: { + action: 'cleared', + description: '삭제 버튼 클릭 시 호출되는 함수입니다.', + }, showMaxChar: {
🤖 Fix all issues with AI agents
In `@frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts`:
- Around line 51-71: SubmitButton's custom &:hover / &:active rules conflict
with the Button component's animated behavior (pulse animation and scale(0.95))
when ApplicationFormPage passes animated={true}; either remove the animated prop
in ApplicationFormPage or update SubmitButton (the styled Button) to
conditionally apply its hover/active rules based on the animated prop (or
harmonize the transform values) so the StyledButton's pulse/scale(0.95)
animation and SubmitButton's translateY(-0.5px)/scale(0.98) do not apply
simultaneously — locate SubmitButton in ApplicationFormPage.styles.ts, the
Button component that defines the pulse/scale behavior, and the animated={true}
usage in ApplicationFormPage.tsx and make the change accordingly.
🧹 Nitpick comments (8)
frontend/src/components/common/InputField/InputField.tsx (3)
81-85: 빈 입력 필드에서도 클리어 버튼이 노출됩니다.현재
value가 빈 문자열일 때도 클리어 버튼이 렌더링됩니다. PR 목적("텍스트 입력 시 클리어 버튼 미노출" 개선)에 따르면, 텍스트가 입력된 경우에만 클리어 버튼을 보여주는 것이 일반적인 UX 패턴입니다.제안: value가 있을 때만 클리어 버튼 노출
- {showClearButton && type !== 'password' && !disabled && ( + {showClearButton && type !== 'password' && !disabled && value && (
91-96:CharCount가InputWrapper밖에 위치하지만 들여쓰기가 내부처럼 되어 있습니다.Line 91에서
InputWrapper가 닫힌 후, Lines 92-96의CharCount블록이 마치InputWrapper내부인 것처럼 들여쓰기되어 있어 가독성이 떨어집니다.들여쓰기 수정 제안
</Styled.InputWrapper> - {showMaxChar && maxLength !== undefined && ( - <Styled.CharCount> - {value.length}/{maxLength} - </Styled.CharCount> - )} + {showMaxChar && maxLength !== undefined && ( + <Styled.CharCount> + {value.length}/{maxLength} + </Styled.CharCount> + )}
41-41:activeButton삼항 연산자 중첩으로 가독성이 낮습니다.한 줄에 삼항 연산이 3단계로 중첩되어 있어 의도 파악이 어렵습니다. 간단한 함수나 분기문으로 분리하면 더 명확합니다.
frontend/src/components/common/InputField/InputField.stories.tsx (2)
80-96:InputRender의args파라미터에any타입 대신 구체적인 타입을 사용하세요.
any대신ComponentProps<typeof InputField>를 사용하면 타입 안전성이 향상됩니다.CustomTextArea.stories.tsx의TextAreaRender도 동일합니다.타입 개선 제안
-const InputRender = (args: any) => { +const InputRender = (args: React.ComponentProps<typeof InputField>) => {
160-169:Disabled스토리에만render: InputRender가 누락되어 있습니다.다른 모든 스토리는
render: InputRender를 사용하고 있으나,Disabled만 빠져있어 일관성이 떨어집니다. 비활성화 상태라 기능상 문제는 없지만 통일하는 것이 좋습니다.frontend/src/components/common/CustomTextArea/CustomTextArea.stories.tsx (2)
136-145:Disabled스토리에render: TextAreaRender가 누락되었습니다.
InputField.stories.tsx의Disabled스토리와 동일한 누락입니다. 일관성을 위해 추가하는 것을 권장합니다.
68-84:TextAreaRender도any타입 대신 구체적인 타입 사용을 권장합니다.
InputRender와 동일한 개선 사항이 적용됩니다.타입 개선 제안
-const TextAreaRender = (args: any) => { +const TextAreaRender = (args: React.ComponentProps<typeof CustomTextArea>) => {frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts (1)
73-79: 태블릿 반응형 블록 내 불필요한 빈 줄이 있습니다.Line 77 뒤의 빈 줄은 의도하지 않은 것으로 보입니다.
🧹 정리 제안
${media.tablet} { && { width: 100%; max-width: 100%; - } }
frontend/src/pages/ApplicationFormPage/ApplicationFormPage.styles.ts
Outdated
Show resolved
Hide resolved
seongwon030
left a comment
seongwon030
left a comment
There was a problem hiding this comment.
개선된 디자인이 훨씬 낫네요 수고하셨습니다 ~
oesnuj
left a comment
oesnuj
left a comment
There was a problem hiding this comment.
장문 모드에서도 초기화 버튼이 생겼네요.
레이아웃도 깔끔해진 것 같습니다
수고하셨습니다!
| className, | ||
| }: ButtonProps) => ( | ||
| <StyledButton | ||
| width={width} | ||
| onClick={onClick} | ||
| animated={animated} | ||
| type={type} | ||
| disabled={disabled} | ||
| className={className} |
There was a problem hiding this comment.
Button 컴포넌트에 className props가 추가된 이유가 궁금합니다
There was a problem hiding this comment.
기존에는 지원서 페이지에 전용 SubmitButton이 따로 있었는데,
UI 개선 과정에서 공통 Button 컴포넌트를 재사용하는 방향으로 정리하고자 했습니다.
다만, 지원서 페이지의 제출하기 버튼의 경우, 버튼 높이가 다르고, 모바일/태블릿 환경에서 전체 폭을 차지하는 등 공통 Button과 스타일 요구사항이 일부 달랐습니다.
공통 Button의 기본 디자인과 클릭 이벤트 로직은 유지하면서, 페이지 단위에서 필요한 스타일만 오버라이드할 수 있게 ClassName props를 추가했습니다.
| }: CustomInputProps) => { | ||
| const [isPasswordVisible, setIsPasswordVisible] = useState(false); | ||
| const activeButton = disabled ? 'none' : type === 'password' ? 'password' : showClearButton ? 'text' : 'none'; | ||
|
|
There was a problem hiding this comment.
삼항 연산자가 3중으로 중첩되어 있어서 읽기가 조금 어려운 것 같아요!
조건을 변수나 함수로 분리하면 나중에 코드를 다시 볼 때 이해하기 더 쉬울 것 같습니다.
There was a problem hiding this comment.
수정하면서 다른 삼항연산자 사용된 부분도 상수 분리로 수정했습니당
87414f6
| @@ -1,5 +1,5 @@ | |||
| import { useEffect, useRef, useState } from 'react'; | |||
| import deleteButton from '@/assets/images/icons/delete_button_icon.svg'; | |||
| import deleteButton from '@/assets/images/icons/input_clear_button_icon.svg'; | |||
There was a problem hiding this comment.
input_clear_button_icon으로 바꾼 거 좋습니다!
import 구문뿐만 아니라 deleteButton도 ClearButton으로 바꾸면 더 좋겠네요~
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/common/CustomTextArea/CustomTextArea.tsx (1)
40-49:⚠️ Potential issue | 🟡 Minor
useEffect의존성 배열에disabled가 누락되어 있습니다.
disabled가true→false로 변경될 때,value가 동일하면 effect가 재실행되지 않아 textarea 높이가auto로 재조정되지 않습니다.🔧 수정 제안
- }, [value]); + }, [value, disabled]);frontend/src/components/common/InputField/InputField.styles.ts (1)
1-4:⚠️ Potential issue | 🟡 MinorPrettier 포맷팅 실패
CI 파이프라인에서 Prettier 포맷팅 이슈가 감지되었습니다.
npx prettier --write를 실행하여 수정해 주세요.
🤖 Fix all issues with AI agents
In `@frontend/src/components/common/InputField/InputField.tsx`:
- Around line 87-91: The clear button can render but do nothing because
showClearButton defaults true while onClear is optional; update the render
condition to only show the button when an onClear handler exists and the input
has value (e.g., add a check for onClear and value alongside the existing
checks) or alternatively hide it when value is empty; change the condition
around Styled.ClearButton (where clearInput is used) to include onClear !==
undefined (and/or Boolean(value)) so clicking calls a provided handler and the
button is not visible when it would be inert.
- Around line 98-102: The container for the input needs positioning so
absolutely-positioned helper text is anchored correctly: add position: relative
to the styled component/Input wrapper named InputContainer so that HelperText
(which uses top: 100%) is positioned relative to InputContainer and no longer
overlaps CharCount when showMaxChar and isError are both true; locate the
InputContainer styled definition (used alongside CharCount and HelperText in
InputField.tsx) and add position: relative to its style rules.
🧹 Nitpick comments (3)
frontend/src/components/common/InputField/InputField.styles.ts (2)
45-47: 하드코딩된 색상값들이 테마 색상과 혼용되고 있습니다
colors.gray[500]은 테마에서 직접 import하여 사용하고 있지만,'red','#28a745','#007bff'등은 하드코딩되어 있습니다. 테마 컬러 시스템을 일관되게 활용하면 유지보수성이 향상됩니다. 급하지 않으면 추후 개선 사항으로 고려해 주세요.Also applies to: 64-71
94-114:ClearButton의 수직 위치가 고정값으로 설정되어 있습니다
top: 13px,bottom: 13px으로 고정되어 Input의height: 45px에 맞춰져 있습니다. Input 높이가 변경되면 깨질 수 있으므로,top: 50%; transform: translateY(-50%)를 사용하면ToggleButton과도 일관되고 더 유연해집니다.제안
export const ClearButton = styled.button` position: absolute; right: 18px; - top: 13px; - bottom: 13px; + top: 50%; + transform: translateY(-50%); background: none;frontend/src/pages/ApplicationFormPage/ApplicationFormPage.tsx (1)
31-58: 기존 코드: Hooks 호출 전 조건부 early return은 React Rules of Hooks 위반입니다.이 PR의 변경 사항은 아니지만, Line 31의
if (!clubId || !applicationFormId) return null;이 Line 33 이후의useGetClubDetail,useGetApplication,useTrackPageView,useAnswers,useEffect등 모든 hooks보다 먼저 실행됩니다. React는 매 렌더링마다 동일한 순서와 횟수로 hooks가 호출될 것을 요구하므로, 이 패턴은 런타임 오류를 유발할 수 있습니다.향후 리팩토링 시 early return을 hooks 호출 이후로 이동하거나, 파라미터 유효성 검증을 별도 컴포넌트로 분리하는 것을 권장합니다.
♻️ 수정 제안 (예시)
const ApplicationFormPage = () => { const { clubId, applicationFormId } = useParams<{ clubId: string; applicationFormId: string; }>(); const navigate = useNavigate(); const questionRefs = useRef<Array<HTMLDivElement | null>>([]); const [invalidQuestionIds, setInvalidQuestionIds] = useState<number[]>([]); const trackEvent = useMixpanelTrack(); - if (!clubId || !applicationFormId) return null; - const { data: clubDetail, error: clubError } = useGetClubDetail(clubId); // ... other hooks ... + if (!clubId || !applicationFormId) return null; + if (isLoading) return <Spinner />;참고:
useGetClubDetail등의 hooks가undefined파라미터를 안전하게 처리하는지 확인이 필요합니다.
| {showClearButton && type !== 'password' && !disabled && ( | ||
| <Styled.ClearButton type='button' onClick={clearInput}> | ||
| <img src={clearIcon} alt='삭제' /> | ||
| <img src={clearButton} alt='삭제' /> | ||
| </Styled.ClearButton> | ||
| )} |
There was a problem hiding this comment.
onClear가 없을 때 클리어 버튼이 렌더링되지만 동작하지 않는 문제
showClearButton의 기본값이 true이고 onClear는 optional이므로, onClear를 전달하지 않은 곳에서 클리어 버튼이 보이지만 클릭해도 아무 동작을 하지 않습니다. 렌더 조건에 onClear 존재 여부도 포함하거나, value가 비어있을 때 숨기는 것을 고려해 주세요.
제안
- {showClearButton && type !== 'password' && !disabled && (
+ {showClearButton && type !== 'password' && !disabled && value && onClear && (📝 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.
| {showClearButton && type !== 'password' && !disabled && ( | |
| <Styled.ClearButton type='button' onClick={clearInput}> | |
| <img src={clearIcon} alt='삭제' /> | |
| <img src={clearButton} alt='삭제' /> | |
| </Styled.ClearButton> | |
| )} | |
| {showClearButton && type !== 'password' && !disabled && value && onClear && ( | |
| <Styled.ClearButton type='button' onClick={clearInput}> | |
| <img src={clearButton} alt='삭제' /> | |
| </Styled.ClearButton> | |
| )} |
🤖 Prompt for AI Agents
In `@frontend/src/components/common/InputField/InputField.tsx` around lines 87 -
91, The clear button can render but do nothing because showClearButton defaults
true while onClear is optional; update the render condition to only show the
button when an onClear handler exists and the input has value (e.g., add a check
for onClear and value alongside the existing checks) or alternatively hide it
when value is empty; change the condition around Styled.ClearButton (where
clearInput is used) to include onClear !== undefined (and/or Boolean(value)) so
clicking calls a provided handler and the button is not visible when it would be
inert.
| {showMaxChar && maxLength !== undefined && ( | ||
| <Styled.CharCount> | ||
| {value.length}/{maxLength} | ||
| </Styled.CharCount> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how InputField component is used - look for instances with both showMaxChar and isError
rg -n 'showMaxChar' --type=ts --type=tsx -A 2 -B 2 -g '!node_modules' | head -100Repository: Moadong/moadong
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Check InputField usage patterns
rg -n '<InputField' -A 15 --type=ts --type=tsx -g '!node_modules' | head -150Repository: Moadong/moadong
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Check the InputField component itself for HelperText positioning
cat -n frontend/src/components/common/InputField/InputField.tsx | head -150Repository: Moadong/moadong
Length of output: 3745
🏁 Script executed:
#!/bin/bash
# Look for styled component definitions for CharCount and HelperText
rg -n 'CharCount|HelperText' --type=ts --type=tsx frontend/src/components/common/InputField/ -A 2Repository: Moadong/moadong
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Find and read the InputField styles file
find frontend/src/components/common/InputField -name "*.styles.*" -o -name "*.css" -o -name "*.scss"Repository: Moadong/moadong
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Search for InputField styles without type specification
rg -n 'CharCount|HelperText' frontend/src/components/common/InputField/ -A 5Repository: Moadong/moadong
Length of output: 2045
🏁 Script executed:
#!/bin/bash
# Check for InputField usage in the codebase to see if both props are used together
rg 'showMaxChar.*isError|isError.*showMaxChar' frontend/src/ -A 2 -B 2Repository: Moadong/moadong
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Read the full styles file to understand InputContainer and overall layout
cat -n frontend/src/components/common/InputField/InputField.styles.tsRepository: Moadong/moadong
Length of output: 3932
InputContainer에 position: relative 추가 필요
CharCount가 블록 레이아웃으로 정상 렌더링되는 것은 확인되었으나, HelperText의 절대 위치 지정이 문제입니다. InputContainer에 position: relative가 없어서 HelperText의 top: 100%가 InputContainer를 기준으로 배치되지 않습니다. 결과적으로 showMaxChar과 isError가 동시에 true일 때 CharCount와 HelperText가 겹칠 수 있습니다. InputContainer에 position: relative를 추가하여 HelperText가 입력 필드 아래에 올바르게 배치되도록 수정해야 합니다.
🤖 Prompt for AI Agents
In `@frontend/src/components/common/InputField/InputField.tsx` around lines 98 -
102, The container for the input needs positioning so absolutely-positioned
helper text is anchored correctly: add position: relative to the styled
component/Input wrapper named InputContainer so that HelperText (which uses top:
100%) is positioned relative to InputContainer and no longer overlaps CharCount
when showMaxChar and isError are both true; locate the InputContainer styled
definition (used alongside CharCount and HelperText in InputField.tsx) and add
position: relative to its style rules.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/common/InputField/InputField.styles.ts`:
- Around line 125-132: CharCount currently uses colors.gray[500], which fails
WCAG AA contrast against a white background; update the styled component
CharCount to use a darker token (e.g., colors.gray[600] or another color token
that meets a 4.5:1 contrast ratio) so the text meets accessibility standards,
and verify the new token in the design tokens/colors module to ensure it
provides sufficient contrast.
🧹 Nitpick comments (3)
frontend/src/components/common/InputField/InputField.styles.ts (3)
95-96:ClearButton의top: 13px; bottom: 13px이Input의height: 45px에 강결합되어 있습니다.Input 높이가 45px일 때
45 - 13*2 = 19px의 버튼 영역이 남습니다. 향후 Input 높이가 변경되면 ClearButton 위치가 깨질 수 있습니다.top: 50%; transform: translateY(-50%)(ToggleButton과 동일한 방식)을 사용하면 높이 변경에 더 유연합니다.♻️ 높이 독립적인 수직 중앙 정렬 제안
export const ClearButton = styled.button` position: absolute; right: 18px; - top: 13px; - bottom: 13px; + top: 50%; + transform: translateY(-50%); background: none;
41-42: padding 선언의 멀티라인 포맷이 다소 읽기 어렵습니다.현재
padding: 12px ${...} 12px 18px가 두 줄에 걸쳐 있어 shorthand 순서(top right bottom left)를 파악하기 어렵습니다.♻️ 가독성 개선 제안
- padding: 12px ${({ $variant }) => INPUT_RIGHT_PADDING[$variant ?? 'none']} - 12px 18px; + padding: 12px ${({ $variant }) => INPUT_RIGHT_PADDING[$variant ?? 'none']} 12px 18px;
43-69: 하드코딩된 색상 값들(red,#28a745,#007bff)이colors테마 상수와 혼용되고 있습니다.border 기본 색상은
colors.gray[500]으로 개선되었지만, error(red), success(#28a745), focus(#007bff) 색상은 여전히 하드코딩되어 있습니다. 테마 일관성을 위해 이들도colors상수로 통일하면 유지보수에 유리합니다.
| export const CharCount = styled.span` | ||
| position: absolute; | ||
| color: #c5c5c5; | ||
| top: 110%; | ||
| right: 0; | ||
| display: block; | ||
| margin-top: 4px; | ||
| text-align: right; | ||
| color: ${colors.gray[500]}; | ||
| font-size: 14px; | ||
| letter-spacing: -0.96px; | ||
| `; |
There was a problem hiding this comment.
CharCount 색상 colors.gray[500](#C5C5C5)의 접근성 대비를 확인해주세요.
흰색 배경 기준 #C5C5C5는 WCAG AA 기준(4.5:1) 대비율을 충족하지 못할 수 있습니다. 기존 colors.gray[600](#989898) 이상을 사용하면 가독성이 향상됩니다.
#!/bin/bash
# gray[500] (`#C5C5C5`) vs white (`#FFFFFF`) 대비율 계산
python3 -c "
def relative_luminance(hex_color):
r, g, b = [int(hex_color[i:i+2], 16) / 255.0 for i in (1, 3, 5)]
def linearize(c):
return c / 12.92 if c <= 0.04045 else ((c + 0.055) / 1.055) ** 2.4
return 0.2126 * linearize(r) + 0.7152 * linearize(g) + 0.0722 * linearize(b)
l1 = relative_luminance('#FFFFFF')
l2 = relative_luminance('#C5C5C5')
ratio = (l1 + 0.05) / (l2 + 0.05)
print(f'gray[500] (`#C5C5C5`) vs white: {ratio:.2f}:1 (WCAG AA requires 4.5:1 for normal text)')
l3 = relative_luminance('#989898')
ratio2 = (l1 + 0.05) / (l3 + 0.05)
print(f'gray[600] (`#989898`) vs white: {ratio2:.2f}:1')
"🤖 Prompt for AI Agents
In `@frontend/src/components/common/InputField/InputField.styles.ts` around lines
125 - 132, CharCount currently uses colors.gray[500], which fails WCAG AA
contrast against a white background; update the styled component CharCount to
use a darker token (e.g., colors.gray[600] or another color token that meets a
4.5:1 contrast ratio) so the text meets accessibility standards, and verify the
new token in the design tokens/colors module to ensure it provides sufficient
contrast.
suhyun113
force-pushed
the
refactor/#1146-application-form-ui-improvement-MOA-608
branch
from
6637df7 to
2633067
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/common/CustomTextArea/CustomTextArea.styles.ts`:
- Around line 24-27: The styled TextArea component declares a hasError prop that
is being passed to the DOM; rename the prop to use the styled-components
transient prefix (change hasError to $hasError) in the TextArea generic and
throughout its template where styling/ border logic references hasError, and
update any conditional border/color logic to use $hasError; also apply the same
change to the InputField.styles.ts props (rename hasError and isSuccess to
$hasError and $isSuccess and update their usages) so non-standard props are not
forwarded to DOM elements.
🧹 Nitpick comments (5)
frontend/src/pages/ApplicationFormPage/ApplicationFormPage.tsx (1)
151-153:animated={true}는animated로 간소화할 수 있습니다.JSX에서 boolean prop은 값 없이 전달하면
true로 처리됩니다.♻️ 제안
- <Styled.SubmitButton onClick={handleSubmit} animated={true}> + <Styled.SubmitButton onClick={handleSubmit} animated>frontend/src/components/common/InputField/InputField.styles.ts (1)
43-45: 하드코딩된 색상값'red','#28a745','#007bff'를 테마 색상 토큰으로 통일하는 것을 고려해보세요.
colors.gray[500]은 테마 토큰을 사용하는 반면,'red','#28a745','#007bff'는 하드코딩되어 있습니다. 디자인 시스템의 일관성을 위해colors.accent등 기존 테마 토큰으로 교체하면 향후 테마 변경 시 일괄 적용이 가능합니다.frontend/src/components/common/InputField/InputField.stories.tsx (1)
160-169:Disabled스토리에만render: InputRender가 누락되어 있습니다.다른 모든 스토리는
InputRender를 사용하지만,Disabled만 빠져 있습니다. 비활성 상태에서도 Storybook 컨트롤 패널을 통한 일관된 동작을 위해render: InputRender를 추가하는 것이 좋습니다.♻️ 수정 제안
export const Disabled: Story = { args: { label: '아이디', value: 'disabled_user', disabled: true, width: '300px', onChange: () => {}, onClear: () => {}, }, + render: InputRender, };frontend/src/components/common/CustomTextArea/CustomTextArea.stories.tsx (2)
136-145:Disabled스토리에render: TextAreaRender가 누락되어 있습니다.
InputField.stories.tsx의Disabled스토리와 동일한 누락입니다. 일관성을 위해 추가를 권장합니다.♻️ 수정 제안
export const Disabled: Story = { args: { label: '피드백', value: '이미 제출된 피드백입니다.', disabled: true, width: '300px', variant: 'outlined', onChange: () => {}, }, + render: TextAreaRender, };
68-84:TextAreaRender헬퍼가InputField.stories.tsx의InputRender와 거의 동일합니다.두 헬퍼 모두
useState로 로컬 상태를 관리하고onChange/onClear를 전달하는 동일 패턴입니다. 현재는 각 스토리 파일에 포함되어 있어 괜찮지만, 반복된다면 공유 유틸리티로 추출을 고려할 수 있습니다.
| export const TextArea = styled.textarea<{ | ||
| hasError?: boolean; | ||
| $variant?: 'outlined' | 'filled'; | ||
| }>` |
There was a problem hiding this comment.
hasError prop에 styled-components 트랜지언트 접두사 $가 누락되어 DOM에 전달됩니다.
$variant는 올바르게 $ 접두사를 사용하고 있지만, hasError는 유효한 HTML 속성이 아니므로 DOM으로 전달되어 React 콘솔 경고가 발생합니다. $hasError로 변경해야 합니다.
이 문제는 InputField.styles.ts의 hasError, isSuccess prop에도 동일하게 적용됩니다.
🔧 수정 제안 (CustomTextArea.styles.ts)
-export const TextArea = styled.textarea<{
- hasError?: boolean;
- $variant?: 'outlined' | 'filled';
-}>`
+export const TextArea = styled.textarea<{
+ $hasError?: boolean;
+ $variant?: 'outlined' | 'filled';
+}>`border 로직도 $hasError로 업데이트 필요:
- border: ${({ hasError, $variant }) =>
+ border: ${({ $hasError, $variant }) =>
$variant === 'filled'
? 'none'
- : `1px solid ${hasError ? 'red' : colors.gray[500]}`};
+ : `1px solid ${$hasError ? 'red' : colors.gray[500]}`};🤖 Prompt for AI Agents
In `@frontend/src/components/common/CustomTextArea/CustomTextArea.styles.ts`
around lines 24 - 27, The styled TextArea component declares a hasError prop
that is being passed to the DOM; rename the prop to use the styled-components
transient prefix (change hasError to $hasError) in the TextArea generic and
throughout its template where styling/ border logic references hasError, and
update any conditional border/color logic to use $hasError; also apply the same
change to the InputField.styles.ts props (rename hasError and isSuccess to
$hasError and $isSuccess and update their usages) so non-standard props are not
forwarded to DOM elements.
lepitaaar
deleted the
refactor/#1146-application-form-ui-improvement-MOA-608
branch
3 participants
#️⃣연관된 이슈
📝작업 내용
작업 배경
등이 기능적으로 오류는 아니었지만,
지원서 페이지는 입력 중심의 페이지로, 사용성에 크게 영향을 주는 부분이라고 판단하여 개선된 UI를 반영하고자 했습니다.
주요 변경 사항
InputField,CustomTextArea에 공통적으로 클리어 버튼 동작 추가input_clear_button_icon.svg로 통일하고 기존 아이콘 제거InputField와CustomTextArea를 동일한 기준으로 맞춤InputField및CustomTextArea에variantProps를 도입하여, 복잡한 조건부 렌더링 대신 명확한 상태 기반 스타일링 적용submitButton등 소문자로 시작하던 컴포넌트 이름을 SubmitButton과 같이 PascalCase로 수정Footer추가중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
스타일