Refactor(host): 주최 측 FestivalCard 로직 공통화 및 구조 개선#268
Draft
jin-evergreen wants to merge 6 commits intodevelopfrom
Draft
Refactor(host): 주최 측 FestivalCard 로직 공통화 및 구조 개선#268jin-evergreen wants to merge 6 commits intodevelopfrom
jin-evergreen wants to merge 6 commits intodevelopfrom
Conversation
📝 WalkthroughSummary by CodeRabbit릴리스 노트
🚀 Walkthrough축제 카드 표시 로직을 FestivalStatusGroup으로 통합하고, D-Day 포맷팅을 공유 유틸로 추출했으며, 라우트 상수를 중앙화하고 deprecated된 FestivalSection 컴포넌트를 제거한 리팩토링입니다. 📋 Changes
🎯 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 👥 Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/host/src/widgets/festival-history-list/festival-history-list.tsx`:
- Around line 16-23: The guard in handleCardClick incorrectly treats 0 as
missing; change the check to a nullish check (e.g., test festivalId == null or
festivalId === null || festivalId === undefined) or make the parameter
non-optional by removing the ? from festivalId in the handleCardClick signature
so numeric IDs like 0 are allowed; update any callers if you choose the
non-optional route and keep the rest of the navigation logic (generatePath with
ROUTE_PATH.NOTICE_LIST and eventId) unchanged.
- Around line 35-40: The CardFestival items use role='button' and tabIndex={0}
but lack keyboard handlers; add an onKeyDown handler on the CardFestival element
that listens for Enter (e.g., key === 'Enter') and Space (e.g., key === ' ' or
'Spacebar') and calls handleCardClick(festivalId) when pressed, making sure to
preventDefault for Space to avoid page scroll; implement the handler inline or
as a small helper (e.g., onCardKeyDown) and keep the existing onClick behavior
unchanged.
In `@apps/host/src/widgets/home/festival-card/festival-card.tsx`:
- Around line 28-31: The onKeyDown handler in festival-card.tsx currently
triggers onCardClick(festivalId) for Enter and Space but doesn't prevent the
default browser action for Space, causing page scroll; update the onKeyDown
callback (the KeyboardEvent handler attached to the element with role='button')
to call event.preventDefault() when event.key === ' ' (or event.code ===
'Space') before invoking onCardClick(festivalId) so the Space keypress does not
trigger default scrolling while keeping Enter behavior intact.
In `@apps/host/src/widgets/home/festival-overview/festival-overview.css.ts`:
- Around line 9-27: Rename generic wrapper style identifiers to more descriptive
container names: change the exported style symbols section, list, and item to
something like sectionContainer, festivalListContainer, and
festivalItemContainer (or names matching their semantic role) both where they
are defined in this file (festival-overview.css.ts) and where they are
imported/used in the component code; update all imports/uses to the new
identifiers to preserve functionality and follow the "container" naming
convention.
- Around line 9-23: Reorder the CSS property declarations in the style objects
for section and list to follow the Mozilla CSS property order (Display & Layout
→ Box Model → Visual → Typography → Content): move display, flexDirection,
alignItems/alignSelf, and gap before width so layout properties come first, then
box-model/size properties; update the export const section and export const list
objects accordingly to place display/flex-related keys before width.
In `@apps/host/src/widgets/home/festival-overview/festival-overview.tsx`:
- Around line 59-77: Replace the non-semantic div wrappers used in the
sections.map render with semantic elements: change the outer container for each
mapped item to a <section> (keeping className={styles.section} and StatusChip
usage) and change the festival list wrapper to a <ul> (keeping
className={styles.list}), rendering each FestivalCard inside an <li> (move the
key={festival.festivalId} to the <li>). Ensure the EmptyCard branch remains
unchanged, and keep existing handlers (handleOpenOptionSheet, handleCardClick)
passed to FestivalCard; update only the HTML tags and key placement so markup is
semantic and accessible.
- Around line 52-55: The navigate call inside the onEdit arrow currently
navigates to the literal path `/events/:eventId/notices/${noticeId}/edit`;
change it to build the path using the actual event id variable (e.g., eventId or
event.id) so both eventId and noticeId are substituted at runtime (update the
onEdit handler that calls navigate and ensure the eventId variable is in scope
or passed into the component/handler).
In `@packages/compositions/src/festival-status-group/festival-status-group.tsx`:
- Around line 14-22: Replace the conditional logic in getChipStatus with a
constant mapping to improve extensibility: define a STATUS_MAP (e.g., a plain
object or Map) that maps Korean status strings like '진행 중' and '진행 완료' to
'current' and 'completed', then update getChipStatus to return STATUS_MAP[text]
?? 'upcoming'; this centralizes mappings and makes adding new statuses (and
tests) simpler while keeping the function body minimal.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
apps/host/src/pages/home/home.tsxapps/host/src/pages/my-history/my-history.tsxapps/host/src/pages/notice-list/notice-list.tsxapps/host/src/shared/libs/format-dday.tsapps/host/src/widgets/festival-history-list/festival-history-list.tsxapps/host/src/widgets/home/festival-card/festival-card.css.tsapps/host/src/widgets/home/festival-card/festival-card.tsxapps/host/src/widgets/home/festival-overview/festival-overview.css.tsapps/host/src/widgets/home/festival-overview/festival-overview.tsxapps/host/src/widgets/home/festival-section/festival-section.css.tsapps/host/src/widgets/home/festival-section/festival-section.tsxapps/host/src/widgets/my-history/festival-list.tsxpackages/ads-ui/src/components/chip/chip.css.tspackages/compositions/src/festival-status-group/festival-status-group.tsxpackages/compositions/src/index.ts
💤 Files with no reviewable changes (4)
- apps/host/src/widgets/home/festival-section/festival-section.css.ts
- apps/host/src/widgets/home/festival-section/festival-section.tsx
- apps/host/src/widgets/my-history/festival-list.tsx
- apps/host/src/widgets/home/festival-card/festival-card.css.ts
Comment on lines
+16
to
+23
| const handleCardClick = (festivalId?: number) => { | ||
| if (!festivalId) { | ||
| return; | ||
| } | ||
| navigate( | ||
| generatePath(ROUTE_PATH.NOTICE_LIST, { | ||
| eventId: String(festivalId), | ||
| }), |
There was a problem hiding this comment.
Suggest: festivalId 체크를 nullish 기준으로 바꿔 의도를 명확히 해주세요.
Line 17의 if (!festivalId)는 0도 차단합니다. 타입이 number 기반이면 festivalId == null 체크로 좁히거나, 아예 파라미터를 non-optional로 두는 편이 안전합니다.
안전한 조건 예시
- const handleCardClick = (festivalId?: number) => {
- if (!festivalId) {
+ const handleCardClick = (festivalId?: number) => {
+ if (festivalId == null) {
return;
}📝 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.
Suggested change
| const handleCardClick = (festivalId?: number) => { | |
| if (!festivalId) { | |
| return; | |
| } | |
| navigate( | |
| generatePath(ROUTE_PATH.NOTICE_LIST, { | |
| eventId: String(festivalId), | |
| }), | |
| const handleCardClick = (festivalId?: number) => { | |
| if (festivalId == null) { | |
| return; | |
| } | |
| navigate( | |
| generatePath(ROUTE_PATH.NOTICE_LIST, { | |
| eventId: String(festivalId), | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/host/src/widgets/festival-history-list/festival-history-list.tsx` around
lines 16 - 23, The guard in handleCardClick incorrectly treats 0 as missing;
change the check to a nullish check (e.g., test festivalId == null or festivalId
=== null || festivalId === undefined) or make the parameter non-optional by
removing the ? from festivalId in the handleCardClick signature so numeric IDs
like 0 are allowed; update any callers if you choose the non-optional route and
keep the rest of the navigation logic (generatePath with ROUTE_PATH.NOTICE_LIST
and eventId) unchanged.
Comment on lines
+35
to
+40
| <CardFestival | ||
| key={key} | ||
| role='button' | ||
| tabIndex={0} | ||
| onClick={() => handleCardClick(festivalId)} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 대상 파일 확인
cat -n apps/host/src/widgets/festival-history-list/festival-history-list.tsx | head -50Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 1799
🏁 Script executed:
# CardFestival 컴포넌트 찾기
find apps -name "*.tsx" -o -name "*.ts" | xargs grep -l "CardFestival" | head -10Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 333
🏁 Script executed:
# CardFestival 컴포넌트 정의 찾기
rg "export.*CardFestival|function CardFestival|const CardFestival" -A 10Repository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 1910
🏁 Script executed:
cat -n packages/ads-ui/src/components/card/card-festival/card-festival.tsxRepository: TEAM-DECIBEL/AMP-CLIENT
Length of output: 1861
Must: 카드 요소에 키보드 접근성 처리(Enter/Space 키)를 추가해 주세요.
Line 35-40에서 role='button' + tabIndex={0}를 지정하면 WCAG 2.1 접근성 표준상 해당 요소가 버튼처럼 동작해야 합니다. 현재 코드는 onKeyDown 핸들러가 없어서 Enter나 Space 키로 작동하지 않아 키보드 사용자가 상호작용할 수 없습니다.
수정 예시
<CardFestival
key={key}
role='button'
tabIndex={0}
onClick={() => handleCardClick(festivalId)}
+ onKeyDown={(event) => {
+ if (event.key === 'Enter' || event.key === ' ') {
+ event.preventDefault();
+ handleCardClick(festivalId);
+ }
+ }}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/host/src/widgets/festival-history-list/festival-history-list.tsx` around
lines 35 - 40, The CardFestival items use role='button' and tabIndex={0} but
lack keyboard handlers; add an onKeyDown handler on the CardFestival element
that listens for Enter (e.g., key === 'Enter') and Space (e.g., key === ' ' or
'Spacebar') and calls handleCardClick(festivalId) when pressed, making sure to
preventDefault for Space to avoid page scroll; implement the handler inline or
as a small helper (e.g., onCardKeyDown) and keep the existing onClick behavior
unchanged.
Comment on lines
28
to
31
| onKeyDown={(event: KeyboardEvent<HTMLElement>) => { | ||
| if (event.key === 'Enter' || event.key === ' ') { | ||
| onCardClick(festival.festivalId); | ||
| onCardClick(festivalId); | ||
| } |
There was a problem hiding this comment.
Must: 키보드 Space 입력 시 기본 동작을 막아 접근성 동작을 안정화해 주세요.
Line 29~30에서 Space 키를 처리할 때 event.preventDefault()가 없어 페이지 스크롤이 발생할 수 있습니다. role='button' 상호작용에서는 기본 동작 차단이 필요합니다.
수정 예시
onKeyDown={(event: KeyboardEvent<HTMLElement>) => {
- if (event.key === 'Enter' || event.key === ' ') {
+ if (event.key === 'Enter' || event.key === ' ') {
+ if (event.key === ' ') {
+ event.preventDefault();
+ }
onCardClick(festivalId);
}
}}📝 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.
Suggested change
| onKeyDown={(event: KeyboardEvent<HTMLElement>) => { | |
| if (event.key === 'Enter' || event.key === ' ') { | |
| onCardClick(festival.festivalId); | |
| onCardClick(festivalId); | |
| } | |
| onKeyDown={(event: KeyboardEvent<HTMLElement>) => { | |
| if (event.key === 'Enter' || event.key === ' ') { | |
| if (event.key === ' ') { | |
| event.preventDefault(); | |
| } | |
| onCardClick(festivalId); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/host/src/widgets/home/festival-card/festival-card.tsx` around lines 28 -
31, The onKeyDown handler in festival-card.tsx currently triggers
onCardClick(festivalId) for Enter and Space but doesn't prevent the default
browser action for Space, causing page scroll; update the onKeyDown callback
(the KeyboardEvent handler attached to the element with role='button') to call
event.preventDefault() when event.key === ' ' (or event.code === 'Space') before
invoking onCardClick(festivalId) so the Space keypress does not trigger default
scrolling while keeping Enter behavior intact.
Comment on lines
+9
to
+27
| export const section = style({ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| alignItems: 'stretch', | ||
| gap: '1.4rem', | ||
| width: '100%', | ||
| }); | ||
|
|
||
| export const list = style({ | ||
| width: '100%', | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: '1rem', | ||
| alignSelf: 'stretch', | ||
| }); | ||
|
|
||
| export const item = style({ | ||
| width: '100%', | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: 래퍼 스타일 이름을 더 의미 있는 *Container 형태로 정리해 주세요.
Line 9, Line 17, Line 25의 section/list/item은 범용성이 높아 의도를 파악하기 어렵습니다. sectionContainer, festivalListContainer처럼 역할이 드러나는 이름이 유지보수에 유리합니다.
As per coding guidelines "Use 'container' as the naming convention for wrapper elements in CSS/HTML" and "Use meaningful style names that reveal element semantics (e.g., user-list-container)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/host/src/widgets/home/festival-overview/festival-overview.css.ts` around
lines 9 - 27, Rename generic wrapper style identifiers to more descriptive
container names: change the exported style symbols section, list, and item to
something like sectionContainer, festivalListContainer, and
festivalItemContainer (or names matching their semantic role) both where they
are defined in this file (festival-overview.css.ts) and where they are
imported/used in the component code; update all imports/uses to the new
identifiers to preserve functionality and follow the "container" naming
convention.
Comment on lines
+9
to
+23
| export const section = style({ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| alignItems: 'stretch', | ||
| gap: '1.4rem', | ||
| width: '100%', | ||
| }); | ||
|
|
||
| export const list = style({ | ||
| width: '100%', | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: '1rem', | ||
| alignSelf: 'stretch', | ||
| }); |
There was a problem hiding this comment.
Must: CSS 속성 순서를 컨벤션 순서로 맞춰주세요.
Line 1415, Line 1821에서 width와 display/flex 배치 순서가 섞여 있습니다. 스타일 충돌을 줄이려면 규칙 순서로 정렬해 주세요.
정렬 예시 패치
export const section = style({
display: 'flex',
flexDirection: 'column',
alignItems: 'stretch',
gap: '1.4rem',
width: '100%',
});
export const list = style({
- width: '100%',
display: 'flex',
flexDirection: 'column',
gap: '1rem',
alignSelf: 'stretch',
+ width: '100%',
});As per coding guidelines "Follow Mozilla Style CSS property order: Display & Layout → Box Model → Visual → Typography → Content".
📝 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.
Suggested change
| export const section = style({ | |
| display: 'flex', | |
| flexDirection: 'column', | |
| alignItems: 'stretch', | |
| gap: '1.4rem', | |
| width: '100%', | |
| }); | |
| export const list = style({ | |
| width: '100%', | |
| display: 'flex', | |
| flexDirection: 'column', | |
| gap: '1rem', | |
| alignSelf: 'stretch', | |
| }); | |
| export const section = style({ | |
| display: 'flex', | |
| flexDirection: 'column', | |
| alignItems: 'stretch', | |
| gap: '1.4rem', | |
| width: '100%', | |
| }); | |
| export const list = style({ | |
| display: 'flex', | |
| flexDirection: 'column', | |
| gap: '1rem', | |
| alignSelf: 'stretch', | |
| width: '100%', | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/host/src/widgets/home/festival-overview/festival-overview.css.ts` around
lines 9 - 23, Reorder the CSS property declarations in the style objects for
section and list to follow the Mozilla CSS property order (Display & Layout →
Box Model → Visual → Typography → Content): move display, flexDirection,
alignItems/alignSelf, and gap before width so layout properties come first, then
box-model/size properties; update the export const section and export const list
objects accordingly to place display/flex-related keys before width.
Comment on lines
52
to
55
| onEdit={(noticeId) => | ||
| // TODO : 공연 수정 뷰로 변경 | ||
| navigate(`/events/:eventId/notices/${noticeId}/edit`) | ||
| } |
There was a problem hiding this comment.
Must: 수정 페이지 이동 경로에서 eventId가 실제 값으로 치환되지 않습니다.
Line 54는 :eventId를 리터럴로 navigate하고 있어서, 수정 화면 진입이 실패할 가능성이 높습니다. eventId와 noticeId를 모두 실제 값으로 치환해 경로를 생성해 주세요.
치환 방식 예시
- onEdit={(noticeId) =>
- // TODO : 공연 수정 뷰로 변경
- navigate(`/events/:eventId/notices/${noticeId}/edit`)
- }
+ onEdit={(noticeId, eventId) => {
+ if (eventId == null) return;
+ navigate(
+ generatePath('/events/:eventId/notices/:noticeId/edit', {
+ eventId: String(eventId),
+ noticeId: String(noticeId),
+ }),
+ );
+ }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/host/src/widgets/home/festival-overview/festival-overview.tsx` around
lines 52 - 55, The navigate call inside the onEdit arrow currently navigates to
the literal path `/events/:eventId/notices/${noticeId}/edit`; change it to build
the path using the actual event id variable (e.g., eventId or event.id) so both
eventId and noticeId are substituted at runtime (update the onEdit handler that
calls navigate and ensure the eventId variable is in scope or passed into the
component/handler).
Comment on lines
+59
to
+77
| {sections.map(({ title, count, festivals, emptyText }) => ( | ||
| <div key={title} className={styles.section}> | ||
| <StatusChip title={title} count={count} /> | ||
|
|
||
| {festivals.length === 0 ? ( | ||
| <EmptyCard>{emptyText}</EmptyCard> | ||
| ) : ( | ||
| <div className={styles.list}> | ||
| {festivals.map((festival) => ( | ||
| <FestivalCard | ||
| key={festival.festivalId} | ||
| festival={festival} | ||
| onMoreClick={handleOpenOptionSheet} | ||
| onCardClick={handleCardClick} | ||
| /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: 섹션/목록 래퍼를 semantic 태그로 바꾸면 구조와 접근성이 더 좋아집니다.
현재는 div 중심 구조라 문서 의미가 약합니다. section + ul/li로 바꾸면 스크린리더 탐색성과 유지보수성이 함께 개선됩니다.
구조 개선 예시
- {sections.map(({ title, count, festivals, emptyText }) => (
- <div key={title} className={styles.section}>
+ {sections.map(({ title, count, festivals, emptyText }) => (
+ <section key={title} className={styles.section} aria-label={title}>
<StatusChip title={title} count={count} />
{festivals.length === 0 ? (
<EmptyCard>{emptyText}</EmptyCard>
) : (
- <div className={styles.list}>
+ <ul className={styles.list}>
{festivals.map((festival) => (
- <FestivalCard
- key={festival.festivalId}
- festival={festival}
- onMoreClick={handleOpenOptionSheet}
- onCardClick={handleCardClick}
- />
+ <li key={festival.festivalId}>
+ <FestivalCard
+ festival={festival}
+ onMoreClick={handleOpenOptionSheet}
+ onCardClick={handleCardClick}
+ />
+ </li>
))}
- </div>
+ </ul>
)}
- </div>
+ </section>
))}As per coding guidelines "Avoid using meaningless div tags; use semantic HTML or custom components instead".
📝 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.
Suggested change
| {sections.map(({ title, count, festivals, emptyText }) => ( | |
| <div key={title} className={styles.section}> | |
| <StatusChip title={title} count={count} /> | |
| {festivals.length === 0 ? ( | |
| <EmptyCard>{emptyText}</EmptyCard> | |
| ) : ( | |
| <div className={styles.list}> | |
| {festivals.map((festival) => ( | |
| <FestivalCard | |
| key={festival.festivalId} | |
| festival={festival} | |
| onMoreClick={handleOpenOptionSheet} | |
| onCardClick={handleCardClick} | |
| /> | |
| ))} | |
| </div> | |
| )} | |
| </div> | |
| {sections.map(({ title, count, festivals, emptyText }) => ( | |
| <section key={title} className={styles.section} aria-label={title}> | |
| <StatusChip title={title} count={count} /> | |
| {festivals.length === 0 ? ( | |
| <EmptyCard>{emptyText}</EmptyCard> | |
| ) : ( | |
| <ul className={styles.list}> | |
| {festivals.map((festival) => ( | |
| <li key={festival.festivalId}> | |
| <FestivalCard | |
| festival={festival} | |
| onMoreClick={handleOpenOptionSheet} | |
| onCardClick={handleCardClick} | |
| /> | |
| </li> | |
| ))} | |
| </ul> | |
| )} | |
| </section> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/host/src/widgets/home/festival-overview/festival-overview.tsx` around
lines 59 - 77, Replace the non-semantic div wrappers used in the sections.map
render with semantic elements: change the outer container for each mapped item
to a <section> (keeping className={styles.section} and StatusChip usage) and
change the festival list wrapper to a <ul> (keeping className={styles.list}),
rendering each FestivalCard inside an <li> (move the key={festival.festivalId}
to the <li>). Ensure the EmptyCard branch remains unchanged, and keep existing
handlers (handleOpenOptionSheet, handleCardClick) passed to FestivalCard; update
only the HTML tags and key placement so markup is semantic and accessible.
Comment on lines
+14
to
+22
| const getChipStatus = (text: string) => { | ||
| if (text === '진행 중') { | ||
| return 'current'; | ||
| } | ||
| if (text === '진행 완료') { | ||
| return 'completed'; | ||
| } | ||
| return 'upcoming'; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: 상태 텍스트 매핑을 상수 맵으로 분리해 확장성을 높여주세요.
Line 14~22의 조건문은 현재는 간단하지만 상태가 늘어나면 분기 유지비가 커집니다. 상수 맵으로 분리하면 수정 포인트를 줄일 수 있습니다.
리팩터링 예시
+const STATUS_BY_TEXT = {
+ '진행 중': 'current',
+ '진행 완료': 'completed',
+} as const;
+
+const getChipStatus = (text: string) => STATUS_BY_TEXT[text as keyof typeof STATUS_BY_TEXT] ?? 'upcoming';
+
const FestivalStatusGroup = ({
dDay,
statusText,
isWishlist,
}: FestivalStatusGroupProps) => {
- const getChipStatus = (text: string) => {
- if (text === '진행 중') {
- return 'current';
- }
- if (text === '진행 완료') {
- return 'completed';
- }
- return 'upcoming';
- };
return (📝 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.
Suggested change
| const getChipStatus = (text: string) => { | |
| if (text === '진행 중') { | |
| return 'current'; | |
| } | |
| if (text === '진행 완료') { | |
| return 'completed'; | |
| } | |
| return 'upcoming'; | |
| }; | |
| const STATUS_BY_TEXT = { | |
| '진행 중': 'current', | |
| '진행 완료': 'completed', | |
| } as const; | |
| const getChipStatus = (text: string) => STATUS_BY_TEXT[text as keyof typeof STATUS_BY_TEXT] ?? 'upcoming'; | |
| const FestivalStatusGroup = ({ | |
| dDay, | |
| statusText, | |
| isWishlist, | |
| }: FestivalStatusGroupProps) => { | |
| return ( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/compositions/src/festival-status-group/festival-status-group.tsx`
around lines 14 - 22, Replace the conditional logic in getChipStatus with a
constant mapping to improve extensibility: define a STATUS_MAP (e.g., a plain
object or Map) that maps Korean status strings like '진행 중' and '진행 완료' to
'current' and 'completed', then update getChipStatus to return STATUS_MAP[text]
?? 'upcoming'; this centralizes mappings and makes adding new statuses (and
tests) simpler while keeping the function body minimal.
|
좋습니다 아주잘되고있네요 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Summary
주최 측 위젯인
FestivalCard컴포넌트 내 칩 관련 로직과 같은 공통되는 로직을 통합하여 UI 일관성을 개선하고, 불필요한 컴포넌트 계층 구조를 단순화하여 Props Drilling을 제거했습니다.📚 Tasks
FestivalCard내부 함수를 제거하고, 공통 로직으로 대체FestivalList컴포넌트명을FestivalHistoryList로 더 명확히 변경하고, 내부 구조 개선🔍 Describe
리팩토링 배경
주최 측 위젯
FestivalCard내부에 날짜를 계산하고 칩 UI를 분기 처리하는 로직이 들어있었는데, 이는 관객 측 서비스와 중복되는 로직이었어요. 따라서 지난 번 관객 측 리팩토링 과정에서 분리해둔FestivalStatusGroup을 활용하여 이를 대체하는 작업을 진행했어요.추가로 기존 주최 측 홈 화면에서
FestivalCard를 렌더링하기 위해서는,FestivalOverview→FestivalSection→FestivalList→FestivalCard로 이어지는 깊은 계층을 가지고 있었어요. 이로 인해 단순한 핸들러 함수를 전달하기 위해서 Props Drilling이 발생하여 유지보수하기 어려운 구조였어요.주요 리팩토링 과정
컴포넌트 계층 단순화
기존 4단계로 나누어져 있던 계층을
FestivalOverview->FestivalCard로 압축하는 작업을 진행했어요.기존에는 아래와 같이 역할이 파편화되어 있었어요.
FestivalOverview: 전체 섹션 데이터(진행 중, 진행 예정) 배열을 정의하는 단순 래퍼 역할FestivalSection: 각 섹션의 헤더를 렌더링하고, 데이터 유무에 따라 분기처리FestivalList: 배열을 단순히 순회FestivalCard: 카드 UI 렌더링이번 리팩토링은 기존에 가장 실질적인 역할을 하던
FestivalSection의 로직을FestivalOverview로 통합하여 응집도를 높였어요.기존에는 섹션 헤더 렌더링과 데이터 유무에 따른 분기 처리가
FestivalSection에 파편화되어 있었고, 그 앞뒤로 Overview와 List가 단순 래퍼 역할만 수행하고 있어요.이에 따라 불필요한 계층을 제거하고,
FestivalOverview가 섹션의 주요 로직을 직접 수행하도록 구조를 압축했어요. 또한, 상위 페이지에서 전달받던onCardClick을 직접 정의해FestivalCard로 바로 전달하여 Props Drilling을 해결했습니다.👀 To Reviewer
구조적으로 괜찮은지, 개선할 점은 없는지 확인해주세요!
이번 PR에서 몇 가지 작은 수정사항들도 함께 작업했어요.
border가 보이지 않는 문제D-7이D+7로 잘못 렌더링되는 문제Chip컴포넌트 디자인 관련 문제를 해결했어요.이 부분도 함께 확인해주시면 감사하겠습니다🙇♂️
📸 Screenshot
2026-02-25.11.32.37.mov