feat: show roleName in group list page#142
Conversation
📝 WalkthroughWalkthroughPull Request는 두 컴포넌트에서 Crown 아이콘의 렌더링 로직을 개선합니다. GroupItem 컴포넌트는 로컬 SVG에서 iconoir-react의 Crown으로 변경하고, 인증 컨텍스트를 추가하여 사용자의 회장 및 매니저 상태를 기반으로 조건부 렌더링을 구현합니다. MemberTableRow 컴포넌트는 Crown 아이콘을 노란색으로 스타일링된 div로 감싸는 마크업 변경을 수행합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/pages/main/GroupItem.tsx (2)
35-42: getCrownColor 함수를 컴포넌트 외부로 이동하는 것을 고려하세요.현재 getCrownColor 함수는 컴포넌트 내부에 정의되어 있어 매 렌더링마다 새로 생성됩니다. 성능 최적화를 위해 함수를 컴포넌트 외부로 추출하거나 useMemo를 사용하는 것을 고려할 수 있습니다.
♻️ 제안하는 리팩토링
+const getCrownColor = ( + isPresident: boolean, + isAdmin: boolean, + isManager: boolean +): string | null => { + if (isPresident) return "text-yellow-500"; + if (isAdmin) return "text-slate-400"; + if (isManager) return "text-amber-700"; + return null; +}; + const GroupItem = ({ groupParams, }: { groupParams: { group: GroupInfoWithPresidentUuid; }; }) => { const group = groupParams.group; const { userInfo } = useAuth(); const { data: userRole } = useSWR( ["userRole", group.uuid || ""], ([_, uuid]) => getUserRole(uuid), ); if (!userRole) return <></>; const isAdmin = userRole.name === "admin"; const isManager = userRole.name === "manager"; const isPresident = userInfo?.uuid === group.presidentUuid; - const getCrownColor = () => { - if (isPresident) return "text-yellow-500"; - if (isAdmin) return "text-slate-400"; - if (isManager) return "text-amber-700"; - return null; - }; - - const crownColor = getCrownColor(); + const crownColor = getCrownColor(isPresident, isAdmin, isManager);
55-57: Crown 아이콘에 접근성 속성을 추가하는 것을 고려하세요.Crown 아이콘이 역할을 나타내는 중요한 UI 요소이므로, 스크린 리더 사용자를 위해 aria-label이나 title 속성을 추가하는 것이 좋습니다.
♻️ 제안하는 개선
- {crownColor && <Crown className={cn(crownColor)} />} + {crownColor && ( + <Crown + className={cn(crownColor)} + aria-label={ + isPresident ? "그룹장" : + isAdmin ? "관리자" : + isManager ? "매니저" : "" + } + /> + )}또는 더 간단하게:
const getRoleLabel = () => { if (isPresident) return "그룹장"; if (isAdmin) return "관리자"; if (isManager) return "매니저"; return ""; };그리고 JSX에서:
- {crownColor && <Crown className={cn(crownColor)} />} + {crownColor && ( + <Crown + className={cn(crownColor)} + aria-label={getRoleLabel()} + /> + )}src/pages/manage/pages/members/sections/memberManagement/MemberTableRow.tsx (1)
66-69: Crown 아이콘 마크업을 간소화하고 접근성을 개선하세요.현재 Crown 아이콘이 추가 div로 감싸져 있는데, 이를 간소화할 수 있습니다. 또한 스크린 리더 사용자를 위해 접근성 속성을 추가하는 것이 좋습니다.
♻️ 제안하는 개선
Option 1: div 제거하고 Crown에 직접 className 적용
<div className="flex items-center gap-2"> {member.name} - <div className="text-yellow-500"> - {isThisMemberPresident && <Crown />} - </div> + {isThisMemberPresident && ( + <Crown + className="text-yellow-500" + aria-label="그룹장" + /> + )} </div>Option 2: div를 유지하되 더 의미있는 구조로 개선
<div className="flex items-center gap-2"> {member.name} - <div className="text-yellow-500"> - {isThisMemberPresident && <Crown />} - </div> + {isThisMemberPresident && ( + <span className="text-yellow-500" role="img" aria-label="그룹장"> + <Crown /> + </span> + )} </div>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/main/GroupItem.tsxsrc/pages/manage/pages/members/sections/memberManagement/MemberTableRow.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/pages/main/GroupItem.tsx (1)
src/utils/clsx/index.ts (1)
cn(5-5)
🔇 Additional comments (4)
src/pages/main/GroupItem.tsx (3)
22-22: userInfo가 undefined인 경우 처리를 확인하세요.useAuth()에서 반환된 userInfo가 undefined일 수 있는 경우(예: 인증 로딩 중), Line 33의 isPresident 체크는 항상 false가 됩니다. 현재 로직이 의도한 동작인지 확인하고, 필요시 로딩 상태를 명시적으로 처리하는 것을 고려하세요.
Also applies to: 33-33
47-53: 변경 사항이 적절합니다.프로필 이미지의 마크업 구조 변경이 깔끔하게 적용되었습니다. 기능상 문제없습니다.
4-4: Crown 컴포넌트는 iconoir-react@7.10.1에 정상적으로 포함되어 있으며, 현재 코드의 import 및 사용이 모두 올바릅니다.src/pages/manage/pages/members/sections/memberManagement/MemberTableRow.tsx (1)
67-67: 색상 일관성이 잘 유지되었습니다.text-yellow-500 색상이 GroupItem.tsx의 그룹장 크라운 색상과 일치하여 일관된 UI를 제공합니다.
| const getCrownColor = () => { | ||
| if (isPresident) return "text-yellow-500"; | ||
| if (isAdmin) return "text-slate-400"; | ||
| if (isManager) return "text-amber-700"; | ||
| return null; | ||
| }; |
| if (isPresident) return "text-yellow-500"; | ||
| if (isAdmin) return "text-slate-400"; | ||
| if (isManager) return "text-amber-700"; |
There was a problem hiding this comment.
| if (isPresident) return "text-yellow-500"; | |
| if (isAdmin) return "text-slate-400"; | |
| if (isManager) return "text-amber-700"; | |
| if (isPresident) return cn("text-yellow-500"); | |
| if (isAdmin) return cn("text-slate-400"); | |
| if (isManager) return cn("text-amber-700"); |
IDE 자동 완성을 위해 cn을 클래스 이름에 항상 넣어주는 게 좋습니다
|
|
||
| <p className="flex items-center gap-2 ml-[15px] mr-[5px] text-lg font-semibold text-dark dark:text-d_white"> | ||
| {group.name} | ||
| {crownColor && <Crown className={cn(crownColor)} />} |
There was a problem hiding this comment.
| {crownColor && <Crown className={cn(crownColor)} />} | |
| {crownColor && <Crown className={crownColor} />} |
각 그룹 내 본인의 역할(roleName)에 따라 왕관 아이콘의 색상을 구분하여 표시하도록 변경했습니다.
기존에 회색으로 표시되던 그룹장의 왕관 아이콘을 그룹 리스트 디자인과 통일하기 위해 금색으로 변경했습니다.
구현 의도: 멤버 리스트에는 이미 역할(roleName)을 텍스트로 보여주는 열이 존재하므로, 시각적 중복을 피하기 위해 그룹장 외의 역할(관리자, 매니저)에는 별도의 왕관 아이콘을 추가하지 않았습니다.
Summary by CodeRabbit
릴리스 노트
그룹 항목과 회원 목록의 사용자 역할 표시 기능을 개선했습니다.
✏️ Tip: You can customize this high-level summary in your review settings.