[Refactor/#79] 워크스페이스 목록 로딩/에러 UI 랜더링 구조 개선 - #89
Merged
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 (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough워크스페이스 목록의 UI 상태 관리를 위해 로딩/에러/빈 상태를 담당하는 세 개의 프레젠테이션 컴포넌트를 추가하고, Workspace 페이지의 목록 렌더링 로직을 하나의 함수로 통합하며 상태 변수명을 더 명확하게 바꿨습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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 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 |
📚 Storybook 배포 완료
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/components/workspace/WorkspaceListLoading.tsx (1)
1-9: 접근성: 로딩 상태에 대한 스크린 리더 지원 고려현재 로딩 메시지가 정적 텍스트로만 표시되어 있어서, 스크린 리더 사용자에게 로딩 상태가 명확히 전달되지 않을 수 있습니다.
role="status"와aria-live="polite"속성을 추가하면 접근성이 개선됩니다.또한, 시각적 로딩 인디케이터(스피너 등)를 추가하면 사용자 경험이 향상될 수 있습니다.
♿ 접근성 개선 제안
export default function WorkspaceListLoading() { return ( - <div className="bg-white p-10 text-center border border-gray-100 rounded-component-lg"> + <div + role="status" + aria-live="polite" + className="bg-white p-10 text-center border border-gray-100 rounded-component-lg" + > <p className="font-body2 text-text-sub"> 워크스페이스 목록을 불러오는 중입니다. </p> </div> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/WorkspaceListLoading.tsx` around lines 1 - 9, The WorkspaceListLoading component currently renders static text that isn't announced by screen readers; update the WorkspaceListLoading function to add accessibility attributes (e.g., set role="status" and aria-live="polite" on the outer div) so assistive tech notifies users of the loading state, and add a visible loading indicator (spinner or animated element) alongside the paragraph text to improve UX; keep these changes within the WorkspaceListLoading component and ensure the spinner has appropriate aria-hidden or descriptive text as needed.src/components/workspace/WorkspaceListError.tsx (1)
12-19: 접근성: 에러 상태 알림을 위한 ARIA 속성 추가 권장에러 메시지가 표시될 때 스크린 리더가 사용자에게 즉시 알릴 수 있도록
role="alert"속성을 추가하면 좋습니다. 이렇게 하면 에러 발생 시 보조 기술이 자동으로 메시지를 읽어줍니다.♿ 접근성 개선 제안
return ( - <div className="bg-white p-10 text-center border border-gray-100 rounded-component-lg space-y-4"> + <div + role="alert" + className="bg-white p-10 text-center border border-gray-100 rounded-component-lg space-y-4" + > <p className="font-body2 text-status-red">{message}</p> <Button type="button" variant="primary" onClick={onRetry}> 다시 시도 </Button> </div> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/WorkspaceListError.tsx` around lines 12 - 19, The error container in the WorkspaceListError component should notify assistive tech immediately; add an ARIA alert to the root element (the div that currently wraps message and Button) by adding role="alert" (and optionally aria-live="assertive") so the screen reader announces the {message} when the error renders; update the JSX in WorkspaceListError (the div around message and the Button with onClick={onRetry}) to include this attribute.src/pages/workspace/Workspace.tsx (1)
47-47: UX 고려: isFetching 포함 시 백그라운드 갱신 중에도 로딩 UI 표시
isListLoading이isFetching을 포함하고 있어서, 백그라운드에서 데이터를 다시 가져올 때(예: 창 포커스 복귀, 수동 refetch)에도 로딩 UI가 표시됩니다. 이로 인해 이미 데이터가 있는 상태에서 잠깐 로딩 화면으로 깜빡이는 현상이 발생할 수 있습니다.초기 로딩 시에만 로딩 UI를 보여주고 싶다면
isLoading만 사용하거나, React Query v5의 경우isPending && !data패턴을 고려해 보세요.💡 개선 옵션
- const isListLoading = workspacesQuery.isLoading || workspacesQuery.isFetching; + // 초기 로딩 시에만 로딩 UI 표시 (백그라운드 refetch 시에는 기존 데이터 유지) + const isListLoading = workspacesQuery.isLoading;또는 데이터가 없을 때만 로딩 표시:
- const isListLoading = workspacesQuery.isLoading || workspacesQuery.isFetching; + const isListLoading = workspacesQuery.isPending && !workspacesQuery.data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/workspace/Workspace.tsx` at line 47, The current isListLoading combines workspacesQuery.isLoading and workspacesQuery.isFetching which shows the loading UI during background refetches; change the isListLoading computation in Workspace.tsx (the const isListLoading = ...) to only use workspacesQuery.isLoading (or, if using React Query v5, use workspacesQuery.isPending && !workspacesQuery.data) or alternatively include a data check (e.g., workspacesQuery.isFetching && !workspacesQuery.data) so the loading UI appears only when there is no existing data.src/components/workspace/WorkspaceEmptyState.tsx (1)
8-11: 구조 일관성: 다른 상태 컴포넌트들과의 렌더링 구조 차이
WorkspaceEmptyState는<li>요소로 렌더링되고,WorkspaceListLoading과WorkspaceListError는<div>로 렌더링됩니다.Workspace.tsx의renderWorkspaceContent에서 빈 상태일 때만<ul>로 감싸고 있어서 구조가 일관되지 않습니다.현재 동작에는 문제가 없지만, 향후 유지보수나 스타일 일관성을 위해 모든 상태 컴포넌트의 루트 요소 타입을 통일하는 것을 고려해 보세요.
♻️ 구조 통일 제안 (선택적)
Option 1: EmptyState를 div로 변경
export default function WorkspaceEmptyState({ message, }: IWorkspaceEmptyStateProps) { return ( - <li className="rounded-component-lg bg-white p-10 text-center border border-gray-100"> + <div className="rounded-component-lg bg-white p-10 text-center border border-gray-100"> <p className="font-body2 text-text-sub">{message}</p> - </li> + </div> ); }그리고 Workspace.tsx에서
<ul>래퍼 제거🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/workspace/WorkspaceEmptyState.tsx` around lines 8 - 11, WorkspaceEmptyState currently renders a <li> while WorkspaceListLoading and WorkspaceListError render <div>, causing inconsistent DOM structure in renderWorkspaceContent; change WorkspaceEmptyState's root element from <li> to <div> (in the WorkspaceEmptyState component) and remove the special-case <ul> wrapper in Workspace.tsx so all status components render the same root type, keeping existing classes/ styling on the new <div> and updating any tests or ARIA/semantic wrappers that expected an <li>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/workspace/WorkspaceEmptyState.tsx`:
- Around line 8-11: WorkspaceEmptyState currently renders a <li> while
WorkspaceListLoading and WorkspaceListError render <div>, causing inconsistent
DOM structure in renderWorkspaceContent; change WorkspaceEmptyState's root
element from <li> to <div> (in the WorkspaceEmptyState component) and remove the
special-case <ul> wrapper in Workspace.tsx so all status components render the
same root type, keeping existing classes/ styling on the new <div> and updating
any tests or ARIA/semantic wrappers that expected an <li>.
In `@src/components/workspace/WorkspaceListError.tsx`:
- Around line 12-19: The error container in the WorkspaceListError component
should notify assistive tech immediately; add an ARIA alert to the root element
(the div that currently wraps message and Button) by adding role="alert" (and
optionally aria-live="assertive") so the screen reader announces the {message}
when the error renders; update the JSX in WorkspaceListError (the div around
message and the Button with onClick={onRetry}) to include this attribute.
In `@src/components/workspace/WorkspaceListLoading.tsx`:
- Around line 1-9: The WorkspaceListLoading component currently renders static
text that isn't announced by screen readers; update the WorkspaceListLoading
function to add accessibility attributes (e.g., set role="status" and
aria-live="polite" on the outer div) so assistive tech notifies users of the
loading state, and add a visible loading indicator (spinner or animated element)
alongside the paragraph text to improve UX; keep these changes within the
WorkspaceListLoading component and ensure the spinner has appropriate
aria-hidden or descriptive text as needed.
In `@src/pages/workspace/Workspace.tsx`:
- Line 47: The current isListLoading combines workspacesQuery.isLoading and
workspacesQuery.isFetching which shows the loading UI during background
refetches; change the isListLoading computation in Workspace.tsx (the const
isListLoading = ...) to only use workspacesQuery.isLoading (or, if using React
Query v5, use workspacesQuery.isPending && !workspacesQuery.data) or
alternatively include a data check (e.g., workspacesQuery.isFetching &&
!workspacesQuery.data) so the loading UI appears only when there is no existing
data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42d5b586-8f65-4320-a0e5-7f308385209e
📒 Files selected for processing (4)
src/components/workspace/WorkspaceEmptyState.tsxsrc/components/workspace/WorkspaceListError.tsxsrc/components/workspace/WorkspaceListLoading.tsxsrc/pages/workspace/Workspace.tsx
📚 Storybook 배포 완료
|
Collaborator
|
P4: 확인했습니다! 수고하셨습니다!! |
YermIm
approved these changes
Mar 11, 2026
Seojegyeong
approved these changes
Mar 11, 2026
Seojegyeong
left a comment
Seojegyeong
left a comment
Collaborator
There was a problem hiding this comment.
P4: 확인했습니다. 수고하셨습니다!
9 tasks
9 tasks
9 tasks
9 tasks
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 #79
✨ 변경사항
✏️ 작업 내용
워크스페이스 목록 조회시 로딩/에러 UI를 별도 컴포넌트로 분리작업 진행
워크스페이스 빈 상태 UI를 별도 컴포넌트로 분리작업 진행
상태 관련 변수명 변경(listLoading -> isListLoading, creating -> isCreating)
작업 내용
😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
N/A
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항