-
Notifications
You must be signed in to change notification settings - Fork 1
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: 톡픽 상세 조회 모바일 컴포넌트 구현 #307
base: dev
Are you sure you want to change the base?
Conversation
Walkthrough이번 PR은 다양한 컴포넌트와 스타일, Storybook 구성을 업데이트합니다. 새로운 SVG 아이콘, ProfileIcon의 리팩토링, Button의 새로운 variant, ReportModal, SummaryBox, VoteToggle, TalkPickSection 등 여러 컴포넌트가 추가되거나 수정되었습니다. 또한, 색상 및 타이포그래피 스타일 변경, TalkPickDetail 타입 확장이 포함되어 있으며, Storybook 문서도 함께 보강되었습니다. Changes
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 11
🔭 Outside diff range comments (3)
src/components/organisms/TalkPickSection/TalkPickSection.tsx (3)
58-63
: 🛠️ Refactor suggestionconsole.log 대신 적절한 에러 처리 필요
개발용 console.log 문을 제거하고 적절한 에러 처리를 추가하는 것이 좋겠습니다.
- .then(() => { - console.log('톡픽 링크 복사 완료!'); - }) - .catch((err) => { - console.log(err); - }); + .then(() => { + showToastModal(SUCCESS.COPY.LINK); + }) + .catch(() => { + showToastModal(ERROR.COPY.LINK); + });
159-163
:⚠️ Potential issueReportModal의 onConfirm 핸들러 구현 필요
ReportModal의 onConfirm 핸들러가 비어있습니다. 신고 기능 구현이 필요합니다.
<ReportModal isOpen={activeModal === 'reportTalkPick'} - onConfirm={() => {}} + onConfirm={() => { + // TODO: Implement report functionality + handleReport(talkPick.id); + onCloseModal(); + showToastModal(SUCCESS.REPORT); + }} onClose={onCloseModal} />
203-207
: 🛠️ Refactor suggestion이미지 최적화 필요
이미지 요소에 width, height, loading 속성을 추가하여 성능과 사용자 경험을 개선하는 것이 좋겠습니다.
<div css={S.talkPickImageWrapper}> {talkPick?.imgUrls.map((url, idx) => ( - <img src={url} alt={`image ${idx + 1}`} /> + <img + src={url} + alt={`image ${idx + 1}`} + width="100%" + height="auto" + loading="lazy" + /> ))} </div>
🧹 Nitpick comments (18)
src/stories/organisms/TalkPickSection.stories.tsx (1)
26-26
: 다양한 상태에 대한 스토리 추가 필요
summaryStatus
가 'SUCCESS'로 하드코딩되어 있습니다. 'PENDING', 'FAIL', 'NOT_REQUIRED' 상태에 대한 스토리도 추가하여 모든 상태를 테스트할 수 있도록 하는 것이 좋겠습니다.export const Default: Story = { play: () => { store.dispatch(setToken('accessToken')); }, }; +export const PendingSummary: Story = { + args: { + talkPick: { + ...defaultTodayTalkPick, + summaryStatus: 'PENDING', + }, + }, +}; + +export const FailedSummary: Story = { + args: { + talkPick: { + ...defaultTodayTalkPick, + summaryStatus: 'FAIL', + }, + }, +}; + +export const NotRequiredSummary: Story = { + args: { + talkPick: { + ...defaultTodayTalkPick, + summaryStatus: 'NOT_REQUIRED', + }, + }, +};src/components/mobile/atoms/SummaryItem/SummaryItem.tsx (1)
9-14
: 컴포넌트 구현이 간단명료합니다만, 접근성 개선이 필요합니다.시맨틱한 HTML 요소 사용과 ARIA 속성 추가를 고려해보세요.
- <div css={summaryItemStyling}> - <div css={numberItemStyling}>{itemNumber}</div> + <article css={summaryItemStyling} role="listitem"> + <span css={numberItemStyling} aria-label={`항목 ${itemNumber}`}>{itemNumber}</span> {children} - </div> + </article>src/stories/mobile/molecules/ReportModal.stories.tsx (1)
4-17
: Storybook 설정이 기본적으로 잘 되어있습니다.하지만 다음과 같은 추가 스토리들을 고려해보세요:
- 닫힌 상태 (isOpen: false)
- 다양한 신고 사유가 선택된 상태
- 에러 상태
src/components/mobile/atoms/Button/Button.tsx (1)
7-12
: 버튼 variant 이름이 좀 더 의미있게 개선되면 좋겠습니다.'outlineHighlightR'와 'outlineHighlightB'보다는 의미를 더 잘 전달하는 이름을 제안드립니다:
- 'outlineHighlightRed' 또는 'outlineError'
- 'outlineHighlightBlue' 또는 'outlineInfo'
또한 각 variant의 용도에 대한 JSDoc 문서화를 추가하면 좋겠습니다:
export interface ButtonProps extends ComponentPropsWithRef<'button'> { size?: 'large' | 'medium'; + /** 버튼의 시각적 스타일을 정의합니다. + * @property primary - 기본 스타일 + * @property roundPrimary - 둥근 모서리의 기본 스타일 + * @property outlineShadow - 그림자가 있는 외곽선 스타일 + * @property outlineHighlightR - 빨간색 강조 외곽선 스타일 (경고/에러용) + * @property outlineHighlightB - 파란색 강조 외곽선 스타일 (정보용) + */ variant?: | 'primary' | 'roundPrimary' | 'outlineShadow' | 'outlineHighlightR' | 'outlineHighlightB';src/stories/mobile/atoms/SummaryItem.stories.tsx (1)
31-48
: 스토리 예시에 접근성 개선이 필요합니다.리스트 아이템에 의미있는 텍스트 콘텐츠를 사용하고 있지만, 스크린 리더 사용자를 위한 추가적인 접근성 개선이 필요합니다.
다음과 같이 aria-label을 추가하는 것을 제안합니다:
- <ul css={storyContainer}> + <ul css={storyContainer} aria-label="요약 아이템 예시 목록">src/components/mobile/molecules/SummaryBox/SummaryBox.tsx (1)
17-60
: contentMap 구현에 대한 개선 제안현재 구현은 잘 작동하지만, 다음과 같은 개선사항을 고려해보세요:
- 각 상태별 컴포넌트를 별도 함수로 분리하여 가독성 향상
- summary prop이 undefined일 때의 처리 추가
다음과 같은 리팩토링을 제안합니다:
+ const renderPendingContent = () => ( + <div css={S.summarySpinnerWrapper}> + <div css={S.summaryTextStyling}>{SUMMARY.TITLE}</div> + <div css={S.summarySpinnerStyling}> + <Spinner css={S.spinnerStyling} /> + </div> + <p css={S.summarySpinnerText}>{SUMMARY.PENDING}</p> + </div> + ); + const renderSuccessContent = (summary?: TalkPickSummary) => ( + <div css={S.summaryWrapper}> + <SummaryItem itemNumber="1">{summary?.summaryFirstLine || ''}</SummaryItem> + <SummaryItem itemNumber="2">{summary?.summarySecondLine || ''}</SummaryItem> + <SummaryItem itemNumber="3">{summary?.summaryThirdLine || ''}</SummaryItem> + </div> + ); const contentMap = { - PENDING: ( - <div css={S.summarySpinnerWrapper}> - ... - </div> - ), + PENDING: renderPendingContent, - SUCCESS: ( - <div css={S.summaryWrapper}> - ... - </div> - ), + SUCCESS: () => renderSuccessContent(summary), ... };src/stories/molecules/SummaryBox.stories.tsx (1)
37-58
: 스토리 구성에 대한 제안각 상태별 스토리가 잘 구성되어 있지만, 문서화 측면에서 개선이 필요합니다.
각 상태별로 parameters에 docs 설명을 추가하는 것을 제안합니다:
export const All: Story = { + parameters: { + docs: { + description: { + story: '`SummaryBox` 컴포넌트의 모든 상태를 보여주는 예시입니다.' + } + } + }, render: () => ( ... ), };src/stories/mobile/molecules/SummaryBox.stories.tsx (1)
19-30
: 모바일 특화 문서화 필요모바일 컴포넌트임을 명확히 하는 문서화가 필요합니다.
다음과 같이 모바일 관련 문서를 추가하는 것을 제안합니다:
const meta = { title: 'mobile/molecules/SummaryBox', component: SummaryBox, parameters: { layout: 'centered', + docs: { + description: { + component: '모바일 환경에 최적화된 SummaryBox 컴포넌트입니다. 모바일에서의 터치 영역과 가독성을 고려하여 설계되었습니다.' + } + } }, ... };src/components/mobile/atoms/Button/Button.style.ts (1)
73-88
: 코드 중복을 제거하여 유지보수성을 향상시키세요.outlineHighlightR과 outlineHighlightB 변형에 대한 사이즈 스타일링이 동일합니다. 중복을 제거하기 위해 공통 스타일을 추출하는 것이 좋습니다.
다음과 같이 리팩토링하는 것을 제안합니다:
+const highlightSizeStyling = { + large: css({}), + medium: css(typo.Comment.SemiBold, { + width: '134px', + height: '72px', + padding: '0 21px', + }), +}; const style = { // ... other variants outlineHighlightR: { - large: css({}), - medium: css(typo.Comment.SemiBold, { - width: '134px', - height: '72px', - padding: '0 21px', - }), + ...highlightSizeStyling, }, outlineHighlightB: { - large: css({}), - medium: css(typo.Comment.SemiBold, { - width: '134px', - height: '72px', - padding: '0 21px', - }), + ...highlightSizeStyling, }, };src/stories/mobile/molecules/VoteToggle.stories.tsx (1)
58-58
: 오타를 수정하세요."Defult"를 "Default"로 수정해야 합니다.
- <h3>Defult</h3> + <h3>Default</h3>src/components/atoms/ProfileIcon/ProfileIcon.style.ts (1)
16-33
: 크기 상수를 객체로 분리하는 것을 고려해보세요.크기 값을 상수 객체로 분리하면 유지보수성이 향상될 것 같습니다.
+const PROFILE_SIZES = { + large: '142px', + small: '40px', + extraSmall: '24px', +} as const; export const getProfileSize = (size: 'extraSmall' | 'small' | 'large') => { const style = { large: css({ - width: '142px', - height: '142px', + width: PROFILE_SIZES.large, + height: PROFILE_SIZES.large, }), small: css({ - width: '40px', - height: '40px', + width: PROFILE_SIZES.small, + height: PROFILE_SIZES.small, }), extraSmall: css({ - width: '24px', - height: '24px', + width: PROFILE_SIZES.extraSmall, + height: PROFILE_SIZES.extraSmall, }), }; return style[size]; };src/components/mobile/molecules/VoteToggle/VoteToggle.tsx (2)
34-36
: 오타를 수정해주세요.
currnetOption
을currentOption
으로 수정해주세요.
58-70
: 로컬 스토리지 키를 상수로 분리하는 것이 좋겠습니다.로컬 스토리지 키가 여러 곳에서 반복되어 사용되고 있습니다. 상수로 분리하면 유지보수가 더 쉬워질 것 같습니다.
+const STORAGE_KEY_PREFIX = 'talkpick_'; +const getStorageKey = (id: number) => `${STORAGE_KEY_PREFIX}${id}`; const handleLoggedOutTalkPickVote = (voteOption: VoteOption) => { if (loggedOutVoteOption === voteOption) { setLoggedOutVoteOption(null); - localStorage.removeItem(`talkpick_${talkPickId}`); + localStorage.removeItem(getStorageKey(talkPickId)); } else { setLoggedOutVoteOption(voteOption); - localStorage.setItem(`talkpick_${talkPickId}`, voteOption); + localStorage.setItem(getStorageKey(talkPickId), voteOption);src/components/mobile/molecules/SummaryBox/SummaryBox.style.ts (2)
29-31
: 애니메이션 지속 시간을 상수로 분리하는 것이 좋겠습니다.애니메이션 지속 시간을 상수로 분리하면 일관성 있는 관리가 가능할 것 같습니다.
+const ANIMATION_DURATION = '2s'; export const summarySpinnerStyling = css({ - animation: `${rotate} 2s infinite linear`, + animation: `${rotate} ${ANIMATION_DURATION} infinite linear`, });
56-63
: transform과 margin 값을 상수로 분리하는 것이 좋겠습니다.반복되는 스타일 값들을 상수로 분리하면 유지보수가 더 쉬워질 것 같습니다.
+const ICON_SCALE = 0.75; +const SPINNER_MARGIN = '10px'; export const iconStyling = css({ - transform: 'scale(0.75)', + transform: `scale(${ICON_SCALE})`, }); export const spinnerStyling = css({ - transform: 'scale(0.75)', - margin: '10px', + transform: `scale(${ICON_SCALE})`, + margin: SPINNER_MARGIN, });src/components/mobile/molecules/ReportModal/ReportModal.style.ts (2)
37-38
: 버튼 크기가 모바일 화면에 비해 너무 큽니다.
width: '143px'
와height: '50px'
는 모바일 화면에서 상당히 큰 크기입니다. 모바일 환경에서의 사용성을 고려하여 버튼 크기를 조정하는 것이 좋습니다.export const buttonStyling = css(typo.Mobile.Text.Medium_14, { display: 'flex', - width: '143px', - height: '50px', + width: '120px', + height: '44px', alignItems: 'center', padding: '0 16px', borderRadius: '8px',
69-71
: 자동 완성 스타일이 불완전합니다.자동 완성 시 배경색만 설정되어 있고, 텍스트 색상이 설정되어 있지 않아 가독성 문제가 발생할 수 있습니다.
':-webkit-autofill': { boxShadow: '0 0 0px 1000px white inset', + '-webkit-text-fill-color': color.BK, },
src/components/mobile/organisms/TalkPickSection/TalkPickSection.style.ts (1)
126-131
: 이미지 최적화가 필요합니다.이미지 스타일링에 최적화 속성이 누락되어 있습니다. 모바일 환경에서의 성능 향상을 위해 이미지 최적화 속성을 추가하는 것이 좋습니다.
'& > img': { maxWidth: '100%', width: 'auto', height: 'auto', borderRadius: '5px', + objectFit: 'cover', + loading: 'lazy', },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/assets/svg/angle-small-down.svg
is excluded by!**/*.svg
src/assets/svg/angle-small-up.svg
is excluded by!**/*.svg
src/assets/svg/mobile-report.svg
is excluded by!**/*.svg
src/assets/svg/pick-icon.svg
is excluded by!**/*.svg
📒 Files selected for processing (28)
src/assets/index.ts
(1 hunks)src/components/atoms/ProfileIcon/ProfileIcon.style.ts
(1 hunks)src/components/atoms/ProfileIcon/ProfileIcon.tsx
(2 hunks)src/components/mobile/atoms/Button/Button.style.ts
(3 hunks)src/components/mobile/atoms/Button/Button.tsx
(1 hunks)src/components/mobile/atoms/SummaryItem/SummaryItem.style.ts
(1 hunks)src/components/mobile/atoms/SummaryItem/SummaryItem.tsx
(1 hunks)src/components/mobile/molecules/ReportModal/ReportModal.style.ts
(1 hunks)src/components/mobile/molecules/ReportModal/ReportModal.tsx
(1 hunks)src/components/mobile/molecules/SummaryBox/SummaryBox.style.ts
(1 hunks)src/components/mobile/molecules/SummaryBox/SummaryBox.tsx
(1 hunks)src/components/mobile/molecules/VoteToggle/VoteToggle.style.ts
(1 hunks)src/components/mobile/molecules/VoteToggle/VoteToggle.tsx
(1 hunks)src/components/mobile/organisms/TalkPickSection/TalkPickSection.style.ts
(1 hunks)src/components/mobile/organisms/TalkPickSection/TalkPickSection.tsx
(1 hunks)src/components/organisms/TalkPickSection/TalkPickSection.tsx
(1 hunks)src/stories/atoms/ProfileIcon.stories.tsx
(2 hunks)src/stories/mobile/atoms/Button.stories.tsx
(2 hunks)src/stories/mobile/atoms/SummaryItem.stories.tsx
(1 hunks)src/stories/mobile/molecules/ReportModal.stories.tsx
(1 hunks)src/stories/mobile/molecules/SummaryBox.stories.tsx
(1 hunks)src/stories/mobile/molecules/VoteToggle.stories.tsx
(1 hunks)src/stories/mobile/organisms/TalkPickSection.stories.tsx
(1 hunks)src/stories/molecules/SummaryBox.stories.tsx
(2 hunks)src/stories/organisms/TalkPickSection.stories.tsx
(1 hunks)src/styles/color.ts
(1 hunks)src/styles/typo.ts
(2 hunks)src/types/talk-pick.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/components/mobile/atoms/SummaryItem/SummaryItem.style.ts
- src/components/mobile/molecules/VoteToggle/VoteToggle.style.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/mobile/molecules/ReportModal/ReportModal.tsx
[error] 39-50: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
src/components/mobile/organisms/TalkPickSection/TalkPickSection.tsx
[error] 212-215: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: storybook
- GitHub Check: build
🔇 Additional comments (15)
src/types/talk-pick.ts (1)
25-25
: LGTM!
writerProfileImgUrl
타입이 적절하게 정의되었습니다.src/styles/color.ts (1)
10-10
: LGTM!기존 회색 계열 색상 팔레트에 잘 어울리는 색상이 추가되었습니다.
src/components/organisms/TalkPickSection/TalkPickSection.tsx (1)
223-223
: LGTM!
talkPickId
의 기본값이 0으로 변경된 것은 적절해 보입니다.src/assets/index.ts (1)
154-155
: 새로운 아이콘 추가가 잘 되었습니다!새로운 모바일 리포트 아이콘과 픽 아이콘이 기존 패턴을 잘 따르고 있습니다.
src/components/mobile/atoms/SummaryItem/SummaryItem.tsx (1)
4-7
: 타입 정의가 명확합니다.itemNumber의 타입을 리터럴 유니온으로 제한한 것이 좋습니다.
src/styles/typo.ts (1)
238-244
: 새로운 모바일 타이포그래피 스타일이 잘 구현되었습니다!새로 추가된 Medium_14, Regular_12, Regular_10 스타일이 기존 타이포그래피 시스템의 일관성을 잘 유지하고 있습니다.
Also applies to: 266-279
src/stories/mobile/atoms/Button.stories.tsx (1)
18-24
: 버튼 컴포넌트의 새로운 variant가 잘 구현되었습니다!outlineHighlightR과 outlineHighlightB variant가 스토리북에 적절히 추가되어 있으며, 각각의 사용 예시가 명확하게 표현되어 있습니다.
Also applies to: 66-73
src/stories/atoms/ProfileIcon.stories.tsx (1)
21-21
: ProfileIcon 컴포넌트의 extraSmall 사이즈가 잘 구현되었습니다!새로운 extraSmall 사이즈 옵션이 기본 케이스와 커스텀 이미지 케이스 모두에 대해 적절히 구현되어 있습니다.
Also applies to: 40-42, 47-52
src/components/atoms/ProfileIcon/ProfileIcon.tsx (1)
12-12
: ProfileIcon 컴포넌트의 타입과 스타일 적용 방식이 개선되었습니다!다음과 같은 개선사항이 잘 구현되어 있습니다:
- size 타입에 extraSmall 옵션 추가
- CSS 스타일 적용 방식을 배열 구문으로 개선
- 명시적인 button type 지정
Also applies to: 18-18, 32-37, 42-47
src/stories/mobile/atoms/SummaryItem.stories.tsx (1)
13-24
: argTypes와 args 설정이 잘 구성되어 있습니다!컴포넌트의 props가 명확하게 정의되어 있고, 적절한 컨트롤 타입이 설정되어 있습니다.
src/components/mobile/molecules/SummaryBox/SummaryBox.tsx (1)
8-11
: Props 타입 정의가 명확합니다.인터페이스가 잘 정의되어 있으며, 적절한 옵셔널 프로퍼티를 사용하고 있습니다.
src/stories/molecules/SummaryBox.stories.tsx (1)
7-17
: 예시 데이터가 잘 구성되어 있습니다.테스트용 데이터가 실제 사용 사례를 잘 반영하고 있습니다.
src/stories/mobile/molecules/SummaryBox.stories.tsx (1)
7-17
: 모바일용 예시 데이터가 적절합니다.실제 모바일 환경에서 발생할 수 있는 데이터를 잘 반영하고 있습니다.
src/components/atoms/ProfileIcon/ProfileIcon.style.ts (1)
3-14
: 스타일 분리가 잘 되어있습니다!기본 스타일과 크기 관련 스타일을 분리한 것이 좋은 접근입니다.
src/components/mobile/organisms/TalkPickSection/TalkPickSection.tsx (1)
143-147
: ReportModal의 onConfirm 핸들러가 비어있습니다.신고 모달의 확인 버튼 핸들러가 구현되지 않았습니다. 신고 처리 로직을 추가해주세요.
|
||
export const Default: Story = { | ||
play: () => { | ||
store.dispatch(setToken('accessToken')); |
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.
민감한 정보 노출에 주의하세요.
스토리북 파일에 하드코딩된 액세스 토큰은 보안상 위험할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
- store.dispatch(setToken('accessToken'));
+ store.dispatch(setToken('dummy_token_for_storybook'));
📝 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.
store.dispatch(setToken('accessToken')); | |
store.dispatch(setToken('dummy_token_for_storybook')); |
{reportReason === '기타' && ( | ||
<input | ||
css={S.reportInputStyling} | ||
placeholder="신고사유를 작성해주세요." | ||
onChange={handleOtherReportReason} | ||
/> | ||
)} |
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.
🛠️ Refactor suggestion
입력 필드에 대한 유효성 검사를 추가하세요.
기타 사유 입력 시 최소/최대 길이 제한과 같은 유효성 검사가 필요합니다.
<input
css={S.reportInputStyling}
placeholder="신고사유를 작성해주세요."
+ minLength={10}
+ maxLength={200}
onChange={handleOtherReportReason}
+ required
/>
📝 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.
{reportReason === '기타' && ( | |
<input | |
css={S.reportInputStyling} | |
placeholder="신고사유를 작성해주세요." | |
onChange={handleOtherReportReason} | |
/> | |
)} | |
{reportReason === '기타' && ( | |
<input | |
css={S.reportInputStyling} | |
placeholder="신고사유를 작성해주세요." | |
minLength={10} | |
maxLength={200} | |
onChange={handleOtherReportReason} | |
required | |
/> | |
)} |
<input | ||
css={S.reportInputStyling} | ||
placeholder="신고사유를 작성해주세요." | ||
onChange={handleOtherReportReason} | ||
/> |
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.
🛠️ Refactor suggestion
접근성을 개선하세요.
입력 필드에 적절한 레이블과 ARIA 속성이 필요합니다.
+<label htmlFor="otherReason" css={S.srOnly}>
+ 기타 신고 사유
+</label>
<input
+ id="otherReason"
css={S.reportInputStyling}
placeholder="신고사유를 작성해주세요."
+ aria-label="기타 신고 사유"
onChange={handleOtherReportReason}
/>
📝 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.
<input | |
css={S.reportInputStyling} | |
placeholder="신고사유를 작성해주세요." | |
onChange={handleOtherReportReason} | |
/> | |
<label htmlFor="otherReason" css={S.srOnly}> | |
기타 신고 사유 | |
</label> | |
<input | |
id="otherReason" | |
css={S.reportInputStyling} | |
placeholder="신고사유를 작성해주세요." | |
aria-label="기타 신고 사유" | |
onChange={handleOtherReportReason} | |
/> |
{reportOptions.map((option) => ( | ||
<button | ||
type="button" | ||
value={option.value} | ||
onClick={() => { | ||
setReportReason(option.value); | ||
setOtherReason(''); | ||
}} | ||
css={[ | ||
S.buttonStyling, | ||
option.value === reportReason && S.selectedButtonStyling, | ||
]} | ||
> | ||
{option.label} | ||
</button> | ||
))} |
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.
map 함수 사용 시 key prop을 추가하세요.
리스트 렌더링 시 각 요소에 고유한 key prop이 필요합니다.
{reportOptions.map((option) => (
<button
+ key={option.value}
type="button"
value={option.value}
📝 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.
{reportOptions.map((option) => ( | |
<button | |
type="button" | |
value={option.value} | |
onClick={() => { | |
setReportReason(option.value); | |
setOtherReason(''); | |
}} | |
css={[ | |
S.buttonStyling, | |
option.value === reportReason && S.selectedButtonStyling, | |
]} | |
> | |
{option.label} | |
</button> | |
))} | |
{reportOptions.map((option) => ( | |
<button | |
key={option.value} | |
type="button" | |
value={option.value} | |
onClick={() => { | |
setReportReason(option.value); | |
setOtherReason(''); | |
}} | |
css={[ | |
S.buttonStyling, | |
option.value === reportReason && S.selectedButtonStyling, | |
]} | |
> | |
{option.label} | |
</button> | |
))} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 39-50: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
const handleTalkPickVote = ( | ||
selectedOption: MyVoteOption, | ||
voteOption: VoteOption, | ||
) => { | ||
if (selectedOption === null) { | ||
createTalkPickVote(voteOption); | ||
} else if (selectedOption === voteOption) { | ||
deleteTalkPickVote(); | ||
} else { | ||
editTalkPickVote(voteOption); | ||
} | ||
}; |
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.
🛠️ Refactor suggestion
에러 처리를 추가하는 것이 좋겠습니다.
mutation 함수 호출 시 에러 처리가 없습니다. try-catch 블록을 추가하여 에러 상황을 처리하는 것이 좋겠습니다.
const handleTalkPickVote = (
selectedOption: MyVoteOption,
voteOption: VoteOption,
) => {
+ try {
if (selectedOption === null) {
createTalkPickVote(voteOption);
} else if (selectedOption === voteOption) {
deleteTalkPickVote();
} else {
editTalkPickVote(voteOption);
}
+ } catch (error) {
+ showToastModal('투표 처리 중 오류가 발생했습니다.');
+ }
};
📝 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.
const handleTalkPickVote = ( | |
selectedOption: MyVoteOption, | |
voteOption: VoteOption, | |
) => { | |
if (selectedOption === null) { | |
createTalkPickVote(voteOption); | |
} else if (selectedOption === voteOption) { | |
deleteTalkPickVote(); | |
} else { | |
editTalkPickVote(voteOption); | |
} | |
}; | |
const handleTalkPickVote = ( | |
selectedOption: MyVoteOption, | |
voteOption: VoteOption, | |
) => { | |
try { | |
if (selectedOption === null) { | |
createTalkPickVote(voteOption); | |
} else if (selectedOption === voteOption) { | |
deleteTalkPickVote(); | |
} else { | |
editTalkPickVote(voteOption); | |
} | |
} catch (error) { | |
showToastModal('투표 처리 중 오류가 발생했습니다.'); | |
} | |
}; |
const { mutate: deleteTalkPick } = useDeleteTalkPickMutation( | ||
talkPick?.id ?? 0, | ||
); | ||
|
||
const handleDeleteButton = () => { | ||
deleteTalkPick(); | ||
onCloseModal(); | ||
}; |
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.
🛠️ Refactor suggestion
삭제 작업 후 피드백이 필요합니다.
삭제 작업 완료 후 사용자에게 피드백을 제공하고 적절한 페이지로 리다이렉션하는 것이 좋겠습니다.
const handleDeleteButton = () => {
- deleteTalkPick();
+ deleteTalkPick({
+ onSuccess: () => {
+ showToastModal('톡픽이 삭제되었습니다.');
+ navigate('/');
+ },
+ onError: () => {
+ showToastModal('삭제 중 오류가 발생했습니다.');
+ },
+ });
onCloseModal();
};
📝 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.
const { mutate: deleteTalkPick } = useDeleteTalkPickMutation( | |
talkPick?.id ?? 0, | |
); | |
const handleDeleteButton = () => { | |
deleteTalkPick(); | |
onCloseModal(); | |
}; | |
const { mutate: deleteTalkPick } = useDeleteTalkPickMutation( | |
talkPick?.id ?? 0, | |
); | |
const handleDeleteButton = () => { | |
deleteTalkPick({ | |
onSuccess: () => { | |
showToastModal('톡픽이 삭제되었습니다.'); | |
navigate('/'); | |
}, | |
onError: () => { | |
showToastModal('삭제 중 오류가 발생했습니다.'); | |
}, | |
}); | |
onCloseModal(); | |
}; |
export const getButtonStyling = (selected: boolean) => | ||
css({ | ||
width: '293px', | ||
height: '44px', | ||
borderRadius: '12px', | ||
backgroundColor: selected ? color.MAIN : color.GY[2], | ||
}); |
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.
🛠️ Refactor suggestion
선택된 버튼의 너비가 일관성이 없습니다.
getButtonStyling
함수에서 설정된 버튼 너비(293px)가 buttonStyling
의 너비(143px)와 크게 다릅니다. 일관된 사용자 경험을 위해 버튼 크기를 통일하는 것이 좋습니다.
export const getButtonStyling = (selected: boolean) =>
css({
- width: '293px',
+ width: '120px',
height: '44px',
borderRadius: '12px',
backgroundColor: selected ? color.MAIN : color.GY[2],
});
Committable suggestion skipped: line range outside the PR's diff.
export const talkPickWrapper = css({ | ||
display: 'flex', | ||
flexDirection: 'column', | ||
width: '330px', | ||
padding: '10px 0', | ||
borderTop: `1px solid${color.GY[4]}`, | ||
}); |
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.
🛠️ Refactor suggestion
고정된 너비 값이 모바일 반응형에 적합하지 않습니다.
width: '330px'
와 같은 고정된 너비는 다양한 모바일 화면 크기에 대응하기 어렵습니다. 상대적인 단위를 사용하는 것이 좋습니다.
export const talkPickWrapper = css({
display: 'flex',
flexDirection: 'column',
- width: '330px',
+ width: '100%',
+ maxWidth: '330px',
padding: '10px 0',
- borderTop: `1px solid${color.GY[4]}`,
+ borderTop: `1px solid ${color.GY[4]}`,
});
📝 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.
export const talkPickWrapper = css({ | |
display: 'flex', | |
flexDirection: 'column', | |
width: '330px', | |
padding: '10px 0', | |
borderTop: `1px solid${color.GY[4]}`, | |
}); | |
export const talkPickWrapper = css({ | |
display: 'flex', | |
flexDirection: 'column', | |
width: '100%', | |
maxWidth: '330px', | |
padding: '10px 0', | |
borderTop: `1px solid ${color.GY[4]}`, | |
}); |
export const talkPickTitleStyling = css(typo.Main.SemiBold, { | ||
width: '300px', | ||
wordBreak: 'break-all', | ||
whiteSpace: 'normal', | ||
color: color.BK, | ||
}); |
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.
🛠️ Refactor suggestion
제목 스타일에 접근성 고려가 필요합니다.
제목의 너비가 고정되어 있어 화면 크기에 따른 유연한 대응이 어렵습니다. 또한, 접근성을 위해 최소 글자 크기를 보장해야 합니다.
export const talkPickTitleStyling = css(typo.Main.SemiBold, {
- width: '300px',
+ width: '100%',
+ maxWidth: '300px',
+ minWidth: '200px',
wordBreak: 'break-all',
whiteSpace: 'normal',
color: color.BK,
+ fontSize: 'clamp(16px, 4vw, 20px)',
});
📝 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.
export const talkPickTitleStyling = css(typo.Main.SemiBold, { | |
width: '300px', | |
wordBreak: 'break-all', | |
whiteSpace: 'normal', | |
color: color.BK, | |
}); | |
export const talkPickTitleStyling = css(typo.Main.SemiBold, { | |
width: '100%', | |
maxWidth: '300px', | |
minWidth: '200px', | |
wordBreak: 'break-all', | |
whiteSpace: 'normal', | |
color: color.BK, | |
fontSize: 'clamp(16px, 4vw, 20px)', | |
}); |
export const toastModalStyling = css({ | ||
position: 'fixed', | ||
top: '65px', | ||
left: '50%', | ||
transform: 'translate(-50%)', | ||
zIndex: '1000', | ||
}); |
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.
🛠️ Refactor suggestion
토스트 모달의 접근성 개선이 필요합니다.
토스트 모달의 위치가 고정되어 있어 다양한 모바일 기기에서 일관된 사용자 경험을 제공하기 어렵습니다. 또한, 접근성을 위한 속성이 누락되어 있습니다.
export const toastModalStyling = css({
position: 'fixed',
- top: '65px',
+ top: 'min(65px, 10vh)',
left: '50%',
transform: 'translate(-50%)',
zIndex: '1000',
+ maxWidth: '90vw',
+ role: 'alert',
+ ariaLive: 'polite',
});
Committable suggestion skipped: line range outside the PR's diff.
|
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: 4
♻️ Duplicate comments (2)
src/components/mobile/molecules/ReportModal/ReportModal.tsx (1)
55-61
:⚠️ Potential issue입력 필드의 유효성 검사와 접근성이 누락되었습니다.
이전 리뷰에서 지적된 유효성 검사와 접근성 문제가 아직 해결되지 않았습니다.
+<label htmlFor="reportReason" css={S.srOnly}> + 기타 신고 사유 +</label> <input + id="reportReason" css={S.reportInputStyling} placeholder="신고사유를 작성해주세요." + minLength={10} + maxLength={200} + aria-label="기타 신고 사유" + required onChange={handleOtherReportReason} />src/components/mobile/organisms/TalkPickSection/TalkPickSection.tsx (1)
111-114
:⚠️ Potential issue삭제 작업 후 피드백과 에러 처리가 누락되었습니다.
이전 리뷰에서 지적된 삭제 작업 후 피드백과 에러 처리가 아직 구현되지 않았습니다.
const handleDeleteButton = () => { - deleteTalkPick(); + deleteTalkPick({ + onSuccess: () => { + showToastModal('톡픽이 삭제되었습니다.'); + navigate('/'); + }, + onError: (error) => { + showToastModal('삭제 중 오류가 발생했습니다.'); + console.error('Delete error:', error); + }, + }); onCloseModal(); };
🧹 Nitpick comments (2)
src/components/mobile/molecules/ReportModal/ReportModal.tsx (1)
62-69
: 버튼의 비활성화 상태 처리가 필요합니다.현재는 스타일링으로만 처리되어 있으며, 실제 버튼의 비활성화 상태가 적용되어 있지 않습니다.
<Button size="large" variant="primary" onClick={handleConfirm} + disabled={!finalReportReason.trim()} css={S.getButtonStyling(!!finalReportReason)} > 설정 완료 </Button>
src/components/mobile/organisms/TalkPickSection/TalkPickSection.tsx (1)
177-183
: ProfileIcon의 접근성 개선이 필요합니다.프로필 이미지에 대한 적절한 대체 텍스트가 누락되어 있습니다.
<ProfileIcon interaction={ talkPick?.writerProfileImgUrl ? 'custom' : 'default' } imgUrl={talkPick?.writerProfileImgUrl ?? ''} size="extraSmall" + alt={`${talkPick?.writer}님의 프로필 이미지`} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/mobile/molecules/ReportModal/ReportModal.tsx
(1 hunks)src/components/mobile/organisms/TalkPickSection/TalkPickSection.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: storybook
export interface ReportModalProps { | ||
isOpen?: boolean; | ||
onConfirm?: (reason: string) => void; | ||
onClose?: () => void; | ||
} |
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.
🛠️ Refactor suggestion
Props 타입에 필수값 표시가 필요합니다.
Props 타입에서 선택적 필드(?
)를 사용하고 있지만, 모달의 정상적인 작동을 위해서는 이러한 props가 필수적입니다.
export interface ReportModalProps {
- isOpen?: boolean;
- onConfirm?: (reason: string) => void;
- onClose?: () => void;
+ isOpen: boolean;
+ onConfirm: (reason: string) => void;
+ onClose: () => void;
}
📝 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.
export interface ReportModalProps { | |
isOpen?: boolean; | |
onConfirm?: (reason: string) => void; | |
onClose?: () => void; | |
} | |
export interface ReportModalProps { | |
isOpen: boolean; | |
onConfirm: (reason: string) => void; | |
onClose: () => void; | |
} |
const handleConfirm = () => { | ||
if (!finalReportReason.trim()) return; | ||
onConfirm?.(finalReportReason); | ||
}; |
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.
🛠️ Refactor suggestion
handleConfirm 함수의 오류 처리가 필요합니다.
빈 문자열 체크만 하고 있으며, 최소/최대 길이 검증이나 사용자 피드백이 없습니다.
const handleConfirm = () => {
- if (!finalReportReason.trim()) return;
+ if (!finalReportReason.trim()) {
+ showToastModal('신고 사유를 입력해주세요.');
+ return;
+ }
+ if (reportReason === '기타' && otherReason.length < 10) {
+ showToastModal('신고 사유는 최소 10자 이상 입력해주세요.');
+ return;
+ }
onConfirm?.(finalReportReason);
};
📝 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.
const handleConfirm = () => { | |
if (!finalReportReason.trim()) return; | |
onConfirm?.(finalReportReason); | |
}; | |
const handleConfirm = () => { | |
if (!finalReportReason.trim()) { | |
showToastModal('신고 사유를 입력해주세요.'); | |
return; | |
} | |
if (reportReason === '기타' && otherReason.length < 10) { | |
showToastModal('신고 사유는 최소 10자 이상 입력해주세요.'); | |
return; | |
} | |
onConfirm?.(finalReportReason); | |
}; |
const handleBookmarkClick = () => { | ||
if (!talkPick) return; | ||
|
||
if (myTalkPick) { | ||
showToastModal(ERROR.BOOKMARK.MY_TALKPICK); | ||
return; | ||
} | ||
|
||
if (talkPick.myBookmark) { | ||
deleteBookmark(); | ||
} else { | ||
createBookmark(); | ||
} | ||
}; |
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.
🛠️ Refactor suggestion
북마크 작업의 에러 처리와 사용자 피드백이 필요합니다.
북마크 생성/삭제 작업에 대한 성공/실패 피드백이 누락되어 있습니다.
const handleBookmarkClick = () => {
if (!talkPick) return;
if (myTalkPick) {
showToastModal(ERROR.BOOKMARK.MY_TALKPICK);
return;
}
if (talkPick.myBookmark) {
- deleteBookmark();
+ deleteBookmark({
+ onSuccess: () => showToastModal('북마크가 삭제되었습니다.'),
+ onError: () => showToastModal('북마크 삭제 중 오류가 발생했습니다.'),
+ });
} else {
- createBookmark();
+ createBookmark({
+ onSuccess: () => showToastModal('북마크가 추가되었습니다.'),
+ onError: () => showToastModal('북마크 추가 중 오류가 발생했습니다.'),
+ });
}
};
📝 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.
const handleBookmarkClick = () => { | |
if (!talkPick) return; | |
if (myTalkPick) { | |
showToastModal(ERROR.BOOKMARK.MY_TALKPICK); | |
return; | |
} | |
if (talkPick.myBookmark) { | |
deleteBookmark(); | |
} else { | |
createBookmark(); | |
} | |
}; | |
const handleBookmarkClick = () => { | |
if (!talkPick) return; | |
if (myTalkPick) { | |
showToastModal(ERROR.BOOKMARK.MY_TALKPICK); | |
return; | |
} | |
if (talkPick.myBookmark) { | |
deleteBookmark({ | |
onSuccess: () => showToastModal('북마크가 삭제되었습니다.'), | |
onError: () => showToastModal('북마크 삭제 중 오류가 발생했습니다.'), | |
}); | |
} else { | |
createBookmark({ | |
onSuccess: () => showToastModal('북마크가 추가되었습니다.'), | |
onError: () => showToastModal('북마크 추가 중 오류가 발생했습니다.'), | |
}); | |
} | |
}; |
<ReportModal | ||
isOpen={activeModal === 'reportTalkPick'} | ||
onConfirm={() => {}} | ||
onClose={onCloseModal} | ||
/> |
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.
ReportModal의 onConfirm 핸들러가 비어있습니다.
신고 기능이 실제로 구현되어 있지 않습니다.
<ReportModal
isOpen={activeModal === 'reportTalkPick'}
- onConfirm={() => {}}
+ onConfirm={(reason) => {
+ // TODO: Implement report API call
+ showToastModal('신고가 접수되었습니다.');
+ onCloseModal();
+ }}
onClose={onCloseModal}
/>
📝 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.
<ReportModal | |
isOpen={activeModal === 'reportTalkPick'} | |
onConfirm={() => {}} | |
onClose={onCloseModal} | |
/> | |
<ReportModal | |
isOpen={activeModal === 'reportTalkPick'} | |
onConfirm={(reason) => { | |
// TODO: Implement report API call | |
showToastModal('신고가 접수되었습니다.'); | |
onCloseModal(); | |
}} | |
onClose={onCloseModal} | |
/> |
💡 작업 내용
💡 자세한 설명
✅ ProfileIcon
✅ SummaryBox
✅ TalkPickSection
📗 참고 자료 (선택)
📢 리뷰 요구 사항 (선택)
🚩 후속 작업 (선택)
✅ 셀프 체크리스트
closes #301
Summary by CodeRabbit
New Features
Style Improvements