Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough워크스페이스 관리 UI(목록 검색, 생성 모달, 설정 페이지)와 관련 타입·카드·textarea 컴포넌트를 추가하고, 공통 UI 마크업/스타일 조정 및 라우팅·레이아웃 래핑을 적용했습니다. 접근성 속성(aria) 추가와 일부 클래스/구조 변경도 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant WorkspacePage as WorkspacePage
participant CreateModal as CreateModal
participant DropdownMenu as DropdownMenu
participant Router as Router
User->>WorkspacePage: 페이지 접근 / 검색어 입력
WorkspacePage->>WorkspacePage: 로컬 리스트 필터링
WorkspacePage->>User: 필터된 카드 목록 렌더
User->>WorkspacePage: '생성' 버튼 클릭
WorkspacePage->>CreateModal: 모달 오픈
User->>CreateModal: 로고/이름/설명 입력 및 제출
CreateModal->>WorkspacePage: 새 워크스페이스 로컬 추가 (API: TODO)
User->>WorkspacePage: 카드의 드롭다운 열기
User->>DropdownMenu: '정보 수정하기' 선택
DropdownMenu->>Router: 네비게이션 요청
Router->>User: `WorkspaceSetting` 페이지로 이동
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
jjjsun
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
src/components/common/dropdownmenu/DropdownMenu.tsx (1)
41-56:⚠️ Potential issue | 🟠 Major여러 인스턴스 동시 렌더링 시
id="dropdown-menu"중복으로 접근성 위반
id="dropdown-menu"와aria-controls="dropdown-menu"가 정적 문자열로 고정되어 있습니다. 이 PR에서 추가된Workspace.tsx처럼 목록에서 여러DropdownMenu를 렌더링하면 같은 페이지에 동일한id가 여러 개 생겨 HTML 명세 위반이고, 스크린 리더가 잘못된 요소를 참조합니다.
useId()로 인스턴스별 고유 ID를 생성하세요.♿️ 수정 예시 (useId 활용)
- import React, { useEffect, useRef, useState } from "react"; + import React, { useEffect, useId, useRef, useState } from "react"; export function DropdownMenu({ ... }) { const [open, setOpen] = useState(false); const ref = useRef<HTMLDivElement | null>(null); + const menuId = useId(); ... <div role="button" ... - aria-controls="dropdown-menu" + aria-controls={menuId} ... > {trigger} </div> {open && ( <div - id="dropdown-menu" + id={menuId} role="menu" ... >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 41 - 56, The DropdownMenu component uses a static id "dropdown-menu" referenced by aria-controls, causing duplicate IDs when multiple instances render; update DropdownMenu to call React's useId() (or equivalent unique-id generator) inside the component and use that value for both the aria-controls on the trigger element and the id on the menu container (replace the hardcoded "dropdown-menu"), leaving the existing open, setOpen, trigger props and event handlers unchanged so each instance gets a unique id.
🧹 Nitpick comments (4)
src/pages/workspace/Workspace.tsx (3)
191-205:textarea스타일이Input컴포넌트와 중복 — 공통 컴포넌트 검토
textarea에 직접 작성된 Tailwind 클래스(bg-gray-50,focus:ring-2,focus:ring-logo-1/30,text-text-main,placeholder:text-text-placeholder등)가Input.tsx의 스타일과 거의 동일합니다. API 연동 이후 디자인 토큰이 변경될 경우 두 곳을 같이 수정해야 합니다. 공용Textarea컴포넌트를 만들거나Input이as="textarea"를 지원하도록 확장하는 것을 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 191 - 205, The textarea in Workspace (id="workspace-desc", value={newDesc}, onChange={setNewDesc}) duplicates Input.tsx styling; extract a shared Textarea component or extend Input to support as="textarea" so styles live in one place. Replace the inline <textarea> with the shared Textarea (or Input as="textarea") and move classes like bg-gray-50, focus:ring-2, focus:ring-logo-1/30, text-text-main, placeholder:text-text-placeholder, rounded-component-md, px-5 py-4, min-h-[120px] into that component so Workspace only passes props (value, onChange, placeholder, id).
27-49: 빈 의존성 배열useMemo([], [])는 안티패턴 — 모듈 레벨 상수로 이동의존성 배열이 비어 있는
useMemo는 실질적으로 컴포넌트 마운트 시 한 번만 계산되므로 모듈 레벨 상수와 동일합니다. 컴포넌트 외부로 꺼내면 불필요한 훅 오버헤드를 줄이고 코드 의도가 더 명확해집니다.♻️ 리팩토링 예시
+ // 컴포넌트 외부 (모듈 레벨) + const MOCK_WORKSPACES: TWorkspace[] = [ + { id: "1", name: "광고회사1", description: "광고회사1입니다.", myRole: "admin" }, + { id: "2", name: "광고회사2", description: "광고회사2입니다.", myRole: "admin" }, + { id: "3", name: "광고회사3", description: "광고회사3입니다.", myRole: "admin" }, + ]; export default function WorkspacePage() { ... - const workspaces: TWorkspace[] = useMemo( - () => [...], - [], - ); + const workspaces = MOCK_WORKSPACES;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 27 - 49, The hard-coded workspaces array is wrapped in useMemo with an empty dependency array (workspaces: TWorkspace[] = useMemo(...)), which is an anti-pattern; move that literal out of the component into a module-level constant (e.g., export const WORKSPACES: TWorkspace[] = [...]) and replace the in-component useMemo usage with a direct reference to WORKSPACES (or import it), remove the now-unused useMemo import, and ensure any references to the old workspaces identifier inside the component are updated to the module-level constant.
109-126: 불필요한 타입 변환과 nullable fallback 정리
String(w.id):TWorkSpaceId는 이미string이므로String()변환이 불필요합니다.{w.description ?? ""}:description은TWorkspace에서 required 필드(string)이므로 nullish coalescing이 동작할 일이 없습니다.{w.myRole ?? "내 직책 및 역할"}: 마찬가지로myRole도 required 필드입니다.♻️ 정리 예시
- key={String(w.id)} + key={w.id} - {w.description ?? ""} + {w.description} - {w.myRole ?? "내 직책 및 역할"} + {w.myRole}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 109 - 126, The JSX is using unnecessary fallbacks and a redundant String() conversion: remove the String(w.id) conversion and use key={w.id} directly (TWorkSpaceId is already string), and drop the nullish coalescing for required fields in the Workspace component — render w.description and w.myRole directly instead of {w.description ?? ""} and {w.myRole ?? "내 직책 및 역할"} inside the render block that maps workspace items (identify by the variable w in Workspace.tsx).src/types/workspace/workspace.ts (1)
6-8:url/logoUrl의 이중 nullable 패턴 및myRole타입 정밀도 개선 고려두 가지 개선 포인트입니다.
url?: string | null은undefined,null,string세 가지 상태가 가능합니다. API 응답 스키마에 따라string | null또는string | undefined중 하나로 통일하면 소비 측 코드에서 불필요한 이중 체크를 피할 수 있습니다.
myRole: string은 실질적으로 어떤 문자열도 허용합니다. 실제 역할 값이 고정된 집합이라면 union type으로 명시하는 것이 타입 안정성에 유리합니다.♻️ 개선 예시
- url?: string | null; - logoUrl?: string | null; - myRole: string; + url?: string; + logoUrl?: string; + myRole: "admin" | "member" | "viewer"; // 실제 역할 값으로 조정🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/workspace/workspace.ts` around lines 6 - 8, The fields url and logoUrl currently allow both undefined and null (url?: string | null), causing unnecessary dual-nullability; pick one based on the API contract (either change to url: string | null or url?: string) and update logoUrl the same way to remove the duplicate check; also replace myRole: string with a precise union or enum (e.g., type UserRole = 'owner' | 'admin' | 'member' | 'guest' or an exported enum) and use that type for myRole so callers get proper autocomplete and type safety, then adjust any consumers of url, logoUrl, and myRole to match the chosen nullability and new role type.
🤖 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/input/Input.tsx`:
- Around line 42-47: The label and input can end up with zero vertical spacing
when wrapperClassName removes the wrapper's gap (gap-2) and the label's mb-2 is
also removed; update the Input component (look for label render using inputId
and wrapperClassName) to ensure a minimal vertical gap always exists by adding a
small margin-bottom to the label (e.g., add a compact class like "mb-1" to the
label element) or by guaranteeing the wrapper includes a gap class when
wrapperClassName doesn't provide one.
In `@src/pages/workspace/Workspace.tsx`:
- Around line 151-165: The Modal currently renders the same heading twice
because the component passes title="워크스페이스 생성" to Modal while also rendering an
internal <h2> with the same text; determine whether Modal already displays its
title and then remove the duplicate: if Modal renders its title prop, delete the
internal <h2 className="font-heading3 text-text-main mb-2"> 워크스페이스 생성 </h2>;
otherwise remove the title prop from the Modal and keep the internal <h2>.
Ensure you only change the Modal usage in the Workspace component
(createOpen/onClose handlers remain unchanged).
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 4-6: The unused variables from useNavigate and useParams (navigate
and workspaceId) cause TS6133 build errors; either remove the declarations
entirely from WorkspaceSetting or mark them as intentionally unused by prefixing
with an underscore (e.g., _navigate and _workspaceId via destructuring from
useParams) so the compiler knows they are deliberate placeholders; update the
const lines that call useNavigate() and useParams() (the navigate and
workspaceId symbols) accordingly and leave the placeholder component return
as-is.
In `@src/routes/MainRoutes.tsx`:
- Around line 50-52: Update the sidebarNav/footerNav paths to match the actual
route "workspace/:workspaceId/settings" and ensure Sidebar.tsx injects the
current workspaceId when rendering footerNav links (it currently does
to={item.path}). Specifically, change the settings entry in sidebarNav to either
a static workspace path like "workspace/{workspaceId}/settings" or convert the
nav item to a path-generator function, then update Sidebar.tsx to obtain
workspaceId (e.g., via useParams() or the Zustand store) and use it to build the
final URL for to={item.path} (or call the generator) so links like the settings
and billing routes resolve correctly to WorkspaceSetting and workspace/billing.
In `@src/types/workspace/workspace.ts`:
- Line 9: The optional field representativeName in the Workspace type is
ambiguous and should not be left undocumented; either remove the
representativeName?: string; declaration from src/types/workspace/workspace.ts
if the API/ERD doesn't include it, or create a tracked issue and replace the
inline "빼도될듯" comment with a TODO referencing that issue (e.g., TODO: track in
ISSUE-###) and a short rationale; ensure any removal is reflected in related
API/type usages (search for representativeName) and update the type export or
docs accordingly.
---
Outside diff comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 41-56: The DropdownMenu component uses a static id "dropdown-menu"
referenced by aria-controls, causing duplicate IDs when multiple instances
render; update DropdownMenu to call React's useId() (or equivalent unique-id
generator) inside the component and use that value for both the aria-controls on
the trigger element and the id on the menu container (replace the hardcoded
"dropdown-menu"), leaving the existing open, setOpen, trigger props and event
handlers unchanged so each instance gets a unique id.
---
Nitpick comments:
In `@src/pages/workspace/Workspace.tsx`:
- Around line 191-205: The textarea in Workspace (id="workspace-desc",
value={newDesc}, onChange={setNewDesc}) duplicates Input.tsx styling; extract a
shared Textarea component or extend Input to support as="textarea" so styles
live in one place. Replace the inline <textarea> with the shared Textarea (or
Input as="textarea") and move classes like bg-gray-50, focus:ring-2,
focus:ring-logo-1/30, text-text-main, placeholder:text-text-placeholder,
rounded-component-md, px-5 py-4, min-h-[120px] into that component so Workspace
only passes props (value, onChange, placeholder, id).
- Around line 27-49: The hard-coded workspaces array is wrapped in useMemo with
an empty dependency array (workspaces: TWorkspace[] = useMemo(...)), which is an
anti-pattern; move that literal out of the component into a module-level
constant (e.g., export const WORKSPACES: TWorkspace[] = [...]) and replace the
in-component useMemo usage with a direct reference to WORKSPACES (or import it),
remove the now-unused useMemo import, and ensure any references to the old
workspaces identifier inside the component are updated to the module-level
constant.
- Around line 109-126: The JSX is using unnecessary fallbacks and a redundant
String() conversion: remove the String(w.id) conversion and use key={w.id}
directly (TWorkSpaceId is already string), and drop the nullish coalescing for
required fields in the Workspace component — render w.description and w.myRole
directly instead of {w.description ?? ""} and {w.myRole ?? "내 직책 및 역할"} inside
the render block that maps workspace items (identify by the variable w in
Workspace.tsx).
In `@src/types/workspace/workspace.ts`:
- Around line 6-8: The fields url and logoUrl currently allow both undefined and
null (url?: string | null), causing unnecessary dual-nullability; pick one based
on the API contract (either change to url: string | null or url?: string) and
update logoUrl the same way to remove the duplicate check; also replace myRole:
string with a precise union or enum (e.g., type UserRole = 'owner' | 'admin' |
'member' | 'guest' or an exported enum) and use that type for myRole so callers
get proper autocomplete and type safety, then adjust any consumers of url,
logoUrl, and myRole to match the chosen nullability and new role type.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/assets/icon/workspace/Vector.svgis excluded by!**/*.svgand included bysrc/**src/assets/icon/workspace/plus.svgis excluded by!**/*.svgand included bysrc/**src/assets/icon/workspace/search.svgis excluded by!**/*.svgand included bysrc/**src/assets/icon/workspace/uploadImg.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (7)
src/components/common/button/Button.tsxsrc/components/common/dropdownmenu/DropdownMenu.tsxsrc/components/common/input/Input.tsxsrc/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsxsrc/routes/MainRoutes.tsxsrc/types/workspace/workspace.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/pages/workspace/Workspace.tsx (1)
151-165: 모달 제목이 중복 렌더링됩니다.
Modal의titleprop("워크스페이스 생성")과 내부<h2>태그가 동일한 텍스트를 렌더링하고 있습니다. Modal 컴포넌트가titleprop을 통해 제목을 이미 표시한다면, 내부<h2>는 제거해야 합니다. 접근성 측면에서도 동일한 heading이 두 번 나오면 스크린 리더 사용자에게 혼란을 줄 수 있습니다.✏️ Modal이 title을 렌더링하는 경우 수정 예시
<div className="px-2"> - <h2 className="font-heading3 text-text-main mb-2"> - 워크스페이스 생성 - </h2> <p className="font-body1 text-text-sub mb-6">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 151 - 165, The Modal is rendering the same heading twice via its title prop ("워크스페이스 생성") and the internal <h2> element; remove the duplicate by deleting the internal <h2 className="font-heading3 ..."> 워크스페이스 생성 </h2> (or alternatively remove the Modal title prop and keep the <h2> if Modal doesn't render an accessible heading), leaving the paragraph and content intact and keeping onClose/setCreateOpen behavior unchanged.
🧹 Nitpick comments (8)
src/pages/workspace/Workspace.tsx (3)
27-49: 정적 데이터에useMemo를 사용할 필요가 없습니다.
workspaces배열은 의존성이 빈 배열([])인 상수 데이터입니다.useMemo로 감싸는 것은 불필요한 오버헤드이며, 컴포넌트 외부의 상수로 선언하는 것이 더 명확합니다. API 연동 시useQuery등으로 교체될 것이므로, 지금은 모듈 스코프 상수로 충분합니다.♻️ 컴포넌트 외부 상수로 추출
+const MOCK_WORKSPACES: TWorkspace[] = [ + { id: "1", name: "광고회사1", description: "광고회사1입니다.", myRole: "admin" }, + { id: "2", name: "광고회사2", description: "광고회사2입니다.", myRole: "admin" }, + { id: "3", name: "광고회사3", description: "광고회사3입니다.", myRole: "admin" }, +]; + export default function WorkspacePage() { const navigate = useNavigate(); // ... - const workspaces: TWorkspace[] = useMemo( - () => [ - { id: "1", name: "광고회사1", description: "광고회사1입니다.", myRole: "admin" }, - { id: "2", name: "광고회사2", description: "광고회사2입니다.", myRole: "admin" }, - { id: "3", name: "광고회사3", description: "광고회사3입니다.", myRole: "admin" }, - ], - [], - ); + const workspaces = MOCK_WORKSPACES;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 27 - 49, The workspaces array is static and wrapped unnecessarily in useMemo; move it out of the component to a module-scope constant (e.g., const WORKSPACES: TWorkspace[] = [...]) and remove the useMemo usage inside the component so the component references WORKSPACES directly; update any local variable named workspaces to reference the new module constant and keep the same shape (id, name, description, myRole) so downstream code using workspaces / TWorkspace remains unaffected.
56-67:menuItems함수가 렌더링마다 새 배열을 생성합니다.현재 목록 크기가 작아 성능 영향은 미미하지만,
menuItems가 매 렌더 시 호출되어DropdownMenu에 새 참조의items를 전달합니다. 추후 목록이 커지거나DropdownMenu에React.memo가 적용될 경우,useCallback으로 감싸거나 아이템 정의를 메모이제이션하는 것이 좋습니다. 지금 당장 필수는 아니므로 참고 수준으로 남깁니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 56 - 67, The menuItems function currently recreates a new array on every render, causing DropdownMenu to receive a new items reference each time; fix this by memoizing the items (e.g., wrap menuItems in useCallback or compute the items with useMemo) so it returns a stable reference for the same workspace id, updating only when dependencies like navigate or the id change; locate the menuItems definition in Workspace.tsx and replace it with a memoized version (or move the static item definitions outside the component) to avoid unnecessary re-renders of DropdownMenu.
185-214: 모달 내 폼 요소가<form>으로 감싸져 있지 않습니다.입력 필드와 제출 버튼이
<form>태그 없이<div>로만 구성되어 있어, 사용자가 Enter 키를 눌러 제출할 수 없습니다.<form onSubmit>으로 감싸면 키보드 접근성과 시맨틱 HTML 모두 개선됩니다.♻️ form 태그 적용 예시
- <div className="space-y-6 mx-auto w-full max-w-[800px]"> + <form + className="space-y-6 mx-auto w-full max-w-[800px]" + onSubmit={(e) => { e.preventDefault(); onSubmitCreate(); }} + > ... - <Button - size="big" - variant="primary" - onClick={onSubmitCreate} - disabled={!newName.trim()} - className="mx-auto px-10 mt-10" - > + <Button + size="big" + variant="primary" + type="submit" + disabled={!newName.trim()} + className="mx-auto px-10 mt-10" + > 생성하기 </Button> - </div> + </form>As per coding guidelines, "시맨틱 HTML, ARIA 속성 사용 확인."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 185 - 214, Wrap the modal's inputs and submit Button in a semantic <form> that uses the existing onSubmitCreate handler (or create a new handleSubmit that calls onSubmitCreate and calls event.preventDefault()). Move the Button to type="submit" so Enter keypresses trigger submission; keep the disabled check using newName.trim(). Ensure the textarea with id "workspace-desc", the Input component for the workspace name, and the Button remain inside the form so keyboard/semantic behavior is preserved.src/pages/workspace/WorkspaceSetting.tsx (5)
36-95: textarea 스타일이Workspace.tsx의 생성 모달과 중복됩니다.Lines 80-86의
<textarea>className이Workspace.tsxLine 200과 거의 동일합니다. 현재 두 곳에서만 사용되지만, 공통Textarea컴포넌트로 추출하면 스타일 일관성 유지와 향후 변경에 유리합니다.Input컴포넌트가 이미 공통화되어 있으므로 같은 패턴을 따르면 됩니다.🤖 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 36 - 95, The textarea in WorkspaceSetting (id="workspace-setting-desc") duplicates styling used in Workspace's creation modal; extract a reusable Textarea component (following the pattern of the existing Input component) that accepts props like id, value, onChange, placeholder, className and label, move the className styling into that component, and replace the raw <textarea> in WorkspaceSetting and the textarea in Workspace (creation modal) with the new Textarea to keep styles consistent and make future updates easier.
28-35: 헤더 영역이Workspace.tsx와 동일하게 반복되고 있습니다.Lines 29-35의
<header>블록이Workspace.tsxLines 80-86과 완전히 동일한 구조와 텍스트입니다. 현재 두 곳뿐이므로 급하지는 않지만, 워크스페이스 관련 페이지가 늘어날 경우 공통 레이아웃 컴포넌트나 페이지 헤더 컴포넌트로 추출하면 유지보수가 편해집니다.🤖 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 28 - 35, Extract the duplicated header block from WorkspaceSetting.tsx and Workspace.tsx into a shared component (e.g., WorkspaceHeader or WorkspacePageHeader) and replace the inline <header> in both files with that new component; the shared component should accept props for title and subtitle (defaulting to "워크 스페이스 관리" and "워크스페이스 정보를 확인하고 관리하세요." if desired), be exported, and then imported/used in WorkspaceSetting and Workspace to remove duplication and centralize header styling and text.
22-26:onDelete에서navigate가 동기적으로 항상 호출됩니다.현재는 API 연동 전이라 문제없지만, 추후 API 연동 시 삭제 요청이 실패해도 무조건
/workspace로 이동하는 구조가 됩니다. API 연동 시점에navigate호출을 삭제 성공 콜백 안으로 이동해야 한다는 점을 참고해 주세요.onSave도 마찬가지로 성공/실패 분기가 필요할 것입니다.🤖 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 22 - 26, The onDelete handler currently calls navigate synchronously (after setDeleteOpen) which will redirect even if a future API delete fails; change onDelete to perform the delete API call first (make onDelete async), await the delete response, call setDeleteOpen(false) and navigate("/workspace", { replace: true }) only on successful deletion, and handle/display errors on failure; apply the same pattern to onSave so navigation and UI state updates occur only in the success branch and failures are surfaced to the user (refer to onDelete, onSave, navigate, setDeleteOpen, and workspaceId to locate the logic).
66-94: 폼 영역이<form>태그로 감싸져 있지 않습니다.
Workspace.tsx의 생성 모달과 동일한 이슈입니다. 입력 필드 + 저장 버튼 구조이므로<form onSubmit>으로 감싸면 Enter 키 제출과 시맨틱 HTML 측면에서 개선됩니다. 저장 버튼(Line 98-106)까지 포함하여<form>으로 래핑하는 것을 권장합니다.As per coding guidelines, "시맨틱 HTML, ARIA 속성 사용 확인."
🤖 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 66 - 94, Wrap the input block (the Input components bound to name, website and the textarea bound to desc) and the save button together in a semantic <form> and handle submission via onSubmit so Enter key triggers save and markup is accessible; add a handler like handleSubmit that calls the existing save/update routine (preventDefault() first) and move any existing save onClick into that handler (or have the button be type="submit") so you keep state setters setName/setDesc/setWebsite unchanged and ensure the form includes the save button and existing ids (e.g., workspace-setting-desc) for accessibility.
148-158: 삭제 확인 모달에 취소 버튼이 없습니다.사용자가 삭제를 재고할 수 있도록 "삭제하기" 버튼 옆에 명시적인 "취소" 버튼을 추가하는 것이 좋습니다. Modal의 닫기(X) 버튼으로도 닫을 수 있지만, 파괴적 액션(destructive action) 모달에서는 명시적 취소 옵션이 UX 관점에서 권장됩니다.
♻️ 취소 버튼 추가 예시
<div className="flex justify-center"> + <Button + variant="secondary" + size="big" + onClick={() => setDeleteOpen(false)} + className="px-12 mr-3" + > + 취소 + </Button> <Button variant="danger" size="big" aria-label="워크스페이스 최종 삭제 버튼" onClick={onDelete} className="px-12" > 삭제하기 </Button> </div>🤖 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 148 - 158, The deletion modal only renders a destructive "삭제하기" Button (onClick={onDelete}) and lacks an explicit cancel control; add a second, non-destructive "취소" Button next to the existing Button that closes the modal (call the modal close handler, e.g., onClose or setShowModal(false)) instead of performing deletion, give it an appropriate aria-label (e.g., "워크스페이스 삭제 취소 버튼"), a neutral variant/size consistent with the existing Button, and ensure proper spacing (e.g., className="mr-3" on the cancel button) so users have an explicit cancel option alongside the destructive action.
🤖 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/pages/workspace/Workspace.tsx`:
- Line 82: The heading text in Workspace.tsx currently uses "워크 스페이스 관리" (with a
space) while other UI (including WorkspaceSetting.tsx line 31) uses "워크스페이스" (no
space); update the <h1> in Workspace.tsx (and the corresponding title in
WorkspaceSetting.tsx) to use the unified string "워크스페이스 관리" so all headings,
modal titles, and buttons consistently display the same Korean term.
- Around line 108-115: The code is rendering the URL string directly via
w.logoUrl; update the rendering inside the card (in Workspace.tsx where
w.logoUrl is used) to conditionally render an image element using w.logoUrl as
the src when present and a styled placeholder (or empty SVG/avatar) when absent,
ensure the image uses the existing container sizing/classes (the div with
className "w-20 h-20 ...") and include an alt attribute (e.g., workspace name or
"logo") and basic onError fallback to the placeholder so a broken URL doesn't
show raw text.
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Line 60: In WorkspaceSetting.tsx, remove the trailing whitespace from the
aria-label value on the logo reset button (the attribute currently set to "로고
이미지 초기화 버튼 "); update the aria-label to "로고 이미지 초기화 버튼" in the JSX/element that
renders the button so screen readers and lint rules see a clean, exact string.
- Line 31: The h1 title string in WorkspaceSetting.tsx ("워크 스페이스 관리") is
inconsistent with Workspace.tsx; update the heading text in the JSX (the <h1> in
WorkspaceSetting component) to "워크스페이스 관리" to match Workspace.tsx and ensure
both components use the exact same string for the workspace management title.
---
Duplicate comments:
In `@src/pages/workspace/Workspace.tsx`:
- Around line 151-165: The Modal is rendering the same heading twice via its
title prop ("워크스페이스 생성") and the internal <h2> element; remove the duplicate by
deleting the internal <h2 className="font-heading3 ..."> 워크스페이스 생성 </h2> (or
alternatively remove the Modal title prop and keep the <h2> if Modal doesn't
render an accessible heading), leaving the paragraph and content intact and
keeping onClose/setCreateOpen behavior unchanged.
---
Nitpick comments:
In `@src/pages/workspace/Workspace.tsx`:
- Around line 27-49: The workspaces array is static and wrapped unnecessarily in
useMemo; move it out of the component to a module-scope constant (e.g., const
WORKSPACES: TWorkspace[] = [...]) and remove the useMemo usage inside the
component so the component references WORKSPACES directly; update any local
variable named workspaces to reference the new module constant and keep the same
shape (id, name, description, myRole) so downstream code using workspaces /
TWorkspace remains unaffected.
- Around line 56-67: The menuItems function currently recreates a new array on
every render, causing DropdownMenu to receive a new items reference each time;
fix this by memoizing the items (e.g., wrap menuItems in useCallback or compute
the items with useMemo) so it returns a stable reference for the same workspace
id, updating only when dependencies like navigate or the id change; locate the
menuItems definition in Workspace.tsx and replace it with a memoized version (or
move the static item definitions outside the component) to avoid unnecessary
re-renders of DropdownMenu.
- Around line 185-214: Wrap the modal's inputs and submit Button in a semantic
<form> that uses the existing onSubmitCreate handler (or create a new
handleSubmit that calls onSubmitCreate and calls event.preventDefault()). Move
the Button to type="submit" so Enter keypresses trigger submission; keep the
disabled check using newName.trim(). Ensure the textarea with id
"workspace-desc", the Input component for the workspace name, and the Button
remain inside the form so keyboard/semantic behavior is preserved.
In `@src/pages/workspace/WorkspaceSetting.tsx`:
- Around line 36-95: The textarea in WorkspaceSetting
(id="workspace-setting-desc") duplicates styling used in Workspace's creation
modal; extract a reusable Textarea component (following the pattern of the
existing Input component) that accepts props like id, value, onChange,
placeholder, className and label, move the className styling into that
component, and replace the raw <textarea> in WorkspaceSetting and the textarea
in Workspace (creation modal) with the new Textarea to keep styles consistent
and make future updates easier.
- Around line 28-35: Extract the duplicated header block from
WorkspaceSetting.tsx and Workspace.tsx into a shared component (e.g.,
WorkspaceHeader or WorkspacePageHeader) and replace the inline <header> in both
files with that new component; the shared component should accept props for
title and subtitle (defaulting to "워크 스페이스 관리" and "워크스페이스 정보를 확인하고 관리하세요." if
desired), be exported, and then imported/used in WorkspaceSetting and Workspace
to remove duplication and centralize header styling and text.
- Around line 22-26: The onDelete handler currently calls navigate synchronously
(after setDeleteOpen) which will redirect even if a future API delete fails;
change onDelete to perform the delete API call first (make onDelete async),
await the delete response, call setDeleteOpen(false) and navigate("/workspace",
{ replace: true }) only on successful deletion, and handle/display errors on
failure; apply the same pattern to onSave so navigation and UI state updates
occur only in the success branch and failures are surfaced to the user (refer to
onDelete, onSave, navigate, setDeleteOpen, and workspaceId to locate the logic).
- Around line 66-94: Wrap the input block (the Input components bound to name,
website and the textarea bound to desc) and the save button together in a
semantic <form> and handle submission via onSubmit so Enter key triggers save
and markup is accessible; add a handler like handleSubmit that calls the
existing save/update routine (preventDefault() first) and move any existing save
onClick into that handler (or have the button be type="submit") so you keep
state setters setName/setDesc/setWebsite unchanged and ensure the form includes
the save button and existing ids (e.g., workspace-setting-desc) for
accessibility.
- Around line 148-158: The deletion modal only renders a destructive "삭제하기"
Button (onClick={onDelete}) and lacks an explicit cancel control; add a second,
non-destructive "취소" Button next to the existing Button that closes the modal
(call the modal close handler, e.g., onClose or setShowModal(false)) instead of
performing deletion, give it an appropriate aria-label (e.g., "워크스페이스 삭제 취소
버튼"), a neutral variant/size consistent with the existing Button, and ensure
proper spacing (e.g., className="mr-3" on the cancel button) so users have an
explicit cancel option alongside the destructive action.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/icon/workspace/message-circle-warning.svgis excluded by!**/*.svgand included bysrc/**src/assets/icon/workspace/warning.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (4)
src/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsxsrc/routes/MainRoutes.tsxsrc/types/workspace/workspace.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types/workspace/workspace.ts
- src/routes/MainRoutes.tsx
jjjsun
changed the title
Collaborator
|
P4: UI 구현 내용 PR은 스크린샷 필수로 포함하기로 했던 것 같습니다! |
Collaborator
|
P1: 피그마 댓글 보니까 준영님께서 백엔드에서 워크스페이스 생성 시에 웹사이트 URL 은 받지 않도록 설계되어 있다고 하십니다! 워크스페이스 관리의 웹사이트 URL은 제거해야 할 것 같습니다. |
Seojegyeong
reviewed
Feb 24, 2026
Collaborator
|
P1: 반응형 설계가 전반적으로 적용되어 있지 않습니다! |
Seojegyeong
reviewed
Feb 24, 2026
Seojegyeong
reviewed
Feb 24, 2026
Collaborator
|
P4: 고생하셨습니다! 마지막으로 논의사항 관련해서는 MainLayout에서 main에 재선님이 해주신 것처럼 (py-8 px-20)추가하면 좋을 것 같아요! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/common/textarea/TextareaField.tsx (1)
25-33:resize함수를useCallback으로 감싸고useLayoutEffect의존성 배열에 추가해야 합니다.
resize가useLayoutEffect내부에서 호출되지만 의존성 배열에 없어eslint-plugin-react-hooks의exhaustive-deps규칙을 위반합니다. 런타임에는ref가 stable해서 문제가 없지만, 의존성 배열이 불완전하면 추후resize내부에 다른 state/prop이 포함될 경우 무음 버그가 발생할 수 있습니다.resize는onInput에서도 재사용되므로useCallback으로 메모이제이션하는 것이 적합합니다.♻️ 제안:
resize를useCallback으로 감싸기- const resize = () => { + const resize = useCallback(() => { const el = ref.current; if (!el) return; el.style.height = "auto"; el.style.height = `${el.scrollHeight}px`; - }; + }, []); + useLayoutEffect(() => { resize(); - }, [value]); + }, [value, resize]);상단 import에
useCallback추가가 필요합니다:-import { useLayoutEffect, useRef } from "react"; +import { useCallback, useLayoutEffect, useRef } from "react";As per coding guidelines,
useEffect의존성 배열 및 불필요한 사용을 검토해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/textarea/TextareaField.tsx` around lines 25 - 33, Wrap the resize function in useCallback and add it to the useLayoutEffect dependency array: change the standalone const resize = () => { ... } to a memoized const resize = useCallback(() => { ... }, [/* include any state/props used inside */]) (also import useCallback) and then update useLayoutEffect(() => { resize(); }, [resize]); this ensures resize is stable for reuse in onInput and satisfies exhaustive-deps.src/components/workspace/WorkspaceCard.tsx (1)
37-42:description과myRole에 대한 nullish coalescing(??)이 TypeScript 타입 정의와 불일치합니다.
TWorkspace.description과TWorkspace.myRole은string(optional/nullable 아님)으로 선언되어 있어?? "",?? "내 직책 및 역할"연산자는 타입상 dead code입니다. API 미연동 상태에서 방어적 코딩 의도가 있다면, 이를 타입에도 반영(description?: string | null,myRole?: string | null)하거나 연동 후 제거하는 것이 타입 현실 일치 관점에서 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/WorkspaceCard.tsx` around lines 37 - 42, The component uses nullish coalescing on w.description and w.myRole although the TWorkspace type declares them as non-nullable strings; update types or the JSX to match reality: either change TWorkspace to allow optional/null values (e.g., description?: string | null, myRole?: string | null) so the checks (w.description ?? "" and w.myRole ?? "내 직책 및 역할") are valid, or remove the redundant ?? fallbacks in WorkspaceCard.tsx (the w.description and w.myRole usages) if the API guarantees non-null strings.
🤖 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/workspace/WorkspaceCard.tsx`:
- Around line 47-58: The trigger currently passes a <button> into DropdownMenu
causing a nested interactive element (div[role="button"] > button) which breaks
accessibility; change WorkspaceCard so the trigger is a non-interactive element
(e.g., the VectorIcon wrapped in a span or the raw SVG) instead of a <button>,
and add an aria-label prop to DropdownMenu (e.g., ariaLabel or aria-label) so
the outer interactive wrapper in DropdownMenu receives the menu label/aria
attributes; update DropdownMenu to accept that prop and apply it to the outer
div (role="button" tabIndex={0} aria-haspopup="menu" aria-expanded={open}
aria-label={ariaLabel}) while leaving menuItems and existing behavior unchanged
so no nested interactive elements remain.
---
Nitpick comments:
In `@src/components/common/textarea/TextareaField.tsx`:
- Around line 25-33: Wrap the resize function in useCallback and add it to the
useLayoutEffect dependency array: change the standalone const resize = () => {
... } to a memoized const resize = useCallback(() => { ... }, [/* include any
state/props used inside */]) (also import useCallback) and then update
useLayoutEffect(() => { resize(); }, [resize]); this ensures resize is stable
for reuse in onInput and satisfies exhaustive-deps.
In `@src/components/workspace/WorkspaceCard.tsx`:
- Around line 37-42: The component uses nullish coalescing on w.description and
w.myRole although the TWorkspace type declares them as non-nullable strings;
update types or the JSX to match reality: either change TWorkspace to allow
optional/null values (e.g., description?: string | null, myRole?: string | null)
so the checks (w.description ?? "" and w.myRole ?? "내 직책 및 역할") are valid, or
remove the redundant ?? fallbacks in WorkspaceCard.tsx (the w.description and
w.myRole usages) if the API guarantees non-null strings.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/assets/icon/workspace/building.svgis excluded by!**/*.svgand included bysrc/**src/assets/icon/workspace/edit-contained.svgis excluded by!**/*.svgand included bysrc/**src/assets/icon/workspace/userProfile.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (6)
src/components/common/input/Input.tsxsrc/components/common/textarea/TextareaField.tsxsrc/components/workspace/WorkspaceCard.tsxsrc/layout/main/MainLayout.tsxsrc/pages/workspace/Workspace.tsxsrc/pages/workspace/WorkspaceSetting.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/pages/workspace/WorkspaceSetting.tsx
- src/components/common/input/Input.tsx
- src/pages/workspace/Workspace.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/common/dropdownmenu/DropdownMenu.tsx (1)
11-20:⚠️ Potential issue | 🔴 Critical
triggerClassNameprop 누락으로 빌드가 실패합니다 —DropdownMenu에 추가가 필요합니다.
WorkspaceCard에서triggerClassName을 전달하고 있지만, 현재DropdownMenu의 props 타입에 정의되어 있지 않아 TypeScript 빌드 오류가 발생합니다. 또한className은 외부 wrapperdiv에 적용되고,div[role="button"]에는 현재className이 전혀 없어 trigger 영역 스타일이 동작하지 않습니다.🐛 수정 제안 (DropdownMenu.tsx)
export function DropdownMenu({ trigger, items, className, + triggerClassName, "aria-label": ariaLabel, }: { trigger: React.ReactNode; items: TMenuItem[]; className?: string; + triggerClassName?: string; "aria-label"?: string; }) { ... <div role="button" tabIndex={0} aria-haspopup="menu" aria-expanded={open} aria-controls="dropdown-menu" aria-label={ariaLabel} + className={triggerClassName} onClick={() => setOpen((v) => !v)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 11 - 20, The DropdownMenu component is missing a triggerClassName prop which breaks builds and prevents styling on the trigger element; update the DropdownMenu props to include triggerClassName?: string (alongside existing className and "aria-label"), accept it in the function signature, and apply that value to the trigger element (the div with role="button" / the trigger wrapper) while keeping className applied to the outer wrapper so both wrapper and trigger get their respective classes; ensure the prop is optional and passed through unchanged from callers like WorkspaceCard.
🧹 Nitpick comments (1)
src/components/workspace/WorkspaceCard.tsx (1)
40-42:myRole폴백 문자열 처리현재 폴백 값
"내 직책 및 역할"이 하드코딩되어 있습니다. 실제 역할 정보가 없을 때 사용자에게 의미 없는 placeholder처럼 보일 수 있으므로, 빈 문자열 처리 또는 조건부 렌더링을 고려해 보세요.💡 제안
- <div className="font-body1 text-text-sub mt-2"> - {w.myRole ?? "내 직책 및 역할"} - </div> + {w.myRole && ( + <div className="font-body1 text-text-sub mt-2">{w.myRole}</div> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/WorkspaceCard.tsx` around lines 40 - 42, The component WorkspaceCard currently uses a hardcoded fallback string for w.myRole which reads like a placeholder; instead, change the rendering to avoid the hardcoded text by conditionally rendering the div or an empty string when w.myRole is falsy (use the existing w.myRole expression in WorkspaceCard.tsx and the same div with className "font-body1 text-text-sub mt-2" to locate the spot), so that when there is no role you either render nothing or a neutral/ARIA-friendly placeholder instead of "내 직책 및 역할".
🤖 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/dropdownmenu/DropdownMenu.tsx`:
- Around line 43-44: The DropdownMenu component currently hardcodes
id="dropdown-menu" and aria-controls="dropdown-menu", causing duplicate IDs and
broken accessibility when multiple DropdownMenu instances render; update
DropdownMenu to generate a stable unique id using React's useId (e.g., const id
= useId()), replace the hardcoded id on the menu panel with `${id}-menu` and set
the trigger's aria-controls to the same `${id}-menu` (also update any
aria-labelledby/aria-describedby references to use the generated id), and ensure
all places in DropdownMenu (including the panel and trigger render paths
referenced around the current id usage) use this generated id so each instance
has a unique, SSR-safe identifier.
In `@src/components/workspace/WorkspaceCard.tsx`:
- Around line 47-52: The build fails because DropdownMenu's props do not include
triggerClassName; update the DropdownMenu component to accept a new optional
prop named triggerClassName (e.g., add it to the props/type used in
DropdownMenu.tsx) and apply that string to the element rendering the trigger
(the div with role="button") so the passed className from WorkspaceCard
(<DropdownMenu ... triggerClassName="h-10 w-10 rounded-xl ..." />) is applied;
ensure the prop is typed optional and forwarded where the trigger element is
rendered in DropdownMenu.
---
Outside diff comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 11-20: The DropdownMenu component is missing a triggerClassName
prop which breaks builds and prevents styling on the trigger element; update the
DropdownMenu props to include triggerClassName?: string (alongside existing
className and "aria-label"), accept it in the function signature, and apply that
value to the trigger element (the div with role="button" / the trigger wrapper)
while keeping className applied to the outer wrapper so both wrapper and trigger
get their respective classes; ensure the prop is optional and passed through
unchanged from callers like WorkspaceCard.
---
Nitpick comments:
In `@src/components/workspace/WorkspaceCard.tsx`:
- Around line 40-42: The component WorkspaceCard currently uses a hardcoded
fallback string for w.myRole which reads like a placeholder; instead, change the
rendering to avoid the hardcoded text by conditionally rendering the div or an
empty string when w.myRole is falsy (use the existing w.myRole expression in
WorkspaceCard.tsx and the same div with className "font-body1 text-text-sub
mt-2" to locate the spot), so that when there is no role you either render
nothing or a neutral/ARIA-friendly placeholder instead of "내 직책 및 역할".
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/common/dropdownmenu/DropdownMenu.tsxsrc/components/workspace/WorkspaceCard.tsx
YermIm
reviewed
Feb 24, 2026
Collaborator
|
P3: 조직 검색 input 호버 시의 스타일이 기본 상태(호버X)에서도 보이면 좋을 거 같습니다. 개인적으로 현재는 검색창의 경계가 모호한 것 같습니다! |
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 (1)
src/components/common/dropdownmenu/DropdownMenu.tsx (1)
26-35:⚠️ Potential issue | 🟠 Major
role="menu"위젯에서 Escape 키 닫기 처리가 누락되어 있습니다WAI-ARIA의 Menu Button Pattern에 따르면
role="menu"위젯은 반드시 Escape 키로 닫힐 수 있어야 하며, 닫힌 후 트리거 버튼으로 포커스를 반환해야 합니다. 현재 트리거의onKeyDown은 Enter/Space만 처리하고, 열린 메뉴 내부에서 Escape를 누르면 아무런 동작도 하지 않습니다.💡 Escape 키 처리 예시
+ const triggerRef = useRef<HTMLDivElement | null>(null); useEffect(() => { const onDocClick = (e: MouseEvent) => { if (!ref.current) return; if (!ref.current.contains(e.target as Node)) setOpen(false); }; + const onKeyUp = (e: KeyboardEvent) => { + if (e.key === "Escape" && open) { + setOpen(false); + triggerRef.current?.focus(); + } + }; document.addEventListener("mousedown", onDocClick); + document.addEventListener("keydown", onKeyUp); return () => { document.removeEventListener("mousedown", onDocClick); + document.removeEventListener("keydown", onKeyUp); }; - }, []); + }, [open]);Also applies to: 47-52
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 26 - 35, DropdownMenu is missing Escape handling: when the menu (role="menu") is open pressing Escape must close it and return focus to the trigger. Add a keydown handler that listens for "Escape" while the menu is open (either on the document or the menu root) and calls setOpen(false) and moves focus back to the trigger element (the same element used by the trigger onKeyDown handler); update the existing trigger onKeyDown to also handle Escape when the menu is open by calling setOpen(false) and focusing the trigger, and ensure you register/removing the event listener in the same effect used for outside clicks (referencing ref, setOpen and the trigger DOM ref/name used in this component) so cleanup happens correctly.
♻️ Duplicate comments (3)
src/pages/workspace/Workspace.tsx (1)
133-203: 모달 관련 이전 리뷰 이슈 해결 확인 (LGTM)
Modaltitleprop 중복 렌더링 →titleprop 제거 후 내부<h2>만 유지하여 해결 ✓"워크 스페이스"띄어쓰기 불일치 →"워크스페이스 관리"로 통일 ✓w.logoUrl문자열 직접 렌더링 →WorkspaceCard내부에서 조건부<img>처리로 해결 ✓- 워크스페이스 목록
ul > li시맨틱 구조 →<ul>/WorkspaceCard(<li>)구조로 해결 ✓🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 133 - 203, Confirm the modal/title duplication and related UI fixes by removing any leftover title prop on the Modal component and keeping the internal <h2> header in Workspace.tsx (verify Modal usage only uses createOpen/onClose/size/padding props), ensure WorkspaceCard now handles w.logoUrl with conditional <img> rendering (no direct string render of w.logoUrl), verify the workspace list uses semantic <ul> with WorkspaceCard rendered as <li>, and confirm the "워크스페이스 관리" spacing/text is consistent and that create flow uses newName/newDesc and onSubmitCreate as shown.src/components/common/dropdownmenu/DropdownMenu.tsx (1)
1-1: 중복 ID 및 접근성 문제 해결 확인 (LGTM)
useId를 도입하여menuId를 동적으로 생성하고,aria-controls/id를 각각 연결한 것이 이전 리뷰에서 지적된 중복 ID 문제를 올바르게 해결합니다.aria-labelprop 지원 추가도WorkspaceCard와의 연동에서 잘 활용되고 있습니다.Also applies to: 24-24, 44-45, 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` at line 1, The DropdownMenu component should consistently avoid duplicate IDs and support accessibility props: ensure every instance uses React's useId to generate menuId (e.g., const menuId = useId()), apply that menuId to the trigger's aria-controls and the menu's id, and accept/forward an aria-label prop from DropdownMenu to the trigger/menu; update any other occurrences (the repeated blocks referenced around the other ranges) to the same pattern so all triggers use id={menuId}, aria-controls={menuId} and the menu element has id={menuId}, and propagate aria-label through to WorkspaceCard integrations.src/components/common/input/Input.tsx (1)
42-50: 라벨-인풋 간격 개선 확인 (LGTM)이전 리뷰에서 지적된
gap-2/mb-2제거로 인한 간격 소실 문제가mb-1을 label에 추가하여 해소되었습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/input/Input.tsx` around lines 42 - 50, The spacing issue between the label and input has been resolved by adding margin-bottom to the label; verify that the <label> element (using inputId) includes className "mb-1" and that the container still uses twMerge with wrapperClassName (in Input.tsx) so no gap-2 or mb-2 classes are present; if missing, add "mb-1" to the label's className to restore the vertical spacing and keep the surrounding wrapper classes unchanged.
🧹 Nitpick comments (3)
src/pages/workspace/Workspace.tsx (3)
76-80: 모달 열기 시 파일 상태 초기화 필요
onOpenCreate에서newName과newDesc는 초기화되지만, 파일 상태(위에서 제안한logoFile,logoPreview)가 추가된다면 이 함수에서 함께 초기화되어야 합니다. 파일 상태 추가 시 누락되지 않도록 주의가 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 76 - 80, Update the onOpenCreate handler to also reset file-related state so the modal opens with a clean form: in the onOpenCreate function (alongside setNewName(""), setNewDesc(""), setCreateOpen(true)), call the file state setters (e.g., setLogoFile(null) and setLogoPreview("") or the equivalents used for logoFile/logoPreview) to ensure any previously selected file or preview is cleared when the create modal is opened.
33-55: 정적 데이터에useMemo를 사용할 필요가 없습니다
workspaces배열은 의존성이 없는 정적 상수이므로useMemo를 사용하는 것은 불필요하고 오히려 혼란을 줍니다. API 연동 전의 목업 데이터라면 컴포넌트 외부의 모듈 레벨 상수로 선언하거나, API 연동 시점에useQuery로 대체하는 것이 적절합니다.💡 개선 예시
+ // 컴포넌트 외부 (모듈 레벨) + const MOCK_WORKSPACES: TWorkspace[] = [ + { id: "1", name: "광고회사1", description: "광고회사1입니다.", myRole: "admin" }, + { id: "2", name: "광고회사2", description: "광고회사2입니다.", myRole: "admin" }, + { id: "3", name: "광고회사3", description: "광고회사3입니다.", myRole: "admin" }, + ]; export default function WorkspacePage() { // ... - const workspaces: TWorkspace[] = useMemo( - () => [...], - [], - ); + const workspaces = MOCK_WORKSPACES;As per coding guidelines,
useCallback,useMemo의 적절한 사용을 검토하도록 명시되어 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 33 - 55, The workspaces constant is static yet wrapped in useMemo; remove useMemo and define the array as a plain constant (e.g. exportable module-level constant or a local const named workspaces: TWorkspace[]) or replace with a data-fetching hook when wiring the API, ensuring you update any references inside the Workspace component that currently rely on the useMemo result (look for the useMemo(...) that returns the workspaces array and the TWorkspace type usage).
62-75:menuItems함수가 매 렌더마다 새 배열을 생성합니다
menuItems는 컴포넌트 내부에 일반 함수로 선언되어 있어 렌더마다 재생성됩니다.WorkspaceCard에React.memo가 적용될 경우, prop 참조가 매번 달라져 메모이제이션 효과가 없어집니다.
useCallback으로 감싸거나, 가능하다면menuItems생성 로직을WorkspaceCard내부 또는 커스텀 훅으로 이동하는 것을 권장합니다.💡 useCallback 적용 예시
- const menuItems = (id: TWorkspace["id"]): TMenuItem[] => [ - { ... }, - { ... }, - ]; + const menuItems = useCallback( + (id: TWorkspace["id"]): TMenuItem[] => [ + { + icon: <EditContainIcon className="h-5 w-5 fill-none stroke-current" />, + label: "정보 수정하기", + onClick: () => { void navigate(`/workspace/${id}/settings`); }, + }, + { + icon: <UserProfileIcon className="h-5 w-5 fill-none stroke-current" />, + label: "멤버 관리", + onClick: () => alert("멤버 관리 기능은 추후 연결 예정"), + }, + ], + [navigate], + );As per coding guidelines,
useCallback,useMemo의 적절한 사용을 검토하도록 명시되어 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` around lines 62 - 75, The menuItems function is recreated on every render causing unstable props for memoized children; wrap the menuItems factory in a stable hook (e.g., useCallback or useMemo) or move its creation into WorkspaceCard or a custom hook so the reference stays stable; specifically, change the current menuItems (id: TWorkspace["id"]): TMenuItem[] => [...] to a memoized callback that captures navigate (or pass navigate into the custom hook/WorkspaceCard) so WorkspaceCard receives a consistent prop and React.memo can work as intended.
🤖 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/input/Input.tsx`:
- Around line 56-59: The success branch of the class expression for the Input
component currently uses the same classes as the default state and includes an
unnecessary border-transparent; update the class string used when the success
prop is true (the conditional that checks success) to provide distinct visual
feedback (for example use a subtle green background and green ring like
bg-status-green/5 and ring-1 ring-status-green/40 or similar) and remove the
unused border-transparent utility; modify the conditional inside the Input
component where success is evaluated so the success branch emits the new green
classes instead of "bg-white ring-1 ring-logo-1/30".
In `@src/pages/workspace/Workspace.tsx`:
- Around line 28-32: The openFile function does a null-check on fileRef.current
then uses optional chaining on the same reference; remove the unnecessary ?. and
call the method consistently — replace fileRef.current?.click() with
fileRef.current.click() (keeping the preceding if (!fileRef.current) return) so
both the .value and .click accesses are consistent and non-optional; ensure you
update the openFile function accordingly.
- Around line 102-108: The search Input currently uses only a placeholder
(component Input with props value={query}, onChange={(e) =>
setQuery(e.target.value)}, rightElement={<SearchIcon .../>}) which is not
accessible; add an accessible name by providing an aria-label (e.g.,
aria-label="조직 검색") or add a visually hidden <label> tied to the same input id
to ensure screen readers can announce the input purpose; update the Input usage
to include the aria-label or id/label pairing while leaving other props (value,
onChange, rightElement) unchanged.
- Around line 82-85: The onPickFile handler in Workspace does not store the
chosen file, so add state (e.g., const [selectedFile, setSelectedFile] =
useState<File | null>(null) and const [previewUrl, setPreviewUrl] =
useState<string | null>(null)) and update onPickFile to
setSelectedFile(e.target.files?.[0]) and create a preview URL via
URL.createObjectURL(...) saved into previewUrl; also ensure you revoke the
object URL on component unmount/update (useEffect cleanup) and use selectedFile
when calling the upload API so the picked file is not lost.
---
Outside diff comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 26-35: DropdownMenu is missing Escape handling: when the menu
(role="menu") is open pressing Escape must close it and return focus to the
trigger. Add a keydown handler that listens for "Escape" while the menu is open
(either on the document or the menu root) and calls setOpen(false) and moves
focus back to the trigger element (the same element used by the trigger
onKeyDown handler); update the existing trigger onKeyDown to also handle Escape
when the menu is open by calling setOpen(false) and focusing the trigger, and
ensure you register/removing the event listener in the same effect used for
outside clicks (referencing ref, setOpen and the trigger DOM ref/name used in
this component) so cleanup happens correctly.
---
Duplicate comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Line 1: The DropdownMenu component should consistently avoid duplicate IDs and
support accessibility props: ensure every instance uses React's useId to
generate menuId (e.g., const menuId = useId()), apply that menuId to the
trigger's aria-controls and the menu's id, and accept/forward an aria-label prop
from DropdownMenu to the trigger/menu; update any other occurrences (the
repeated blocks referenced around the other ranges) to the same pattern so all
triggers use id={menuId}, aria-controls={menuId} and the menu element has
id={menuId}, and propagate aria-label through to WorkspaceCard integrations.
In `@src/components/common/input/Input.tsx`:
- Around line 42-50: The spacing issue between the label and input has been
resolved by adding margin-bottom to the label; verify that the <label> element
(using inputId) includes className "mb-1" and that the container still uses
twMerge with wrapperClassName (in Input.tsx) so no gap-2 or mb-2 classes are
present; if missing, add "mb-1" to the label's className to restore the vertical
spacing and keep the surrounding wrapper classes unchanged.
In `@src/pages/workspace/Workspace.tsx`:
- Around line 133-203: Confirm the modal/title duplication and related UI fixes
by removing any leftover title prop on the Modal component and keeping the
internal <h2> header in Workspace.tsx (verify Modal usage only uses
createOpen/onClose/size/padding props), ensure WorkspaceCard now handles
w.logoUrl with conditional <img> rendering (no direct string render of
w.logoUrl), verify the workspace list uses semantic <ul> with WorkspaceCard
rendered as <li>, and confirm the "워크스페이스 관리" spacing/text is consistent and
that create flow uses newName/newDesc and onSubmitCreate as shown.
---
Nitpick comments:
In `@src/pages/workspace/Workspace.tsx`:
- Around line 76-80: Update the onOpenCreate handler to also reset file-related
state so the modal opens with a clean form: in the onOpenCreate function
(alongside setNewName(""), setNewDesc(""), setCreateOpen(true)), call the file
state setters (e.g., setLogoFile(null) and setLogoPreview("") or the equivalents
used for logoFile/logoPreview) to ensure any previously selected file or preview
is cleared when the create modal is opened.
- Around line 33-55: The workspaces constant is static yet wrapped in useMemo;
remove useMemo and define the array as a plain constant (e.g. exportable
module-level constant or a local const named workspaces: TWorkspace[]) or
replace with a data-fetching hook when wiring the API, ensuring you update any
references inside the Workspace component that currently rely on the useMemo
result (look for the useMemo(...) that returns the workspaces array and the
TWorkspace type usage).
- Around line 62-75: The menuItems function is recreated on every render causing
unstable props for memoized children; wrap the menuItems factory in a stable
hook (e.g., useCallback or useMemo) or move its creation into WorkspaceCard or a
custom hook so the reference stays stable; specifically, change the current
menuItems (id: TWorkspace["id"]): TMenuItem[] => [...] to a memoized callback
that captures navigate (or pass navigate into the custom hook/WorkspaceCard) so
WorkspaceCard receives a consistent prop and React.memo can work as intended.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/assets/icon/workspace/edit-contained.svgis excluded by!**/*.svgand included bysrc/**src/assets/icon/workspace/userProfile.svgis excluded by!**/*.svgand included bysrc/**
📒 Files selected for processing (4)
src/components/common/dropdownmenu/DropdownMenu.tsxsrc/components/common/input/Input.tsxsrc/components/workspace/WorkspaceCard.tsxsrc/pages/workspace/Workspace.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/workspace/WorkspaceCard.tsx
Seojegyeong
approved these changes
Feb 25, 2026
YermIm
approved these changes
Feb 25, 2026
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
3 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.
🚨 관련 이슈
Closed #50
✨ 변경사항
✏️ 작업 내용
😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
사이드바 제외하고 페이지 부분에서 양옆 padding을 다같이 맞추면 좋을것같아요. 저는 일단
py-8 px-20이렇게 지정해놓았는데, 다른 분들 어떻게 하셨는지 궁금합니다!=>MainLayout에
max-w-[1600px] py-6 px-4 sm:px-6 lg:px-20적용해두었습니다Summary by CodeRabbit
Summary by CodeRabbit
릴리스 노트
New Features
Style
Chores