Conversation
📝 WalkthroughWalkthrough멤버 관리 리팩토링: 제네릭 검색/선택 컴포넌트(SearchSelect) 추가, 멤버 타입 통일(TWorkspaceMember), 페이지 수준 상태 관리로 이동, 멤버 삭제 플로우와 권한 변경(임시저장/저장/취소) 기능이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SearchSelect
participant MemberSearchSelect
participant TransferOwnershipModal
participant MemberManagement
User->>SearchSelect: 입력 및 포커스
SearchSelect->>SearchSelect: keyword 업데이트 → filteredOptions 계산
SearchSelect-->>User: 필터된 옵션 목록 렌더
User->>SearchSelect: 옵션 선택
SearchSelect->>MemberSearchSelect: onSelect(member)
MemberSearchSelect->>TransferOwnershipModal: selectedMember 갱신
User->>TransferOwnershipModal: 소유권 이전 확인
TransferOwnershipModal->>MemberManagement: onConfirm(selectedMember)
MemberManagement->>MemberManagement: roles 상태 업데이트 (비동기 처리, toast)
sequenceDiagram
actor User
participant MemberItem
participant MemberManagement
participant DeleteMemberModal
participant Backend
User->>MemberItem: 삭제 버튼 클릭
MemberItem->>MemberManagement: openDeleteMember(member)
MemberManagement->>MemberManagement: 검증(자신/마지막관리자)
MemberManagement->>DeleteMemberModal: 모달 오픈(selectedDeleteMember)
User->>DeleteMemberModal: 삭제 확인 클릭
DeleteMemberModal->>MemberManagement: onConfirm(member)
MemberManagement->>Backend: DELETE /members/{memberId}
Backend-->>MemberManagement: 성공/실패
MemberManagement->>MemberManagement: members 상태 갱신 및 모달 종료
MemberManagement-->>User: toast 알림
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
검토 관점 (간단 체크리스트)
필요하면 특정 파일(예: SearchSelect의 이벤트 핸들러, MemberManagement의 handleDeleteMember/handleTransferOwnership) 코드를 지정해서 더 깊게 리뷰해줄게. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
📚 Storybook 배포 완료
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/workspace/WorkspaceSetting.tsx (1)
77-79:⚠️ Potential issue | 🟡 Minor
useEffect의존성 배열에fetchWorkspaceDetail이 누락되었습니다.ESLint의
react-hooks/exhaustive-deps규칙에 따르면fetchWorkspaceDetail이 의존성 배열에 포함되어야 합니다. 현재orgId만 포함되어 있어 린터 경고가 발생할 수 있습니다.🐛 제안하는 수정
fetchWorkspaceDetail을useCallback으로 감싸거나, 함수를useEffect내부로 이동하세요:+ const fetchWorkspaceDetail = useCallback(async () => { + // ... 기존 로직 + }, [orgId]); useEffect(() => { void fetchWorkspaceDetail(); - }, [orgId]); + }, [fetchWorkspaceDetail]);또는 함수를 useEffect 내부에 정의:
useEffect(() => { + const fetchWorkspaceDetail = async () => { + // ... 기존 로직 + }; void fetchWorkspaceDetail(); }, [orgId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/WorkspaceSetting.tsx` around lines 77 - 79, The useEffect in WorkspaceSetting currently depends only on orgId while calling fetchWorkspaceDetail, causing an exhaustive-deps lint warning; wrap fetchWorkspaceDetail with useCallback (e.g., define fetchWorkspaceDetail via useCallback inside the WorkspaceSetting component) or move the fetchWorkspaceDetail definition inside the useEffect so that the dependency array is complete, ensuring the effect lists fetchWorkspaceDetail and orgId (or only orgId if the function is defined inside the effect) and eliminating the lint error.src/components/workspace/MemberRoleSelect.tsx (1)
64-76:⚠️ Potential issue | 🟡 Minor
disabled상태에서 opacity가 중복 적용됩니다.Line 65와 Line 76 모두
disabled상태일 때opacity-50을 적용하고 있어, 실제로는 opacity가 0.25(0.5 × 0.5)로 렌더링될 수 있습니다.🐛 제안하는 수정
<div - className={`overflow-hidden rounded-[22px] ${disabled ? "opacity-50" : ""}`} + className="overflow-hidden rounded-[22px]" > <button type="button" disabled={disabled} aria-expanded={isOpen} aria-haspopup="menu" aria-controls={isOpen && !disabled ? menuId : undefined} onClick={() => setIsOpen((prev) => !prev)} className={`flex h-10 min-w-25 items-center justify-between gap-3 rounded-[22px] px-4 font-body2 transition-colors ${ triggerStyleMap[role] } ${disabled ? "cursor-not-allowed opacity-50" : "cursor-pointer hover:brightness-95"}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/MemberRoleSelect.tsx` around lines 64 - 76, The wrapper div and the button both apply "opacity-50" when disabled, causing double-opacity; remove the duplicated opacity class from one of them (prefer keeping it only on the outer div or only on the button) so the disabled state uses a single "opacity-50" application; update the JSX in MemberRoleSelect (the outer div and the button that uses disabled, triggerStyleMap, aria-controls and onClick/setIsOpen) to ensure only one element includes "opacity-50" while preserving the disabled attribute, cursor-not-allowed class, and existing triggerStyleMap usage.
🧹 Nitpick comments (7)
src/components/workspace/TransferOwnershipBlockedModal.tsx (1)
22-23:WarnIcon크기가 다른 모달과 다릅니다.다른 모달들(
DeleteMemberModal,TransferOwnershipModal,WorkspaceSetting)에서는WarnIcon에w-15 h-15크기를 지정하고 있습니다. 일관성을 위해 동일한 크기를 적용하는 것을 권장합니다.♻️ 제안하는 수정
- icon={<WarnIcon className="text-status-red" aria-hidden="true" />} + icon={<WarnIcon className="text-status-red w-15 h-15" aria-hidden="true" />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/TransferOwnershipBlockedModal.tsx` around lines 22 - 23, The WarnIcon used inside ModalContent in TransferOwnershipBlockedModal has a different size than other modals; update the icon's className from just "text-status-red" to include the standard sizing "w-15 h-15" (i.e., use "text-status-red w-15 h-15") so it matches DeleteMemberModal, TransferOwnershipModal, and WorkspaceSetting; modify the JSX where WarnIcon is rendered in TransferOwnershipBlockedModal to include the additional classes.src/components/workspace/PermissionTable.tsx (1)
11-13: 타입 정의가 중복됩니다.
TPermissionRow가 이미defaultMemberEnabled: boolean을 포함하고 있고,admin/member필드가 제거되었으므로Omit처리가 불필요합니다.♻️ 제안하는 수정
-const permissionRows: Array< - Omit<TPermissionRow, "admin" | "member"> & { defaultMemberEnabled: boolean } -> = [ +const permissionRows: TPermissionRow[] = [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/PermissionTable.tsx` around lines 11 - 13, The type annotation on permissionRows is redundant because TPermissionRow already includes defaultMemberEnabled and admin/member were removed; replace the current Omit-based type with the direct TPermissionRow type (e.g., use Array<TPermissionRow> or TPermissionRow[]) or remove the explicit annotation to let TypeScript infer it, and ensure references to permissionRows, TPermissionRow, defaultMemberEnabled, admin, and member are updated accordingly.src/components/workspace/MemberItem.tsx (1)
46-55: 역할 스타일이MemberRoleSelect와 중복됩니다.
member.isMe일 때 적용되는 역할 스타일(Line 50-51)이MemberRoleSelect.tsx의triggerStyleMap과 동일합니다. 한 곳에서 스타일이 변경되면 불일치가 발생할 수 있습니다.♻️ 공통 스타일 추출 제안
triggerStyleMap을 별도 상수 파일로 분리하거나,MemberRoleSelect에서 export하여 재사용하는 것을 권장합니다:// MemberRoleSelect.tsx에서 export export const roleStyleMap: Record<TMemberRole, string> = { ADMIN: "bg-status-blue/80 text-white shadow-sm", MEMBER: "bg-chart-3/15 text-text-auth-sub", }; // MemberItem.tsx에서 import하여 사용 import { roleStyleMap } from "./MemberRoleSelect";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/MemberItem.tsx` around lines 46 - 55, Extract the role-to-class mapping into a single exported constant (e.g., roleStyleMap or roleStyleMap: Record<TMemberRole,string>) and use it from MemberRoleSelect (or a new shared constants file) in both components; in MemberRoleSelect keep using the exported map for its triggerStyleMap logic and in MemberItem replace the inline ternary inside the member.isMe span with roleStyleMap[member.role] and use the same text ("관리자"/"멤버") logic, ensuring both components import the shared roleStyleMap so styles stay consistent.src/components/workspace/TransferOwnershipModal.tsx (1)
51-51: className에 불필요한 trailing space가 있습니다.
"px-2 py-6 "끝에 공백이 포함되어 있습니다. 기능에 영향은 없지만 정리하면 좋겠습니다.♻️ 제안하는 수정
- <div className="text-center px-2 py-6 "> + <div className="text-center px-2 py-6">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/TransferOwnershipModal.tsx` at line 51, The className string in TransferOwnershipModal's JSX div (the element with className "text-center px-2 py-6 ") contains an unnecessary trailing space; edit the JSX in TransferOwnershipModal.tsx to remove the trailing space so the className becomes "text-center px-2 py-6" (update the div where className is set).src/pages/workspace/WorkspaceSetting.tsx (1)
310-310:WarnIcon크기가 모달 내부와 다릅니다.Line 310에서는
w-12 h-12, Line 322-323에서는w-15 h-15를 사용하고 있습니다.ControlBox와Modal컨텍스트 차이로 의도된 것일 수 있으나, 디자인 시스템 관점에서 일관성 검토를 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/WorkspaceSetting.tsx` at line 310, The WarnIcon size is inconsistent: it's rendered as w-12 h-12 in the leadingSlot (WarnIcon) but as w-15 h-15 elsewhere inside the same modal/ControlBox context; update the className on the WarnIcon in the leadingSlot (or the other occurrences) so all instances use the same sizing token (e.g., change w-12 h-12 to w-15 h-15 or vice versa) to match the design system, and ensure any surrounding components (ControlBox / Modal) don't override sizing via CSS so the icon appears consistently.src/components/workspace/MemberRoleSelect.tsx (1)
83-101: 키보드 내비게이션 추가를 권장합니다.현재
role="menu"와role="menuitem"이 설정되어 있지만, 키보드 내비게이션(화살표 키, Enter, Escape)이 구현되어 있지 않습니다. 접근성 표준(WCAG)에서는 메뉴 컴포넌트의 키보드 조작을 권장합니다.향후 개선 시 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/MemberRoleSelect.tsx` around lines 83 - 101, The menu currently renders with role="menu" and role="menuitem" but lacks keyboard navigation; update the MemberRoleSelect component to add keyboard handlers that, when isOpen is true, trap focus and allow ArrowDown/ArrowUp to move a focused index over restOptions, Enter/Space to call handleSelect(option) for the highlighted item, and Escape to close the menu (mirroring whatever closes isOpen). Ensure each rendered button uses a focusable/tabindex state (or set aria-activedescendant/aria-selected) and that menuId and restOptions are used to locate items; keep role attributes (role="menu"/"menuitem") and ensure focus is set to the first/selected item when opening and returned to the trigger when closing.src/components/common/select/SearchSelect.tsx (1)
89-95: 공용 searchable select라면 combobox ARIA를 같이 연결해두는 게 좋겠습니다.지금은 입력창과 팝업이 시각적으로만 연결되어 있어서, 보조기기에서는 드롭다운 열린 상태와 옵션 목록의 관계를 알기 어렵습니다.
aria-expanded/aria-controls,role="listbox"/option정도는 이 컴포넌트 레벨에서 붙여두는 편이 재사용 시 안전합니다.As per coding guidelines,
src/**: 접근성: 시맨틱 HTML, ARIA 속성 사용 확인.Also applies to: 99-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/select/SearchSelect.tsx` around lines 89 - 95, The Input in SearchSelect.tsx (value={keyword}, onFocus={handleFocus}, onChange -> handleChangeKeyword) needs ARIA combobox linking: add aria-expanded tied to the component's open state (e.g., isOpen), aria-controls pointing to the popup/list id, and role="combobox" on the input (or wrapper) as appropriate; ensure the popup/list element has role="listbox" and each item has role="option" and an id that matches aria-activedescendant when navigating. Update handleFocus/other open/close logic to maintain the isOpen state used for aria-expanded and ensure the generated list id is stable (e.g., derive from component id or a generated uid) so the input's aria-controls correctly references the listbox.
🤖 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/components/common/select/SearchSelect.tsx`:
- Around line 41-43: The effect that resets keyword runs because getOptionLabel
is included as a dependency and is passed inline from MemberSearchSelect; change
the effect in useEffect(() => { setKeyword(selectedOption ?
getOptionLabel(selectedOption) : ""); }, [selectedOption, getOptionLabel]); to
synchronize only when selectedOption actually changes (remove getOptionLabel
from deps or compare previous selectedOption) so typing isn't reset on parent
re-renders — alternatively stabilize the parent's callback by wrapping
getOptionLabel in useCallback in MemberSearchSelect; additionally, add
accessibility attributes: wire the input to the dropdown with aria-expanded and
aria-controls on the input, give the dropdown container role="listbox" and each
item role="option" (and ensure options have appropriate aria-selected/ids) so
screen readers can navigate the custom select.
In `@src/components/workspace/InviteMemberModal.tsx`:
- Around line 50-61: handleInvite currently always shows success because there
is no await/API call; replace the placeholder with a real async request that
constructs a TInviteMemberRequest (using trimmedEmail), await the API call
inside handleInvite, only call toast.success and setForm({ email: "" }) after
the awaited call succeeds, and move toast.error and console.log("초대 실패", error)
into the catch to surface the actual error; ensure you reference handleInvite,
TInviteMemberRequest, setForm and toast so the request, response handling, and
error logging are implemented correctly.
In `@src/components/workspace/MemberList.tsx`:
- Around line 133-139: In MemberList.tsx change the JSX list key to use the
memberId identifier to match the callbacks: when mapping members (members.map)
pass key={member.memberId} to the MemberItem component (the map that renders
<MemberItem ... />) so the reconciliation key aligns with the
onRoleChange/onDeleteClick callbacks that use member.memberId, preventing stale
row reuse when emails change or duplicate emails exist.
In `@src/pages/workspace/MemberManagement.tsx`:
- Around line 108-116: handleRoleChange currently applies newRole blindly which
can break the admin invariant; before calling setMembers in handleRoleChange
compute current adminCount from members, locate the target member, and if the
target is an ADMIN and newRole !== 'ADMIN' and adminCount === 1, reject the
change (no-op or show error); additionally, if targetMemberId equals the current
user's id (e.g. currentUserId/sessionUserId) prevent demoting your own role to
non-ADMIN unless ownership transfer flow completed; only call setMembers(prev =>
prev.map(...)) after these checks so member.role updates cannot create
adminCount === 0 or allow self-demotion without transfer.
- Around line 88-90: The members state is initialized once with
useState(mockMembers) so it persists across workspaceId changes; update
MemberManagement to reset or reload members whenever workspaceId changes by
adding an effect that watches useParams()/workspaceId (orgId) and either clears
setMembers([]) or triggers the members fetch, and when using react-query include
workspaceId/orgId in the query key so cached data is per-workspace. Also revise
handleRoleChange to enforce the same "last admin" constraint as openDeleteMember
by checking current members for remaining admins before allowing a role change
that would remove the last admin, and surface a validation/error if the change
is forbidden.
---
Outside diff comments:
In `@src/components/workspace/MemberRoleSelect.tsx`:
- Around line 64-76: The wrapper div and the button both apply "opacity-50" when
disabled, causing double-opacity; remove the duplicated opacity class from one
of them (prefer keeping it only on the outer div or only on the button) so the
disabled state uses a single "opacity-50" application; update the JSX in
MemberRoleSelect (the outer div and the button that uses disabled,
triggerStyleMap, aria-controls and onClick/setIsOpen) to ensure only one element
includes "opacity-50" while preserving the disabled attribute,
cursor-not-allowed class, and existing triggerStyleMap usage.
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 77-79: The useEffect in WorkspaceSetting currently depends only on
orgId while calling fetchWorkspaceDetail, causing an exhaustive-deps lint
warning; wrap fetchWorkspaceDetail with useCallback (e.g., define
fetchWorkspaceDetail via useCallback inside the WorkspaceSetting component) or
move the fetchWorkspaceDetail definition inside the useEffect so that the
dependency array is complete, ensuring the effect lists fetchWorkspaceDetail and
orgId (or only orgId if the function is defined inside the effect) and
eliminating the lint error.
---
Nitpick comments:
In `@src/components/common/select/SearchSelect.tsx`:
- Around line 89-95: The Input in SearchSelect.tsx (value={keyword},
onFocus={handleFocus}, onChange -> handleChangeKeyword) needs ARIA combobox
linking: add aria-expanded tied to the component's open state (e.g., isOpen),
aria-controls pointing to the popup/list id, and role="combobox" on the input
(or wrapper) as appropriate; ensure the popup/list element has role="listbox"
and each item has role="option" and an id that matches aria-activedescendant
when navigating. Update handleFocus/other open/close logic to maintain the
isOpen state used for aria-expanded and ensure the generated list id is stable
(e.g., derive from component id or a generated uid) so the input's aria-controls
correctly references the listbox.
In `@src/components/workspace/MemberItem.tsx`:
- Around line 46-55: Extract the role-to-class mapping into a single exported
constant (e.g., roleStyleMap or roleStyleMap: Record<TMemberRole,string>) and
use it from MemberRoleSelect (or a new shared constants file) in both
components; in MemberRoleSelect keep using the exported map for its
triggerStyleMap logic and in MemberItem replace the inline ternary inside the
member.isMe span with roleStyleMap[member.role] and use the same text
("관리자"/"멤버") logic, ensuring both components import the shared roleStyleMap so
styles stay consistent.
In `@src/components/workspace/MemberRoleSelect.tsx`:
- Around line 83-101: The menu currently renders with role="menu" and
role="menuitem" but lacks keyboard navigation; update the MemberRoleSelect
component to add keyboard handlers that, when isOpen is true, trap focus and
allow ArrowDown/ArrowUp to move a focused index over restOptions, Enter/Space to
call handleSelect(option) for the highlighted item, and Escape to close the menu
(mirroring whatever closes isOpen). Ensure each rendered button uses a
focusable/tabindex state (or set aria-activedescendant/aria-selected) and that
menuId and restOptions are used to locate items; keep role attributes
(role="menu"/"menuitem") and ensure focus is set to the first/selected item when
opening and returned to the trigger when closing.
In `@src/components/workspace/PermissionTable.tsx`:
- Around line 11-13: The type annotation on permissionRows is redundant because
TPermissionRow already includes defaultMemberEnabled and admin/member were
removed; replace the current Omit-based type with the direct TPermissionRow type
(e.g., use Array<TPermissionRow> or TPermissionRow[]) or remove the explicit
annotation to let TypeScript infer it, and ensure references to permissionRows,
TPermissionRow, defaultMemberEnabled, admin, and member are updated accordingly.
In `@src/components/workspace/TransferOwnershipBlockedModal.tsx`:
- Around line 22-23: The WarnIcon used inside ModalContent in
TransferOwnershipBlockedModal has a different size than other modals; update the
icon's className from just "text-status-red" to include the standard sizing
"w-15 h-15" (i.e., use "text-status-red w-15 h-15") so it matches
DeleteMemberModal, TransferOwnershipModal, and WorkspaceSetting; modify the JSX
where WarnIcon is rendered in TransferOwnershipBlockedModal to include the
additional classes.
In `@src/components/workspace/TransferOwnershipModal.tsx`:
- Line 51: The className string in TransferOwnershipModal's JSX div (the element
with className "text-center px-2 py-6 ") contains an unnecessary trailing space;
edit the JSX in TransferOwnershipModal.tsx to remove the trailing space so the
className becomes "text-center px-2 py-6" (update the div where className is
set).
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Line 310: The WarnIcon size is inconsistent: it's rendered as w-12 h-12 in the
leadingSlot (WarnIcon) but as w-15 h-15 elsewhere inside the same
modal/ControlBox context; update the className on the WarnIcon in the
leadingSlot (or the other occurrences) so all instances use the same sizing
token (e.g., change w-12 h-12 to w-15 h-15 or vice versa) to match the design
system, and ensure any surrounding components (ControlBox / Modal) don't
override sizing via CSS so the icon appears consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 082c6f37-3bd4-4145-a8ab-e8c19de1381c
⛔ Files ignored due to path filters (1)
src/assets/icon/common/mail.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (13)
src/components/common/select/SearchSelect.tsxsrc/components/workspace/DeleteMemberModal.tsxsrc/components/workspace/InviteMemberModal.tsxsrc/components/workspace/MemberItem.tsxsrc/components/workspace/MemberList.tsxsrc/components/workspace/MemberRoleSelect.tsxsrc/components/workspace/MemberSearchSelect.tsxsrc/components/workspace/PermissionTable.tsxsrc/components/workspace/TransferOwnershipBlockedModal.tsxsrc/components/workspace/TransferOwnershipModal.tsxsrc/pages/workspace/MemberManagement.tsxsrc/pages/workspace/WorkspaceSetting.tsxsrc/types/workspace/workspace.ts
🚨 관련 이슈
Closed #110
✨ 변경사항
✏️ 작업 내용
TWorkspaceMember로 통일해서 중복타입 제거MemberManageMent를 기준으로 멤버 목록, 역할 변경, 관리자 변경, 멤버 삭제 흐름을 같은 멤버 상태 참조하도록 리팩토링 진행TransferOwnershipBlockedModal나오도록 수정SearchSelect기반으로 분리할 수 있도록 구조 정리하고, 멤버 검색 select의 재사용성을 높임변경사항 저장하기버튼 추가하여, 중요도가 높은 작업인 권한 변경작업의 변경하기 쉬운 토글의 위험성 해결MemberRoleSelect스타일 수정해서 시각적으로 구분이 명확하도록 수정email에서memberId중심으로 정리해서 식별 안정성 높임😅 미완성 작업
📢 논의 사항 및 참고 사항
Summary by CodeRabbit
새로운 기능
버그 수정
개선 사항