Claude/nice allen#2140
Conversation
Summary of ChangesHello @dusvlf111, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 큐레이션 화면 내 챌린지 비교 기능을 전면적으로 재설계하고 개선합니다. 기존의 테이블 및 캐러셀 형태의 비교 UI를 제거하고, 사용자가 챌린지를 선택하여 비교할 수 있는 비교함 기능과 함께 전용 비교 결과 카드를 도입했습니다. 또한, 자주 비교되는 챌린지 조합을 추천하는 기능을 추가하여 사용자 경험을 향상시켰습니다. 이와 더불어, 스티키 내비게이션을 간소화하고 FAQ 섹션의 UI 안정성을 확보하는 등 전반적인 사용자 인터페이스를 개선했습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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
이번 PR은 큐레이션 챌린지 비교 기능을 전면 재설계하여 사용자 경험을 개선하고 코드를 정리하는 작업을 포함하고 있습니다. 기존의 복잡한 비교표와 캐러셀을 제거하고, 비교함(Cart) 개념을 도입하여 사용자가 원하는 프로그램을 직접 선택해 비교할 수 있도록 변경되었습니다. 전반적으로 코드의 모듈화가 잘 이루어졌으나, 일부 컴포넌트 내부에 비대한 렌더링 로직이 포함되어 있거나 매직 넘버가 사용된 부분이 있어 개선이 필요합니다. 특히 가이드라인에서 강조하는 매직 넘버의 상수화와 구현 세부 사항의 추상화 원칙을 적용하면 더 견고한 코드가 될 것입니다.
| const allPrograms = CHALLENGE_COMPARISON; | ||
| const row1 = allPrograms.slice(0, 4); | ||
| const row2 = allPrograms.slice(4); |
There was a problem hiding this comment.
4라는 숫자가 매직 넘버로 사용되었습니다. 가이드라인의 "Naming Magic Numbers" 규칙에 따라, 한 행에 표시할 아이템 개수를 의미하는 상수로 정의하여 가독성을 높여주세요.
| const allPrograms = CHALLENGE_COMPARISON; | |
| const row1 = allPrograms.slice(0, 4); | |
| const row2 = allPrograms.slice(4); | |
| const ITEMS_PER_ROW = 4; | |
| const allPrograms = CHALLENGE_COMPARISON; | |
| const row1 = allPrograms.slice(0, ITEMS_PER_ROW); | |
| const row2 = allPrograms.slice(ITEMS_PER_ROW); |
References
- 매직 넘버를 의미 있는 이름을 가진 상수로 교체하여 코드의 명확성을 높여야 합니다. (link)
| const renderCard = (challenge: (typeof allPrograms)[number]) => { | ||
| const program = PROGRAMS[challenge.programId]; | ||
| const inCart = isInCart(challenge.programId); | ||
| const bgColor = CARD_COLORS[challenge.programId]; | ||
| const isDark = bgColor === '#161c2f'; | ||
|
|
||
| return ( | ||
| <div key={challenge.programId} className="flex w-[240px] flex-col"> | ||
| {/* 카드 본체 */} | ||
| <div className="flex flex-col gap-3"> | ||
| {/* 썸네일 영역 */} | ||
| <div | ||
| className="flex h-[180px] w-[240px] items-end overflow-hidden rounded-[7px] p-4" | ||
| style={{ backgroundColor: bgColor }} | ||
| > | ||
| <div className="flex flex-col gap-0.5"> | ||
| <span | ||
| className={`text-xs font-medium ${isDark ? 'text-white/70' : 'text-white/80'}`} | ||
| > | ||
| {program.subtitle} | ||
| </span> | ||
| <span className="text-base font-bold text-white"> | ||
| {program.title} | ||
| </span> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* 텍스트 영역 */} | ||
| <div className="flex flex-col gap-3 px-1"> | ||
| <span className="line-clamp-2 text-lg font-bold leading-[26px] text-[#27272d]"> | ||
| {program.title} | ||
| </span> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="text-[11px] font-medium leading-[14px] text-[#27272d]"> | ||
| 진행기간 | ||
| </span> | ||
| <span className="text-[11px] font-medium leading-[14px] text-[#4138a3]"> | ||
| {program.duration} | ||
| </span> | ||
| </div> | ||
| <div className="flex items-start gap-1.5"> | ||
| <CheckIcon className="mt-1 shrink-0 text-[#7a7d84]" /> | ||
| <span className="line-clamp-2 text-sm leading-[22px] text-[#27272d]"> | ||
| {program.target} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* 비교함 담기 버튼 */} | ||
| <div className="mt-5 flex gap-1"> | ||
| {inCart ? ( | ||
| <> | ||
| <button | ||
| type="button" | ||
| onClick={() => toggleCartItem(challenge.programId)} | ||
| className="flex flex-1 items-center justify-center gap-1 rounded-lg bg-[#dbddfd] px-2 py-2.5" | ||
| > | ||
| <CheckIcon className="text-[#5f66f6]" /> | ||
| <span className="text-xs font-semibold leading-4 text-[#5f66f6]"> | ||
| 비교함 담기 | ||
| </span> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| removeFromCart(challenge.programId); | ||
| }} | ||
| className="flex w-[46px] items-center justify-center rounded-lg bg-[#e7e7e7]" | ||
| > | ||
| <CloseIcon /> | ||
| </button> | ||
| </> | ||
| ) : ( | ||
| <button | ||
| type="button" | ||
| onClick={() => toggleCartItem(challenge.programId)} | ||
| disabled={isFull} | ||
| className={`flex w-full items-center justify-center rounded-lg px-2 py-2.5 transition-colors ${ | ||
| isFull | ||
| ? 'cursor-not-allowed bg-[#e7e7e7] opacity-50' | ||
| : 'bg-[#e7e7e7] hover:bg-[#dbddfd] hover:text-[#5f66f6]' | ||
| }`} | ||
| > | ||
| <span className="text-xs font-semibold leading-4 text-[#5c5f66]"> | ||
| 비교함 담기 | ||
| </span> | ||
| </button> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
renderCard 함수는 컴포넌트 내부에서 정의되어 있으며 UI 로직이 상당히 깁니다. 가이드라인의 "Abstracting Implementation Details" 원칙에 따라, 이를 별도의 ChallengeCard 컴포넌트로 추출하여 ChallengeCompareSection의 인지 부하를 줄이고 유지보수성을 높이는 것을 권장합니다.
References
- 복잡한 로직이나 인터랙션은 전용 컴포넌트로 추상화하여 관심사를 분리해야 합니다. (link)
| const titleParts = programs.map((p) => { | ||
| if (p.title.includes('경험정리')) return '경험정리'; | ||
| if (p.title.includes('이력서')) return '이력서'; | ||
| if (p.title.includes('대기업')) return '대기업 자소서'; | ||
| if (p.title.includes('자기소개서')) return '자소서'; | ||
| if (p.title.includes('포트폴리오')) return '포트폴리오'; | ||
| if (p.title.includes('마케팅')) return '마케팅'; | ||
| if (p.title.includes('HR')) return 'HR'; | ||
| return p.title; | ||
| }); |
There was a problem hiding this comment.
프로그램 제목에서 핵심 키워드를 추출하는 로직이 반복적인 if 문으로 작성되어 있습니다. 매핑 객체를 사용하거나 별도의 유틸리티 함수로 분리하여 로직을 더 선언적이고 단순하게 개선할 수 있습니다.
const titleParts = programs.map((p) => {
const TITLE_MAP: Record<string, string> = {
'경험정리': '경험정리',
'이력서': '이력서',
'대기업': '대기업 자소서',
'자기소개서': '자소서',
'포트폴리오': '포트폴리오',
'마케팅': '마케팅',
'HR': 'HR',
};
const key = Object.keys(TITLE_MAP).find((k) => p.title.includes(k));
return key ? TITLE_MAP[key] : p.title;
});
| {/* FAQ 리스트 */} | ||
| <div className="flex w-[50rem] flex-col gap-3"> | ||
| {/* FAQ 리스트 — min-h로 카테고리 전환 시 높이 점핑 방지 */} | ||
| <div className="flex min-h-[600px] w-[50rem] flex-col gap-3"> |
There was a problem hiding this comment.
600px는 매직 넘버입니다. 가이드라인의 "Naming Magic Numbers" 규칙에 따라 의미 있는 이름의 상수로 정의하여 사용해 주세요.
References
- 매직 넘버를 의미 있는 이름을 가진 상수로 교체하여 코드의 명확성을 높여야 합니다. (link)
1 participant
연관 작업