-
Notifications
You must be signed in to change notification settings - Fork 0
[DESIGN] 비동기 처리 관련 컴포넌트 개선 #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough이번 변경에서는 비동기 처리 및 에러, 로딩 상태를 위한 UI 컴포넌트들이 추가 및 개선되었습니다. 주요 컴포넌트에 disabled 및 skeleton(로딩) 상태 지원이 도입되었고, Storybook 스토리 파일이 대거 추가되었습니다. 일부 컴포넌트의 Tailwind CSS 클래스 순서가 정돈되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UIComponent
participant Skeleton
participant ErrorIndicator
participant LoadingIndicator
User->>UIComponent: 데이터 요청/상호작용
alt 로딩 중
UIComponent->>Skeleton: 로딩 상태 skeletonEnabled=true
Skeleton-->>User: Skeleton UI 표시
else 에러 발생
UIComponent->>ErrorIndicator: 에러 메시지 및 onClickRetry 전달
ErrorIndicator-->>User: 에러 및 재시도 버튼 표시
else 정상 응답
UIComponent-->>User: 정상 데이터 표시
end
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/components/LabledCheckBox/LabeledCheckbox.tsx (1)
26-48
: 체크박스 입력 요소의 disabled 상태 전달 필요현재 구현에서
disabled
prop이 input 요소에 전달되지 않아 실제로 체크박스가 비활성화되지 않습니다.다음 diff를 적용하여 수정하세요:
<input {...rest} type="checkbox" checked={checked} + disabled={disabled} className={` relative h-5 - w-5 cursor-pointer + w-5 + ${disabled ? '' : 'cursor-pointer'} appearance-none rounded-sm border border-neutral-300 checked:border-transparent checked:bg-blue-500 checked:before:absolute checked:before:left-1/2 checked:before:top-[-4px] checked:before:-translate-x-1/2 checked:before:text-xl checked:before:font-bold checked:before:text-background-default checked:before:content-['✓'] focus:outline-none `}
🧹 Nitpick comments (6)
src/components/ErrorBoundary/ErrorPage.tsx (1)
29-29
: Tailwind 클래스 순서 변경 확인 완료
text-4xl md:text-5xl
순서 조정만 이루어졌으며 시각적·기능적 영향은 없습니다. 추후 동일한 단순 변경이 반복되지 않도록tailwindcss-classnames-order
같은 플러그인을 ESLint에 연동해 자동 정렬을 고려해도 좋겠습니다.src/components/Skeleton/Skeleton.stories.tsx (1)
4-8
: 스토리 제목의 대소문자 일관성을 확인해주세요.다른 스토리 파일들은
Components/ComponentName
형식을 사용하는데, 여기서는components/Skeleton
을 사용하고 있습니다. 일관성을 위해Components/Skeleton
으로 변경하는 것을 고려해보세요.title: 'Components/Skeleton',src/components/ErrorIndicator/ErrorIndicator.tsx (2)
17-24
: 재시도 버튼 렌더링 로직을 개선해보세요.현재
onClick={() => onClickRetry()}
형태로 불필요한 익명 함수를 생성하고 있습니다.다음과 같이 직접 전달하는 것이 더 효율적입니다:
- onClick={() => onClickRetry()} + onClick={onClickRetry}
13-25
: 접근성 개선을 고려해보세요.에러 상태를 시각적으로만 표현하고 있어 스크린 리더 사용자에게 불편할 수 있습니다.
다음과 같이 접근성을 개선할 수 있습니다:
- <div className="flex h-full w-full flex-col items-center justify-center space-y-4 p-16"> + <div className="flex h-full w-full flex-col items-center justify-center space-y-4 p-16" role="alert" aria-live="polite"> <MdErrorOutline className="size-32 text-red-500" /> - <p className="break-keep text-center text-xl">{children}</p> + <p className="break-keep text-center text-xl" id="error-message">{children}</p> {onClickRetry && ( <button onClick={onClickRetry} className="small-button enabled px-8 py-1" + aria-describedby="error-message" > 다시 시도하기 </button> )} </div>src/components/HeaderTableInfo/HeaderTableInfo.tsx (1)
13-26
: 조건부 렌더링 구조를 단순화할 수 있습니다.현재 두 개의 조건부 블록으로 나누어져 있는데, 하나의 조건부 렌더링으로 간소화할 수 있습니다.
다음과 같이 개선할 수 있습니다:
- <> - {isLoading && ( - <div className="flex flex-col space-y-[4px]"> - <Skeleton width={120} height={16} /> - <Skeleton width={360} height={24} /> - </div> - )} - {!isLoading && ( - <div className="flex flex-col space-y-[4px]"> - <h1 className="text-sm">테이블 이름</h1> - <h1 className="text-2xl font-bold">{displayName}</h1> - </div> - )} - </> + <div className="flex flex-col space-y-[4px]"> + {isLoading ? ( + <> + <Skeleton width={120} height={16} /> + <Skeleton width={360} height={24} /> + </> + ) : ( + <> + <h1 className="text-sm">테이블 이름</h1> + <h1 className="text-2xl font-bold">{displayName}</h1> + </> + )} + </div>src/components/HeaderTitle/HeaderTitle.tsx (1)
13-24
: 조건부 렌더링 구조를 단순화할 수 있습니다.HeaderTableInfo와 동일한 패턴으로 조건부 렌더링을 개선할 수 있습니다.
다음과 같이 개선할 수 있습니다:
- <> - {isLoading && ( - <div className="flex items-center justify-center"> - <Skeleton width={540} height={32} /> - </div> - )} - {!isLoading && ( - <h1 className="w-full max-w-[50vw] overflow-hidden text-ellipsis whitespace-nowrap text-3xl font-bold"> - {displayTitle} - </h1> - )} - </> + {isLoading ? ( + <div className="flex items-center justify-center"> + <Skeleton width={540} height={32} /> + </div> + ) : ( + <h1 className="w-full max-w-[50vw] overflow-hidden text-ellipsis whitespace-nowrap text-3xl font-bold"> + {displayTitle} + </h1> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/components/ClearableInput/ClearableInput.stories.tsx
(1 hunks)src/components/ClearableInput/ClearableInput.tsx
(1 hunks)src/components/ErrorBoundary/ErrorPage.tsx
(1 hunks)src/components/ErrorIndicator/ErrorIndicator.stories.tsx
(1 hunks)src/components/ErrorIndicator/ErrorIndicator.tsx
(1 hunks)src/components/HeaderTableInfo/HeaderTableInfo.tsx
(1 hunks)src/components/HeaderTitle/HeaderTitle.tsx
(1 hunks)src/components/LabledCheckBox/LabeledCheckbox.stories.tsx
(1 hunks)src/components/LabledCheckBox/LabeledCheckbox.tsx
(2 hunks)src/components/LoadingIndicator/LoadingIndicator.stories.tsx
(1 hunks)src/components/LoadingIndicator/LoadingIndicator.tsx
(1 hunks)src/components/Skeleton/Skeleton.stories.tsx
(1 hunks)src/components/Skeleton/Skeleton.tsx
(1 hunks)src/page/TableComposition/components/TableNameAndType/TableNameAndType.tsx
(4 hunks)src/page/TimerPage/components/FirstUseToolTip.tsx
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/components/LabledCheckBox/LabeledCheckbox.stories.tsx (1)
src/components/ClearableInput/ClearableInput.stories.tsx (1)
Disabled
(23-30)
src/components/ClearableInput/ClearableInput.stories.tsx (1)
src/components/LabledCheckBox/LabeledCheckbox.stories.tsx (1)
Disabled
(42-48)
src/components/HeaderTableInfo/HeaderTableInfo.tsx (1)
src/components/Skeleton/Skeleton.tsx (1)
Skeleton
(11-20)
src/components/HeaderTitle/HeaderTitle.tsx (1)
src/components/Skeleton/Skeleton.tsx (1)
Skeleton
(11-20)
src/components/Skeleton/Skeleton.stories.tsx (1)
src/components/Skeleton/Skeleton.tsx (1)
Skeleton
(11-20)
src/components/LoadingIndicator/LoadingIndicator.stories.tsx (2)
src/components/LoadingIndicator/LoadingIndicator.tsx (1)
LoadingIndicator
(4-13)src/components/ErrorIndicator/ErrorIndicator.stories.tsx (1)
Default
(14-18)
🔇 Additional comments (30)
src/page/TimerPage/components/FirstUseToolTip.tsx (3)
24-24
: 클래스 순서 통일 확인 – 문제 없음
변경 사항이 스타일 결과에 영향을 주지 않습니다.
44-44
: 같은 패턴의 순서 교정 확인
일관성 유지 목적임을 확인했습니다.
62-62
: 클래스 정렬 유지
시각·동작 변화 없으며 승인합니다.src/page/TableComposition/components/TableNameAndType/TableNameAndType.tsx (5)
54-54
:section
패딩 클래스 재정렬 확인
p-6 md:p-8
순으로 조정된 것만 확인되었습니다.
55-55
: 라벨 텍스트 크기 클래스 순서 수정
flex … md:text-2xl
형태로 통일된 것 확인, 문제 없습니다.
65-65
: 두 번째 라벨 동일 수정
동일 패턴의 순서 변경으로 승인합니다.
75-75
: 팀명 라벨 클래스 교정
기능 영향 없으며 승인합니다.
105-105
: 종소리 설정 라벨 클래스 교정
일관성 확보 차원의 수정으로 승인합니다.src/components/LoadingIndicator/LoadingIndicator.tsx (1)
1-13
: 컴포넌트 구조가 잘 설계되었습니다.
LoadingIndicator
컴포넌트가 잘 구현되어 있습니다. 한국어 기본 메시지와 함께 재사용 가능한 구조로 되어 있고, Tailwind CSS를 사용한 스타일링도 적절합니다.src/components/Skeleton/Skeleton.tsx (3)
1-4
: 인터페이스 정의가 명확합니다.
SkeletonProps
인터페이스가 선택적 속성으로 잘 정의되어 있습니다.
6-10
: JSDoc 문서화가 잘 되어 있습니다.컴포넌트의 목적과 매개변수에 대한 한국어 설명이 명확하고 자세합니다.
11-20
: 스켈레톤 UI 구현이 적절합니다.기본값 설정과 동적 크기 조정을 위한 인라인 스타일 사용이 적절합니다. Tailwind CSS의 애니메이션 클래스와 함께 잘 구현되어 있습니다.
src/components/LoadingIndicator/LoadingIndicator.stories.tsx (1)
1-18
: Storybook 스토리 구성이 표준을 잘 따르고 있습니다.
LoadingIndicator
컴포넌트의 Storybook 스토리가 적절히 구성되어 있습니다. 메타데이터 설정과 기본 스토리 정의가 표준 패턴을 따르고 있으며, 한국어 텍스트도 적절합니다.src/components/Skeleton/Skeleton.stories.tsx (1)
14-16
: 기본 스토리 구성이 적절합니다.기본값을 가진 컴포넌트에 대해 빈 args를 사용하는 것이 적절합니다.
src/components/ErrorIndicator/ErrorIndicator.stories.tsx (1)
1-25
: ErrorIndicator 스토리 구성이 포괄적이고 잘 설계되었습니다.두 가지 스토리를 통해 컴포넌트의 기본 상태와 재시도 버튼이 활성화된 상태를 모두 보여주어 좋은 커버리지를 제공합니다. 한국어 메시지도 프로젝트 맥락에 적절합니다.
src/components/ErrorIndicator/ErrorIndicator.tsx (2)
4-6
: 인터페이스 설계가 잘 되어 있습니다.
PropsWithChildren
을 확장하여 유연한 메시지 전달을 가능하게 하고, 선택적 재시도 콜백을 제공하는 깔끔한 인터페이스입니다.
8-11
: 기본값 설정이 적절합니다.사용자 친화적인 한국어 기본 메시지가 설정되어 있어 좋습니다. 구조 분해 할당을 통한 props 처리도 깔끔합니다.
src/components/HeaderTableInfo/HeaderTableInfo.tsx (4)
1-1
: Skeleton 컴포넌트 import가 적절합니다.상대 경로를 사용한 import가 올바르게 설정되어 있습니다.
5-5
: 스켈레톤 상태를 위한 props 추가가 좋습니다.선택적 boolean 타입의
skeletonEnabled
prop으로 로딩 상태를 제어할 수 있게 한 것이 적절합니다.
9-9
: props 변수명 변경이 의미있습니다.
skeletonEnabled
를isLoading
으로 재명명한 것이 컴포넌트 내부에서 의도를 더 명확하게 표현합니다.
14-19
: 스켈레톤 레이아웃이 실제 콘텐츠와 잘 맞습니다.두 개의 스켈레톤 요소가 실제 헤더 구조(작은 제목 + 큰 제목)와 일치하는 크기로 설정되어 있어 자연스러운 로딩 경험을 제공합니다.
src/components/HeaderTitle/HeaderTitle.tsx (4)
1-1
: Skeleton 컴포넌트 import가 적절합니다.일관된 상대 경로를 사용한 import가 올바르게 설정되어 있습니다.
5-5
: 일관된 스켈레톤 상태 props 설계입니다.HeaderTableInfo와 동일한
skeletonEnabled
prop 패턴을 사용하여 일관성을 유지한 것이 좋습니다.
15-17
: 스켈레톤 크기와 정렬이 적절합니다.540x32px 크기의 스켈레톤이 제목 영역에 적합하고, 중앙 정렬로 시각적 일관성을 유지하고 있습니다.
1-26
: 스켈레톤 패턴 일관성 확인 요청아래 두 컴포넌트의 로딩 상태 시 렌더링되는 스켈레톤 UI가 서로 다른 형태로 구현되어 있습니다.
디자인 의도상 이 차이가 맞는지, 또는 공통 가이드라인에 맞춰 통일이 필요한지 검토해주세요.
HeaderTitle (src/components/HeaderTitle/HeaderTitle.tsx)
- 로딩 시:
<div className="flex items-center justify-center">
Skeleton width={540} height={32}
(단일 placeholder)HeaderTableInfo (src/components/HeaderTableInfo/HeaderTableInfo.tsx)
- 로딩 시:
<div className="flex flex-col space-y-[4px]">
Skeleton width={120} height={16}
Skeleton width={360} height={24}
(두 개의 placeholder)위 차이가 의도된 것인지 확인 후, 필요 시 스켈레톤 스타일을 통일하거나 디자인 팀에 문의 부탁드립니다.
src/components/ClearableInput/ClearableInput.tsx (1)
6-6
: disabled prop 추가가 적절합니다.비동기 처리 중 사용자 입력을 방지하기 위한 disabled 상태 지원이 잘 구현되었습니다.
src/components/ClearableInput/ClearableInput.stories.tsx (1)
23-30
: Disabled 상태 스토리 추가가 적절합니다.disabled 상태에 대한 시각적 문서화와 테스트 커버리지를 제공하는 좋은 추가입니다.
src/components/LabledCheckBox/LabeledCheckbox.tsx (2)
6-6
: disabled prop 추가가 적절합니다.비동기 처리 중 사용자 상호작용을 방지하기 위한 disabled 상태 지원이 잘 추가되었습니다.
15-17
: 주석 개선이 명확합니다.disabled 상태에 따른 라벨 색상 로직을 명확하게 설명하는 주석으로 개선되었습니다.
src/components/LabledCheckBox/LabeledCheckbox.stories.tsx (1)
42-48
: Disabled 상태 스토리 추가가 적절합니다.disabled 상태에 대한 시각적 문서화를 제공하는 좋은 추가입니다. 한국어 라벨도 일관성 있게 작성되었습니다.
className={` | ||
w-full rounded-md border border-neutral-300 p-3 pr-10 text-base text-neutral-900 placeholder-neutral-400 focus:outline-none | ||
${disabled ? 'bg-neutral-400' : ''} | ||
`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
입력 요소의 disabled 상태 전달 및 clear 버튼 비활성화 필요
현재 구현에서 다음 문제들이 있습니다:
disabled
prop이 input 요소에 명시적으로 전달되지 않음- disabled 상태에서도 clear 버튼이 여전히 작동함
다음 diff를 적용하여 수정하세요:
<input
{...rest}
value={value}
+ disabled={disabled}
className={`
w-full rounded-md border border-neutral-300 p-3 pr-10 text-base text-neutral-900 placeholder-neutral-400 focus:outline-none
${disabled ? 'bg-neutral-400' : ''}
`}
/>
{value && (
<button
type="button"
onClick={onClear}
+ disabled={disabled}
className="absolute right-3 top-1/2 -translate-y-1/2 text-xl text-neutral-400 hover:text-neutral-600"
>
🤖 Prompt for AI Agents
In src/components/ClearableInput/ClearableInput.tsx around lines 21 to 24, the
input element does not explicitly receive the disabled prop, and the clear
button remains active even when disabled. Fix this by passing the disabled prop
directly to the input element and conditionally disabling the clear button when
disabled is true, ensuring the button is non-interactive in that state.
<label | ||
className={`md:text-base flex cursor-pointer items-center gap-2 text-sm ${labelColorClass}`} | ||
className={` | ||
flex cursor-pointer items-center gap-2 text-sm md:text-base | ||
${disabled ? 'text-neutral-400' : 'text-neutral-900'} | ||
`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
라벨 커서 상태 개선 필요
disabled 상태에서 라벨의 cursor-pointer
클래스가 여전히 적용되어 클릭 가능해 보입니다.
다음 diff를 적용하여 수정하세요:
className={`
- flex cursor-pointer items-center gap-2 text-sm md:text-base
+ flex items-center gap-2 text-sm md:text-base
+ ${disabled ? 'text-neutral-400' : 'text-neutral-900 cursor-pointer'}
- ${disabled ? 'text-neutral-400' : 'text-neutral-900'}
`}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<label | |
className={`md:text-base flex cursor-pointer items-center gap-2 text-sm ${labelColorClass}`} | |
className={` | |
flex cursor-pointer items-center gap-2 text-sm md:text-base | |
${disabled ? 'text-neutral-400' : 'text-neutral-900'} | |
`} | |
<label | |
className={` | |
flex items-center gap-2 text-sm md:text-base | |
${disabled ? 'text-neutral-400' : 'text-neutral-900 cursor-pointer'} | |
`} |
🤖 Prompt for AI Agents
In src/components/LabledCheckBox/LabeledCheckbox.tsx between lines 20 and 24,
the label element always has the 'cursor-pointer' class even when disabled,
which misleads users into thinking it is clickable. Modify the className logic
to conditionally apply 'cursor-pointer' only when not disabled, removing it when
disabled to reflect the correct cursor state.
🚩 연관 이슈
closed #323
📝 작업 내용
🔔 알립니다
현재 이슈 #310 코드가 너무 많아서, 팀원 여러분께서 리뷰를 조금 더 편하게 하실 수 있도록 PR을 더 세부적으로 쪼개고 있습니다. 총 3개 PR로 나누어질 예정이며, 이 PR은 2번째 PR입니다.
문제 상황
개선 사항
스켈레톤 UI 도입
스켈레톤 UI는 위 그림처럼 비동기로 데이터 패칭을 요청한 후 로딩 중일 때, 실제 데이터가 렌더링 될 화면의 윤곽을 보여주는 애니메이션 UI입니다. (출처)
로딩 중 시나리오에 대응하기 위해, 스켈레톤 UI의 기본이 될
Skeleton
컴포넌트를 새로 추가하였고, 해당 컴포넌트를 기존의HeaderTitle
과HeaderTableInfo
에 적용하여 사용자가 데이터가 로딩 중임을 알 수 있게 개선했습니다.기존 컴포넌트가 비활성화될 수 있게 개선
LabledCheckBox
와ClearableInput
은 아쉽게도 비활성화(disabled)를 지원하고 있진 않았습니다. 마찬가지로 로딩 중 시나리오에 대응하기 위해, 로딩 중에는 입력 칸과 체크박스가 비활성화되도록 기존 컴포넌트를 개선하였습니다.Windows 바이너리에서 로딩 및 오류를 안내하는 컴포넌트 가져옴
debate-timer-fe-win에서 추가했던
ErrorIndicator
와LoadingIndicator
를 가져왔습니다. 이 둘은 각각 오류 발생 그리고 로딩 중일 때 나타나는 컴포넌트 중 하나로 쓰일 예정입니다.그 외 수정사항
TailwindCSS 태그 경고 나타나는 컴포넌트들, 태그 순서 수정하였습니다.
🏞️ 스크린샷 (선택)
스켈레톤 UI
ClearableInput
개선LabeledCheckbox
개선ErrorIndicator
추가LoadingIndicator
추가🗣️ 리뷰 요구사항 (선택)
스켈레톤 UI를 적용하긴 했는데, 데이터 패칭 후 UI 렌더링까지 몇 초 이상 걸리면 모르겠는데, 거의 깜빡이고 사라지는 수준이다 보니까, 오히려 사용자 경험을 해칠 수 있다는 생각도 드네요. 이 부분에 대한 의견도 남겨주시면 감사하겠습니다.
Summary by CodeRabbit
신규 기능
스토리북
스타일