[refactor] 동아리 목록, 상세 및 채팅 페이지 리디자인 반영#189
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough라우팅 선언을 일부 재구성해 Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/Club/ClubList/components/SearchBar.tsx (1)
7-12:⚠️ Potential issue | 🟠 MajorProps 타입을 판별 유니온으로 분리하고, input에 접근성 속성을 추가해주세요.
현재 타입이면
isButton과 입력 props를 함께 넘겨도 허용되어<Link>안에<input>이 들어가는 조합을 타입이 막아주지 못합니다. 또한 input 요소에aria-label이 없어 접근성을 만족하지 않습니다.-interface SearchBarProps { - isButton?: boolean; - value?: string; - onChange?: (value: string) => void; - onSubmit?: (e: FormEvent<HTMLFormElement>) => void; - autoFocus?: boolean; -} +type SearchBarProps = + | { + isButton: true; + value?: never; + onChange?: never; + onSubmit?: never; + autoFocus?: never; + } + | { + isButton?: false; + value?: string; + onChange?: (value: string) => void; + onSubmit?: (e: FormEvent<HTMLFormElement>) => void; + autoFocus?: boolean; + };input 요소에도
aria-label="동아리 이름으로 검색"추가하세요.Also applies to: 22-23, 25-31, 36-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Club/ClubList/components/SearchBar.tsx` around lines 7 - 12, The SearchBarProps currently allows invalid combinations (e.g., isButton with input callbacks) and missing accessibility on the input; change SearchBarProps into a discriminated union (e.g., { isButton: true; onSubmit: ... } | { isButton?: false; value?: string; onChange?: (v:string)=>void; autoFocus?: boolean }) so TypeScript prevents passing input props when isButton is true, update the SearchBar component to use this union (references: SearchBarProps, isButton, onChange, onSubmit) and add aria-label="동아리 이름으로 검색" to the input element; apply the same discriminated-union + aria-label fix to the other similar components/instances noted (lines 22-23, 25-31, 36-46).src/pages/Club/ClubDetail/index.tsx (1)
84-104:⚠️ Potential issue | 🟠 Major현재 탭만 조건부 렌더링하도록 리팩토링이 필요합니다.
모든 탭이 조건 검증 후
<Activity>내에서 렌더링되고 있습니다.mode="hidden"이더라도 자식 컴포넌트가 마운트되면서useGetClubRecruitment(ClubRecruitment.tsx L18),useGetClubMembers(ClubMember.tsx L60) 등 데이터 훅이 즉시 실행됩니다. 결과적으로 사용자가 보는 탭과 관계없이 모든 활성 탭의 요청이 함께 발생합니다. 상태 보존이 필수가 아니라면 현재 탭만 렌더링하는 방식으로 초기 로드 성능을 개선할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Club/ClubDetail/index.tsx` around lines 84 - 104, The current code mounts all tab child components inside <Activity> even when mode="hidden", causing hooks like useGetClubRecruitment and useGetClubMembers to run; change rendering so only the active tab's <Activity> and its child are mounted. Concretely, for each tab (symbols: Activity, ClubRecruit, ClubIntro, ClubMemberTab, ClubAccount) move the currentTab checks outward so you render the Activity+child only when currentTab matches (e.g., only render ClubRecruit when currentTab === 'recruitment' and clubDetail.recruitment.status !== 'CLOSED'; only render ClubMemberTab when currentTab === 'members' and clubDetail.isMember; only render ClubAccount when currentTab === 'account' and (clubDetail.isMember || clubDetail.isApplied'); keep ClubIntro rendered only when currentTab === 'intro'). This prevents mounting children and stops their hooks (useGetClubRecruitment, useGetClubMembers) from executing for inactive tabs.
🧹 Nitpick comments (2)
src/pages/Chat/index.tsx (1)
58-84: 목록 아이템 텍스트는 typography token으로 맞춰 주세요.이 구간에
text-[16px],text-[12px],leading-[1.6]가 반복돼서 채팅 목록만 theme scale에서 벗어납니다.text-sub*/text-body*/text-cap*조합으로 맞추는 편이 유지보수에 좋습니다.As per coding guidelines, "Use typography tokens (
text-h1throughtext-h5,text-sub1throughtext-sub4,text-body1throughtext-body3,text-cap1throughtext-cap2) fromsrc/styles/theme.css".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Chat/index.tsx` around lines 58 - 84, The chat list uses hardcoded utility font classes (e.g., text-[16px], text-[12px], leading-[1.6]) on elements rendering room.roomName, formatTime(room.lastSentAt), previewMessage and the unread badge which breaks the theme scale; replace those with the appropriate typography tokens from src/styles/theme.css (for example text-sub*/text-body*/text-cap* combinations) on the span containing room.roomName, the timestamp span that calls formatTime, the previewMessage <p>, and any small badge text so all items use the theme tokens instead of explicit sizes while preserving existing class names like BellOffIcon and hasUnreadMessage logic.src/pages/Club/ClubDetail/components/ClubMember.tsx (1)
1-1: 배지 스타일은cn()과 테마 토큰으로 맞춰주세요.지금은
clsx직접 사용과 hex 색상 클래스가 같이 들어와 있어서, 테마 변경 시 이 컴포넌트만 따로 수정하게 됩니다.POSITION_BADGE_STYLES를theme.css기반 토큰으로 바꾸고 클래스 병합도cn()으로 통일하는 편이 좋겠습니다.As per coding guidelines
Use cn() utility from src/utils/ts/cn.ts to merge Tailwind CSS classesandPrioritize color tokens from src/styles/theme.css.Also applies to: 13-17, 41-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Club/ClubDetail/components/ClubMember.tsx` at line 1, The component is using clsx and hardcoded hex color classes; replace the clsx import with the cn() utility and refactor POSITION_BADGE_STYLES to use theme CSS tokens (from theme.css) instead of hex classes, then update all places where clsx is used (including the badge render paths referenced by POSITION_BADGE_STYLES and the occurrences around lines 13-17 and 41-45) to call cn(...) with the new token-based class names; ensure you import cn() from src/utils/ts/cn.ts and keep the same class-merge semantics so the badge styles follow theme tokens and use cn() consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Around line 103-105: Update the routing so the clubs/:clubId route uses a
dedicated Layout instance with the page-specific props instead of the shared
Layout: change the Route for path "clubs/:clubId" to render <Layout
contentClassName="bg-indigo-5" showBottomNav={false}> wrapping <ClubDetail />
(or otherwise pass those props into Layout when rendering ClubDetail), leaving
"clubs/:clubId/applications" to use the standard Layout with its existing props;
ensure you reference the Layout component and the ClubDetail element so the
top/overscroll background and header padding come from the Layout's main area
and match the redesign.
In `@src/components/layout/Header/components/ChatHeader.tsx`:
- Around line 33-37: 현재 ChatHeader 컴포넌트에서 사이드바 여는 버튼을 isGroup으로 감싸면 1:1 다이렉트
채팅에서 사이드바(알림 토글)에 접근할 수 없어 집니다; 수정하려면 ChatHeader 내에서 onClick 핸들러 openSidebar와 버튼
렌더링(HamburgerIcon 포함)은 항상 렌더되도록 변경하고, 그룹 전용으로 보여야 할 참가자 목록/섹션(참고: 기존 diff의 사이드바
첫 섹션, lines 54-70에 해당하는 UI)은 계속해서 isGroup 조건으로만 감싸 유지하세요; 즉, 제거 대상은 버튼을 감싸는
isGroup 조건이고, 유지할 조건은 참가자 목록 렌더링 부분입니다.
In `@src/pages/Chat/ChatRoom.tsx`:
- Around line 156-159: The message list container (the div with ref
scrollContainerRef in ChatRoom.tsx) needs to be exposed as a live region so
assistive tech announces new messages; update that div to include role="log" and
provide an accessible name (using aria-label or aria-labelledby, e.g.,
aria-label="Chat messages") and consider adding aria-atomic/aria-live attributes
as needed to ensure new message announcements; ensure the change is applied to
the same element that receives appended messages so screen readers detect
additions.
In `@src/pages/Club/ClubList/components/SearchBar.tsx`:
- Around line 25-31: The search input in SearchBar.tsx lacks an accessible name;
update the <input> in the SearchBar component to include an accessible name
(preferably add an aria-label attribute or wire a visible <label> to it).
Specifically, either add aria-label={ariaLabel ?? SEARCH_PLACEHOLDER} and accept
an optional ariaLabel prop on the SearchBar, or add a <label
htmlFor="club-search"> tied to input id="club-search"; ensure the new prop or
id/label change is used where value and onChange are passed so screen readers
can announce the field.
---
Outside diff comments:
In `@src/pages/Club/ClubDetail/index.tsx`:
- Around line 84-104: The current code mounts all tab child components inside
<Activity> even when mode="hidden", causing hooks like useGetClubRecruitment and
useGetClubMembers to run; change rendering so only the active tab's <Activity>
and its child are mounted. Concretely, for each tab (symbols: Activity,
ClubRecruit, ClubIntro, ClubMemberTab, ClubAccount) move the currentTab checks
outward so you render the Activity+child only when currentTab matches (e.g.,
only render ClubRecruit when currentTab === 'recruitment' and
clubDetail.recruitment.status !== 'CLOSED'; only render ClubMemberTab when
currentTab === 'members' and clubDetail.isMember; only render ClubAccount when
currentTab === 'account' and (clubDetail.isMember || clubDetail.isApplied');
keep ClubIntro rendered only when currentTab === 'intro'). This prevents
mounting children and stops their hooks (useGetClubRecruitment,
useGetClubMembers) from executing for inactive tabs.
In `@src/pages/Club/ClubList/components/SearchBar.tsx`:
- Around line 7-12: The SearchBarProps currently allows invalid combinations
(e.g., isButton with input callbacks) and missing accessibility on the input;
change SearchBarProps into a discriminated union (e.g., { isButton: true;
onSubmit: ... } | { isButton?: false; value?: string; onChange?:
(v:string)=>void; autoFocus?: boolean }) so TypeScript prevents passing input
props when isButton is true, update the SearchBar component to use this union
(references: SearchBarProps, isButton, onChange, onSubmit) and add
aria-label="동아리 이름으로 검색" to the input element; apply the same
discriminated-union + aria-label fix to the other similar components/instances
noted (lines 22-23, 25-31, 36-46).
---
Nitpick comments:
In `@src/pages/Chat/index.tsx`:
- Around line 58-84: The chat list uses hardcoded utility font classes (e.g.,
text-[16px], text-[12px], leading-[1.6]) on elements rendering room.roomName,
formatTime(room.lastSentAt), previewMessage and the unread badge which breaks
the theme scale; replace those with the appropriate typography tokens from
src/styles/theme.css (for example text-sub*/text-body*/text-cap* combinations)
on the span containing room.roomName, the timestamp span that calls formatTime,
the previewMessage <p>, and any small badge text so all items use the theme
tokens instead of explicit sizes while preserving existing class names like
BellOffIcon and hasUnreadMessage logic.
In `@src/pages/Club/ClubDetail/components/ClubMember.tsx`:
- Line 1: The component is using clsx and hardcoded hex color classes; replace
the clsx import with the cn() utility and refactor POSITION_BADGE_STYLES to use
theme CSS tokens (from theme.css) instead of hex classes, then update all places
where clsx is used (including the badge render paths referenced by
POSITION_BADGE_STYLES and the occurrences around lines 13-17 and 41-45) to call
cn(...) with the new token-based class names; ensure you import cn() from
src/utils/ts/cn.ts and keep the same class-merge semantics so the badge styles
follow theme tokens and use cn() consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 228a5e57-85d0-43d0-8fd5-009ac7f8a92e
⛔ Files ignored due to path filters (1)
src/assets/svg/chat-send-arrow.svgis excluded by!**/*.svg,!src/assets/**and included by**
📒 Files selected for processing (15)
src/App.tsxsrc/components/layout/Header/components/ChatHeader.tsxsrc/components/layout/Header/headerConfig.tssrc/components/layout/Header/routeTitles.tssrc/pages/Chat/ChatRoom.tsxsrc/pages/Chat/index.tsxsrc/pages/Club/Application/components/AccountInfo.tsxsrc/pages/Club/ClubDetail/components/ClubIntro.tsxsrc/pages/Club/ClubDetail/components/ClubMember.tsxsrc/pages/Club/ClubDetail/components/ClubRecruitment.tsxsrc/pages/Club/ClubDetail/index.tsxsrc/pages/Club/ClubList/components/ClubCard.tsxsrc/pages/Club/ClubList/components/SearchBar.tsxsrc/pages/Club/ClubList/index.tsxsrc/pages/Club/ClubSearch/index.tsx
💤 Files with no reviewable changes (1)
- src/components/layout/Header/routeTitles.ts
Comment on lines
103
to
105
| <Route element={<Layout />}> | ||
| <Route path="clubs/:clubId" element={<ClubDetail />} /> | ||
| <Route path="clubs/:clubId/applications" element={<ApplicationPage />} /> |
There was a problem hiding this comment.
ClubDetail는 전용 Layout 배경을 분리해주세요.
ClubDetail는 src/pages/Club/ClubDetail/index.tsx에서 루트 배경을 bg-indigo-5로 두고 있지만, 실제 페이지 배경과 헤더 padding 영역은 Layout의 <main>이 담당합니다. 지금처럼 공용 <Layout /> 아래에 두면 상세 상단/overscroll 구간은 기본 배경색이 남아서 리디자인과 다른 띠가 보일 수 있습니다. clubs/:clubId만 contentClassName="bg-indigo-5"를 주는 Layout으로 분리하는 편이 안전합니다.
💡 제안
- <Route element={<Layout />}>
- <Route path="clubs/:clubId" element={<ClubDetail />} />
+ <Route element={<Layout contentClassName="bg-indigo-5" />}>
+ <Route path="clubs/:clubId" element={<ClubDetail />} />
+ </Route>
+ <Route element={<Layout />}>
<Route path="clubs/:clubId/applications" element={<ApplicationPage />} />As per coding guidelines src/pages/**/*.tsx: Pass showBottomNav (bottom tab display) and contentClassName (background color, etc.) props to Layout component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.tsx` around lines 103 - 105, Update the routing so the clubs/:clubId
route uses a dedicated Layout instance with the page-specific props instead of
the shared Layout: change the Route for path "clubs/:clubId" to render <Layout
contentClassName="bg-indigo-5" showBottomNav={false}> wrapping <ClubDetail />
(or otherwise pass those props into Layout when rendering ClubDetail), leaving
"clubs/:clubId/applications" to use the standard Layout with its existing props;
ensure you reference the Layout component and the ClubDetail element so the
top/overscroll background and header padding come from the Layout's main area
and match the redesign.
Comment on lines
156
to
159
| <div | ||
| ref={scrollContainerRef} | ||
| className="bg-indigo-0 min-h-0 flex-1 overflow-x-hidden overflow-y-auto overscroll-contain pb-4" | ||
| className="min-h-0 flex-1 overflow-x-hidden overflow-y-auto overscroll-contain py-3" | ||
| > |
There was a problem hiding this comment.
메시지 목록을 live region으로 노출해주세요.
현재 컨테이너는 동적으로 추가되는 메시지를 보조기기가 채팅 로그로 인식할 근거가 없습니다. 채팅처럼 순차적으로 항목이 추가되는 영역은 role="log"와 접근 가능한 이름을 두는 패턴이어서, 지금처럼 포커스가 입력창에 머무는 화면에서는 새 메시지 안내가 누락될 수 있습니다. (developer.mozilla.org)
💡 제안
<div
ref={scrollContainerRef}
+ role="log"
+ aria-label="채팅 메시지"
className="min-h-0 flex-1 overflow-x-hidden overflow-y-auto overscroll-contain py-3"
>📝 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
| <div | |
| ref={scrollContainerRef} | |
| className="bg-indigo-0 min-h-0 flex-1 overflow-x-hidden overflow-y-auto overscroll-contain pb-4" | |
| className="min-h-0 flex-1 overflow-x-hidden overflow-y-auto overscroll-contain py-3" | |
| > | |
| <div | |
| ref={scrollContainerRef} | |
| role="log" | |
| aria-label="채팅 메시지" | |
| className="min-h-0 flex-1 overflow-x-hidden overflow-y-auto overscroll-contain py-3" | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Chat/ChatRoom.tsx` around lines 156 - 159, The message list
container (the div with ref scrollContainerRef in ChatRoom.tsx) needs to be
exposed as a live region so assistive tech announces new messages; update that
div to include role="log" and provide an accessible name (using aria-label or
aria-labelledby, e.g., aria-label="Chat messages") and consider adding
aria-atomic/aria-live attributes as needed to ensure new message announcements;
ensure the change is applied to the same element that receives appended messages
so screen readers detect additions.
Comment on lines
+25
to
+31
| <input | ||
| className="flex-1 bg-transparent text-base font-medium text-indigo-300 outline-none placeholder:text-indigo-300" | ||
| placeholder={SEARCH_PLACEHOLDER} | ||
| value={value} | ||
| onChange={onChange ? (e) => onChange(e.target.value) : undefined} | ||
| autoFocus={autoFocus} | ||
| /> |
There was a problem hiding this comment.
검색 input에 접근 가능한 이름을 추가해 주세요.
placeholder만으로는 스크린리더 이름이 잡히지 않습니다. aria-label이나 연결된 <label>이 필요합니다.
예시 수정
<input
+ type="search"
+ aria-label={SEARCH_PLACEHOLDER}
className="flex-1 bg-transparent text-base font-medium text-indigo-300 outline-none placeholder:text-indigo-300"
placeholder={SEARCH_PLACEHOLDER}
value={value}As per coding guidelines, "src/pages/**/components/**/*.tsx: ... 접근성(aria-*, role, 키보드 탐색)을 우선 확인하는지".
📝 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
| <input | |
| className="flex-1 bg-transparent text-base font-medium text-indigo-300 outline-none placeholder:text-indigo-300" | |
| placeholder={SEARCH_PLACEHOLDER} | |
| value={value} | |
| onChange={onChange ? (e) => onChange(e.target.value) : undefined} | |
| autoFocus={autoFocus} | |
| /> | |
| <input | |
| type="search" | |
| aria-label={SEARCH_PLACEHOLDER} | |
| className="flex-1 bg-transparent text-base font-medium text-indigo-300 outline-none placeholder:text-indigo-300" | |
| placeholder={SEARCH_PLACEHOLDER} | |
| value={value} | |
| onChange={onChange ? (e) => onChange(e.target.value) : undefined} | |
| autoFocus={autoFocus} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Club/ClubList/components/SearchBar.tsx` around lines 25 - 31, The
search input in SearchBar.tsx lacks an accessible name; update the <input> in
the SearchBar component to include an accessible name (preferably add an
aria-label attribute or wire a visible <label> to it). Specifically, either add
aria-label={ariaLabel ?? SEARCH_PLACEHOLDER} and accept an optional ariaLabel
prop on the SearchBar, or add a <label htmlFor="club-search"> tied to input
id="club-search"; ensure the new prop or id/label change is used where value and
onChange are passed so screen readers can announce the field.
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
1 participant
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 by CodeRabbit
릴리스 노트
새로운 기능
스타일
변경