code review #200
code review #200
Conversation
Walkthroughμ΄ PRμμλ Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the toolβs configuration or disable the tool if itβs a critical failure. π§ eslint (1.23.1)
src/components/common/crew-list/crew-card-list.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't determine the plugin "react-hooks" uniquely.
Please remove the "plugins" setting from either config or remove either plugin installation. If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 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 (
|
![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
π§Ή Outside diff range and nitpick comments (7)
src/components/common/modal/confirm-cancel-modal.tsx (1)
Line range hint
71-74: μ€νμΌ μμ λΆλ¦¬λ₯Ό μ μν©λλ€Modal μ»΄ν¬λνΈμ μ€νμΌ κ°μ²΄λ₯Ό μμλ‘ λΆλ¦¬νλ©΄ μ¬μ¬μ©μ±κ³Ό μ μ§λ³΄μμ±μ΄ ν₯μλ κ² κ°μ΅λλ€.
λ€μκ³Ό κ°μ΄ λ³κ²½νλ κ²μ μ μν©λλ€:
+const MODAL_STYLES = { + content: { + boxShadow: '0 25px 50px -12px rgba(0,0,0,0.1)', + borderRadius: '12px' + } +}; + +const OVERLAY_PROPS = { + opacity: 0.5, + blur: 2, +}; <Modal opened={opened} onClose={handleCancel} centered withCloseButton={false} size="xs" - styles={{ - content: { boxShadow: '0 25px 50px -12px rgba(0,0,0,0.1)', borderRadius: '12px' }, - }} - overlayProps={{ - opacity: 0.5, - blur: 2, - }} + styles={MODAL_STYLES} + overlayProps={OVERLAY_PROPS} >src/components/common/profile/index.tsx (3)
Line range hint
19-25: νμ μ μ κ°μ μ΄ νμν©λλ€size νμ μ λ³λμ νμ μΌλ‘ λΆλ¦¬νμ¬ κ΄λ¦¬νλ©΄ μ¬μ¬μ©μ±κ³Ό μ μ§λ³΄μμ±μ΄ ν₯μλ κ² κ°μ΅λλ€.
+type ProfileSize = 'small' | 'header' | 'large' | 'full'; interface ProfileProps { - size?: 'small' | 'header' | 'large' | 'full'; + size?: ProfileSize; imageUrl?: string; editable?: boolean; isCaptain?: boolean; onClick?: () => void; priority?: boolean; }
Line range hint
44-51: μμ μ μλ₯Ό μ»΄ν¬λνΈ μΈλΆλ‘ μ΄λν΄μΌ ν©λλ€μ»΄ν¬λνΈ λ΄λΆμ μμ μ μλ₯Ό μΈλΆλ‘ μ΄λνλ κ²μ λμν©λλ€. λ€μκ³Ό κ°μ΄ κ°μ νλ©΄ μ’μ κ² κ°μ΅λλ€:
+const PROFILE_SIZE_CLASSES = { + small: 'w-6 h-6', + header: 'sm:w-7 sm:h-7 md:w-[40px] md:h-[40px] lg:w-[40px] lg:h-[40px]', + large: 'w-14 h-14', + full: 'w-full h-full', + captain: 'w-20 h-20', +} as const; export function Profile({ size = 'full', imageUrl, editable = false, isCaptain = false, onClick, priority = false, }: ProfileProps) { - const sizeClasses = { - small: 'w-6 h-6', - header: 'sm:w-7 sm:h-7 md:w-[40px] md:h-[40px] lg:w-[40px] lg:h-[40px]', - large: 'w-14 h-14', - full: 'w-full h-full', - captain: 'w-20 h-20', - };
Line range hint
35-43: size κ²°μ λ‘μ§ λ¨μνκ° νμν©λλ€νμ¬ size κ²°μ λ‘μ§μ΄ λ€μ 볡μ‘ν©λλ€. μ°μ μμλ₯Ό λͺ νν νκ³ early return ν¨ν΄μ μ¬μ©νμ¬ κ°μ ν μ μμ΅λλ€.
- let finalSize = size; - if (editable) { - finalSize = 'full'; - } else if (isCaptain) { - finalSize = 'large'; - } + const getFinalSize = () => { + if (editable) return 'full'; + if (isCaptain) return 'large'; + return size; + }; + + const finalSize = getFinalSize();src/components/common/crew-list/crew-card-list.tsx (3)
Line range hint
21-25: νμ μμ μ± κ°μ νμνμ¬ νμ λ¨μΈ(as)μ μ¬μ©νμ¬ λ°μ΄ν°λ₯Ό λ³ννκ³ μμ΅λλ€. μ΄λ λ°νμ μλ¬μ μνμ΄ μμ΅λλ€.
λ€μκ³Ό κ°μ΄ μ λ€λ¦κ³Ό νμ κ°λλ₯Ό νμ©νμ¬ κ°μ ν μ μμ΅λλ€:
function getCrewDataList<T extends MyCrewList | MainCrewList>( data: InfiniteData<MyCrewListResponse | MainCrewListResponse>, type: 'my-crew' | 'main-crew' ): T[] { return data.pages.flatMap((page) => page.content as T[]); }
Line range hint
44-71: 볡μ‘ν μ‘°κ±΄λΆ λ λλ§ λ‘μ§ λ¨μν νμνμ¬ μ½λμμλ μ¬λ¬ λ²μ νμ 체ν¬μ ν λ³νμ΄ λ°μνκ³ μμ΄ κ°λ μ±κ³Ό μ μ§λ³΄μμ±μ΄ λ¨μ΄μ§λλ€.
μ»΄ν¬λνΈλ₯Ό λΆλ¦¬νμ¬ κ° νμ λ³λ‘ μ μ© λ λλ§ λ‘μ§μ ꡬννλ κ²μ μΆμ²λ립λλ€:
// MyCrewCardμ MainCrewCardλ‘ λΆλ¦¬ function MyCrewCard({ inform }: { inform: MyCrewList }) { return ( <CrewCard inWhere="my-crew" id={inform.id} // ... λλ¨Έμ§ props currentCount={inform.currentCount} totalGathering={inform.totalGathering} crewMembers={inform.crewMembers} /> ); } function MainCrewCard({ inform }: { inform: MainCrewList }) { const isConfirmed = inform.participantCount === inform.totalCount; return ( <CrewCard inWhere="main-crew" id={inform.id} // ... λλ¨Έμ§ props participantCount={inform.participantCount} totalGatheringCount={inform.totalGatheringCount} isConfirmed={isConfirmed} /> ); }
Line range hint
55-61: μ΄λ―Έμ§ URL μ²λ¦¬ λ‘μ§ κ°μ νμνλμ½λ©λ μ΄λ―Έμ§ URLκ³Ό λ¬Έμμ΄ λΉκ΅ λ‘μ§μ΄ μμ΅λλ€. μ΄λ μ μ§λ³΄μμ±μ μ νμν¬ μ μμ΅λλ€.
μμλ‘ λΆλ¦¬νκ³ μ ν¨μ± κ²μ¬ ν¨μλ₯Ό λ§λλ κ²μ μΆμ²λ립λλ€:
const DEFAULT_IMAGE_URL = 'https://media.istockphoto.com/...'; function isValidImageUrl(url: string): boolean { return url !== 'string' && url.length > 0; } // μ¬μ© imageUrl={isValidImageUrl(inform?.imageUrl) ? inform.imageUrl : DEFAULT_IMAGE_URL}
π Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
π Files selected for processing (8)
README.md(1 hunks)src/components/common/crew-list/crew-card-list.tsx(1 hunks)src/components/common/crew-list/crew-card.tsx(1 hunks)src/components/common/gathering-card/gathering-card.stories.tsx(1 hunks)src/components/common/modal/confirm-cancel-modal.tsx(1 hunks)src/components/common/profile/index.tsx(1 hunks)src/components/common/skeleton/reviewable-gathering-skeleton-list/index.tsx(1 hunks)src/types/create-crew.d.ts(1 hunks)
β Files skipped from review due to trivial changes (5)
- README.md
- src/components/common/crew-list/crew-card.tsx
- src/components/common/gathering-card/gathering-card.stories.tsx
- src/components/common/skeleton/reviewable-gathering-skeleton-list/index.tsx
- src/types/create-crew.d.ts
π Additional comments (4)
src/components/common/modal/confirm-cancel-modal.tsx (3)
Line range hint 11-17: μΈν°νμ΄μ€ μ μκ° λͺ
νν©λλ€!
νμ propsμ μ νμ propsκ° μ ꡬλΆλμ΄ μμΌλ©°, νμ μ΄ μ ννκ² μ§μ λμ΄ μμ΅λλ€.
Line range hint 19-45: JSDoc λ¬Έμνκ° μ λμ΄μμ΅λλ€!
μ¬μ© μμκΉμ§ ν¬ν¨λ μμΈν λ¬Έμνκ° μΈμμ μ λλ€.
5-9: μΈν°νμ΄μ€ νμ₯ μ μ κ²μ¦μ΄ νμν©λλ€
ConfirmModalPropsλ₯Ό νμ₯νλ κ²μ΄ μ’μ μ μμ΄μ§λ§, λ¨Όμ κΈ°μ‘΄ μΈν°νμ΄μ€μ ꡬ쑰λ₯Ό νμΈν΄μΌ ν©λλ€.
λ€μ μ€ν¬λ¦½νΈλ‘ ConfirmModalProps μΈν°νμ΄μ€λ₯Ό νμΈνκ² μ΅λλ€:
μΈν°νμ΄μ€ ꡬ쑰 νμΈ ν 리ν©ν λ§μ λμλ릴κΉμ?
β Verification successful
Let me check the implementation of both modal components to make a complete assessment.
μΈν°νμ΄μ€ νμ₯μ΄ μ μ ν©λλ€
ConfirmModalPropsμ ConfirmCancelModalPropsμ ꡬ쑰λ₯Ό νμΈν κ²°κ³Ό, μΈν°νμ΄μ€ νμ₯μ΄ λ§€μ° μ μ ν΄ λ³΄μ
λλ€. ConfirmModalPropsλ λ€μ μμ±λ€μ κ°μ§κ³ μμ΅λλ€:
- children: ReactNode
- opened: boolean
- onClose: () => void
- onConfirm: () => void
ConfirmCancelModal μ»΄ν¬λνΈλ μ΄ λͺ¨λ μμ±λ€μ μ¬μ©νλ©΄μ μΆκ°λ‘ onCancel μμ±λ§ μ νμ μΌλ‘ μ¬μ©νκ³ μμ΅λλ€. λ°λΌμ TODO μ½λ©νΈμμ μ μν λλ‘ λ€μκ³Ό κ°μ΄ νμ₯νλ©΄ μ’μ κ² κ°μ΅λλ€:
interface ConfirmCancelModalProps extends ConfirmModalProps {
onCancel?: () => void;
}π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# ConfirmModalProps μΈν°νμ΄μ€ κ²μ
ast-grep --pattern 'interface ConfirmModalProps {
$$$
}'
# κ΄λ ¨ νμΌ κ²μ
rg -l "ConfirmModalProps"
Length of output: 596
Script:
#!/bin/bash
# Get both modal implementations
rg -A 20 "export.*function.*Modal" src/components/common/modal/confirm-modal.tsx
rg -A 20 "export.*function.*Modal" src/components/common/modal/confirm-cancel-modal.tsx
Length of output: 1021
src/components/common/profile/index.tsx (1)
Line range hint 19-19: μλ‘μ΄ 'header' μ¬μ΄μ¦ μ¬μ© κ²μ¦μ΄ νμν©λλ€
μλ‘ μΆκ°λ 'header' μ¬μ΄μ¦κ° λ€λ₯Έ μ»΄ν¬λνΈμμ μ¬λ°λ₯΄κ² μ¬μ©λκ³ μλμ§ νμΈμ΄ νμν©λλ€.
β Verification successful
'header' μ¬μ΄μ¦κ° μ¬λ°λ₯΄κ² μ¬μ©λκ³ μμμ νμΈνμ΅λλ€
κ²μ κ²°κ³Όλ₯Ό ν΅ν΄ λ€μ μ¬νλ€μ νμΈνμ΅λλ€:
headerμ¬μ΄μ¦λ ν€λ μ»΄ν¬λνΈμμ μ μ νκ² μ¬μ©λκ³ μμ΅λλ€ (src/components/common/header/presenter.tsx)- Profile μ»΄ν¬λνΈμ size propμ
small,header,large,fullμ΅μ μ μ μμ μΌλ‘ μ§μν©λλ€ (Profile.stories.tsx) - λ€λ₯Έ μ»΄ν¬λνΈλ€μμλ κ°κ°μ μ©λμ λ§λ size κ°μ μΌκ΄λκ² μ¬μ©νκ³ μμ΅λλ€
π Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: header μ¬μ΄μ¦ μ¬μ© κ²μ¦
# Profile μ»΄ν¬λνΈμ size prop μ¬μ© κ²μ
rg "size=['\"]header['\"]" --type tsx
rg "<Profile.*header.*?>" --type tsx
Length of output: 138
Script:
#!/bin/bash
# Description: Profile μ»΄ν¬λνΈμ size prop μ¬μ© κ²μ (νμΌ νμ₯μ μμ )
# TypeScript/React νμΌμμ 'header' μ¬μ΄μ¦ μ¬μ© κ²μ
rg "size=['\"]header['\"]" --type ts --type tsx --type jsx
rg "<Profile.*header.*?>" --type ts --type tsx --type jsx
# Profile μ»΄ν¬λνΈ νμΌ μ°ΎκΈ°
fd -e tsx -e ts -e jsx "Profile"
# size propμ΄ μ¬μ©λ λͺ¨λ ν¨ν΄ κ²μ
rg "size=" --type ts --type tsx --type jsx
Length of output: 334
Script:
#!/bin/bash
# Profile μ»΄ν¬λνΈ κ΄λ ¨ νμΌ λ΄μ© νμΈ
cat src/components/common/profile/Profile.stories.tsx
# μ 체 μ½λλ² μ΄μ€μμ size μμ± κ²μ (νμ₯μ μ μΈ)
rg "size="
# Profile μ»΄ν¬λνΈ μ¬μ© κ²μ (νμ₯μ μ μΈ)
rg "<Profile"
# λͺ¨λ TypeScript/React νμΌ κ²μ
fd . -e ts -e tsx -e jsx
Length of output: 15865
| // TODO: νμ΄μ§ μ 체μμ inWhere κ° my-crew μΈμ§ main-crew μΈμ§ 체ν¬νλ λΆλΆμ΄ λ°λ³΅λλλ° μ΄λ₯Ό μ€μΌ μ μλ λ°©λ²μ΄ μμκΉμ? | ||
|
|
There was a problem hiding this comment.
π οΈ Refactor suggestion
TODO μ½λ©νΈμ λν 리ν©ν λ§ μ μ
inWhere 체ν¬λ₯Ό μ€μ΄κΈ° μν λͺ κ°μ§ λ°©λ²μ μ μλ립λλ€:
- 컀μ€ν ν μ λ§λ€μ΄ crew νμ λ³ λ‘μ§μ λΆλ¦¬
- νμ κ°λλ₯Ό μ¬μ©νμ¬ νμ λ¨μΈ(type assertion) μ€μ΄κΈ°
- Higher-Order Componentλ₯Ό μ¬μ©νμ¬ κ³΅ν΅ λ‘μ§ μΆμν
μμ μ½λ:
// νμ
κ°λ μμ
function isMyCrewList(crew: MyCrewList | MainCrewList): crew is MyCrewList {
return 'currentCount' in crew;
}
// 컀μ€ν
ν
μμ
function useCrewData(data: InfiniteData<MyCrewListResponse | MainCrewListResponse>, type: 'my-crew' | 'main-crew') {
return useMemo(() => ({
crewDataList: data.pages.flatMap((page) => page.content),
gridColsStyle: type === 'my-crew' ? '' : 'lg:grid-cols-2'
}), [data, type]);
}
1 participant
π Issue Ticket
Ticket
βοΈ Description
β Checklist
PR
Test
Summary by CodeRabbit
New Features
ConfirmCancelModalμ»΄ν¬λνΈκ° μΆκ°λμ΄ νμΈ λ° μ·¨μ λμμ κ΄λ¦¬ν©λλ€.Profileμ»΄ν¬λνΈμ μλ‘μ΄ ν¬κΈ° μ΅μ 'header'κ° μΆκ°λμμ΅λλ€.Documentation
README.mdνμΌμ 첫 μ¬μ©μ μλ΄λ₯Ό μν TODO νλͺ©μ΄ μΆκ°λμμ΅λλ€.Bug Fixes
CrewCardListμCrewCardμ»΄ν¬λνΈμμ λ°λ³΅μ μΈ μ²΄ν¬λ₯Ό μ€μ΄κΈ° μν μ£Όμμ΄ μΆκ°λμμ΅λλ€.Chores