[Feat] #90 - 공통 Progress Indicators 컴포넌트 추가#91
Merged
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…y-matching/aim-frontend into feat/shared-empty_states
isAllChecked 계산식의 빈 배열 케이스를 보정 Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
line 1, 2 수정
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 23 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive suite of new UI components (Avatar, AvatarGroup, EmptyState, ListItem, Table, ProgressBar, CircularProgress, Spinner, Skeleton) along with Storybook documentation, icon exports, and supporting Tailwind CSS animations for progress bars and shimmer effects. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/shared/ui/progress/progress.tsx (2)
174-189: Consider addingaria-hiddento skeleton loaders.Skeletons are visual placeholders and shouldn't be announced by screen readers. Adding
aria-hidden="true"prevents confusion.♻️ Proposed fix
return ( <div + aria-hidden="true" className={`relative overflow-hidden bg-[color:var(--color-gray-200,`#E5E5E5`)] ${shapeClasses[shape]} ${className}`} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/progress/progress.tsx` around lines 174 - 189, The Skeleton component renders visual-only placeholders that may be announced by screen readers; update the outer wrapper in the Skeleton functional component to include aria-hidden="true" so assistive tech ignores it (modify the div returned by export const Skeleton to add aria-hidden="true"); ensure this attribute is applied regardless of shape/className so all skeleton variants are hidden from AT.
152-164: Add accessibility attributes to Spinner.The spinner should indicate its purpose to screen readers, especially when used as a loading indicator.
♿ Proposed fix to add accessibility
return ( <div + role="status" + aria-label="Loading" className={`rounded-full animate-spin border-[color:var(--color-gray-200,`#E5E5E5`)] border-t-[color:var(--color-primary-800,`#004A9C`)] ${sizeClasses[size]} ${className}`} - /> + > + <span className="sr-only">Loading...</span> + </div> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/progress/progress.tsx` around lines 152 - 164, The Spinner component currently lacks accessibility semantics; update the Spinner function to include role="status" on the root div and provide screen-reader text (e.g., a visually-hidden <span> with "Loading…" or an aria-label="Loading") so assistive tech knows it's a loading indicator; modify the JSX returned by Spinner to include role="status" and either an inner visually-hidden span with "Loading…" (use your project's sr-only class) or set aria-label/aria-live appropriately, keeping existing sizeClasses and className handling intact.src/shared/ui/avatars/avatars.tsx (2)
126-131: Add error handling for broken image URLs.If the image fails to load (404, network error), the browser will show a broken image icon instead of gracefully falling back to initials or the default icon.
♻️ Proposed fix to handle image load errors
+import React, { useState } from "react"; -import React from "react";Then in the component:
export const Avatar = ({ ... }: AvatarProps): React.ReactElement => { + const [imgError, setImgError] = useState(false); const isClickable = !!onClick; ... return ( <div className={avatarClasses} onClick={onClick}> - {src ? ( + {src && !imgError ? ( <img src={src} alt={name || "User avatar"} className="w-full h-full object-cover rounded-full" + onError={() => setImgError(true)} /> ) : name ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/avatars/avatars.tsx` around lines 126 - 131, The image tag currently shows a broken icon on load failure — add local state (e.g., hasImageError via useState) in the avatars component (avatars.tsx / Avatar or Avatars component) and change the render condition to use src && !hasImageError; attach an onError handler to the <img> (onError={() => setHasImageError(true)}) so when loading fails you fall back to rendering initials or the default icon (use the existing name prop for initials); also reset hasImageError if src prop changes to support new images.
182-188: Consider scaling the +N indicator text size with avatar size.The
+Nindicator uses a fixedtext-[14px]regardless of the avatarsizeprop. Forxsorsmavatars, this text may overflow; for2xlor3xl, it may appear too small.♻️ Proposed fix to use dynamic text sizing
Add a size-to-text mapping similar to
avatarTextSizeClasses:+const overflowTextSizeClasses: Record<AvatarSize, string> = { + xs: "text-[8px]", + sm: "text-[10px]", + md: "text-[12px]", + lg: "text-[14px]", + xl: "text-[18px]", + "2xl": "text-[24px]", + "3xl": "text-[32px]", +};Then update the +N indicator:
- <div - className={`${avatarSizeClasses[size]} relative inline-flex items-center justify-center rounded-full bg-[color:var(--color-gray-200,`#E5E5E5`)] text-[color:var(--color-gray-600,`#666666`)] font-bold text-[14px] border-[3px] border-white box-content shadow-[0_2px_8px_rgba(0,0,0,0.08)] flex-shrink-0 z-0`} + <div + className={`${avatarSizeClasses[size]} relative inline-flex items-center justify-center rounded-full bg-[color:var(--color-gray-200,`#E5E5E5`)] text-[color:var(--color-gray-600,`#666666`)] font-bold ${overflowTextSizeClasses[size]} border-[3px] border-white box-content shadow-[0_2px_8px_rgba(0,0,0,0.08)] flex-shrink-0 z-0`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/avatars/avatars.tsx` around lines 182 - 188, The +N indicator currently uses a fixed "text-[14px]" which can overflow or look small for different avatar sizes; replace the hardcoded size by deriving a text class from the existing avatarTextSizeClasses mapping (the same mapping used for avatar label sizing) and apply that class to the +{hiddenCount} div. Locate the +{hiddenCount} block in the avatars component (where avatarSizeClasses[size] is used) and swap the fixed text-[14px] for the dynamic class from avatarTextSizeClasses[size], ensuring the mapping covers all size keys (xs, sm, md, lg, xl, 2xl, 3xl) and falls back to a sensible default.
🤖 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/shared/ui/avatars/avatars.tsx`:
- Around line 123-125: The avatar div in avatars.tsx becomes interactive when
onClick is provided but lacks keyboard accessibility; update the returned
element (the div using avatarClasses and onClick) to be keyboard-focusable and
operable: when onClick exists add tabIndex={0}, role="button" and an onKeyDown
handler that calls the same onClick for Enter and Space keys (or convert to a
semantic <button> if styling allows), and ensure aria-label or aria-labelledby
is set to describe the avatar for screen readers.
In `@src/shared/ui/empty_states/empty-states.stories.tsx`:
- Around line 38-86: The story handlers in NoContent, NoResults, ErrorState, and
AccessDenied perform side-effects (alert, window.location.reload, history.back)
which must be side-effect free; replace those onClick implementations in the
primaryAction/secondaryAction args with non-destructive stubs (e.g., Storybook
action logger or simple no-op/console.log) so clicking in docs only demonstrates
the component without blocking, reloading, or navigating the preview; update the
onClick values for primaryAction in NoContent, NoResults, ErrorState and both
primaryAction/secondaryAction in AccessDenied accordingly.
In `@src/shared/ui/lists/lists.stories.tsx`:
- Around line 32-40: The story ListItemWithImage uses a remote image URL in its
args (args.imageUrl) which makes Storybook nondeterministic; replace the remote
picsum URL with a checked-in static asset or a data URI and update the story
args accordingly (modify ListItemWithImage.render/args to point to the local
asset or data URI) so the ListItem story uses a deterministic, versioned image.
In `@src/shared/ui/lists/lists.tsx`:
- Around line 241-267: The current list rendering uses only divs
(tableContainerClasses, tableHeaderRowClasses, columns mapping,
hasCheckbox/isAllChecked/onCheckAll) which breaks semantic table relationships
and accessibility; convert the header and body to real table semantics (use
<table>, <thead>, <tbody>, <tr>, <th>, <td> or at minimum add appropriate
role="table"/"rowgroup"/"row"/"columnheader"/"cell" attributes) so screen
readers can associate headers with cells, and ensure each checkbox (the
select-all checkbox and per-row checkboxes rendered alongside rows) has an
accessible name via aria-label or aria-labelledby and proper aria-checked
handling; update column header rendering (where columns.map produces header
cells) to be columnheader/th elements (or role="columnheader") and link them to
cells so assistive tech can determine column relationships.
- Around line 145-166: The interactive area using getListItemClasses with
handleItemClick must be keyboard-accessible and the checkbox must be labeled:
make the wrapper element that currently has onClick behave like a button by
adding tabIndex={0}, role="button", and a keyDown handler that invokes
handleItemClick on Enter/Space; ensure isDisabled prevents focus and activation.
For the checkbox (hasCheckbox, onCheck, id, isChecked, isDisabled,
handleCheckClick) provide an accessible label—either wrap the input in a <label>
that includes visible or visually-hidden text describing the item (e.g., using
the list item title or id) or add aria-label/aria-labelledby referencing the
item text—so screen readers know what the checkbox toggles, and keep the
existing e.stopPropagation() to avoid bubbling.
- Around line 91-112: getListItemClasses and getTableRowClasses currently always
add interactive classes (cursor-pointer, hover, group) even when no click
handler exists; update each function to accept an additional boolean flag
indicating whether a click handler is provided (e.g., hasOnClick for
getListItemClasses and hasOnRowClick for getTableRowClasses) and only append the
interactive classes when isDisabled is false AND the new handler flag is true;
keep the active/selected styles unchanged but remove cursor-pointer/hover/group
from the non-interactive branch so read-only items/rows no longer appear
clickable.
In `@src/shared/ui/progress/progress.tsx`:
- Around line 87-142: The CircularProgress component lacks ARIA attributes;
update the component (CircularProgress) to add accessibility attributes on the
wrapper (the outer div returned): set role="progressbar", aria-valuemin="0",
aria-valuemax="100", and aria-valuenow={clampedValue}; also accept an optional
ariaLabel prop (or default to a sensible string like "Progress") and add it as
aria-label or use aria-labelledby if a visible label exists (respecting
showLabel) so screen readers get a descriptive name for the progress indicator.
- Around line 16-75: The ProgressBar component lacks ARIA semantics; update the
ProgressBar JSX so the track or fill element exposes role="progressbar" with
aria-valuemin="0", aria-valuemax="100" and aria-valuenow={clampedValue} and
include either an aria-label prop or aria-labelledby that references the top
label when labelPosition === "top" (generate a stable id if needed); ensure the
accessible name reflects the purpose (e.g., "진행률" or a passed-in label) and keep
these attributes on the element that represents the progress value (refer to
ProgressBar, clampedValue, labelPosition, and fillClasses).
---
Nitpick comments:
In `@src/shared/ui/avatars/avatars.tsx`:
- Around line 126-131: The image tag currently shows a broken icon on load
failure — add local state (e.g., hasImageError via useState) in the avatars
component (avatars.tsx / Avatar or Avatars component) and change the render
condition to use src && !hasImageError; attach an onError handler to the <img>
(onError={() => setHasImageError(true)}) so when loading fails you fall back to
rendering initials or the default icon (use the existing name prop for
initials); also reset hasImageError if src prop changes to support new images.
- Around line 182-188: The +N indicator currently uses a fixed "text-[14px]"
which can overflow or look small for different avatar sizes; replace the
hardcoded size by deriving a text class from the existing avatarTextSizeClasses
mapping (the same mapping used for avatar label sizing) and apply that class to
the +{hiddenCount} div. Locate the +{hiddenCount} block in the avatars component
(where avatarSizeClasses[size] is used) and swap the fixed text-[14px] for the
dynamic class from avatarTextSizeClasses[size], ensuring the mapping covers all
size keys (xs, sm, md, lg, xl, 2xl, 3xl) and falls back to a sensible default.
In `@src/shared/ui/progress/progress.tsx`:
- Around line 174-189: The Skeleton component renders visual-only placeholders
that may be announced by screen readers; update the outer wrapper in the
Skeleton functional component to include aria-hidden="true" so assistive tech
ignores it (modify the div returned by export const Skeleton to add
aria-hidden="true"); ensure this attribute is applied regardless of
shape/className so all skeleton variants are hidden from AT.
- Around line 152-164: The Spinner component currently lacks accessibility
semantics; update the Spinner function to include role="status" on the root div
and provide screen-reader text (e.g., a visually-hidden <span> with "Loading…"
or an aria-label="Loading") so assistive tech knows it's a loading indicator;
modify the JSX returned by Spinner to include role="status" and either an inner
visually-hidden span with "Loading…" (use your project's sr-only class) or set
aria-label/aria-live appropriately, keeping existing sizeClasses and className
handling intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54d2e72d-c40d-4146-944a-c8ca71a9e838
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
src/index.csssrc/shared/ui/avatars/avatars.stories.tsxsrc/shared/ui/avatars/avatars.tsxsrc/shared/ui/empty_states/empty-states.stories.tsxsrc/shared/ui/empty_states/empty-states.tsxsrc/shared/ui/icons/index.tsxsrc/shared/ui/lists/lists.stories.tsxsrc/shared/ui/lists/lists.tsxsrc/shared/ui/progress/progress.stories.tsxsrc/shared/ui/progress/progress.tsxtailwind.config.js
Comment on lines
+123
to
+125
| return ( | ||
| <div className={avatarClasses} onClick={onClick}> | ||
| {/* 1. 이미지 우선 렌더링 */} |
Contributor
There was a problem hiding this comment.
Clickable avatar lacks keyboard accessibility.
When onClick is provided, the avatar becomes interactive but is not keyboard-accessible. Users navigating with a keyboard cannot focus or activate it.
♿ Proposed fix to add keyboard accessibility
- return (
- <div className={avatarClasses} onClick={onClick}>
+ const handleKeyDown = (e: React.KeyboardEvent) => {
+ if (onClick && (e.key === "Enter" || e.key === " ")) {
+ e.preventDefault();
+ onClick();
+ }
+ };
+
+ return (
+ <div
+ className={avatarClasses}
+ onClick={onClick}
+ onKeyDown={handleKeyDown}
+ tabIndex={isClickable ? 0 : undefined}
+ role={isClickable ? "button" : undefined}
+ >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/avatars/avatars.tsx` around lines 123 - 125, The avatar div in
avatars.tsx becomes interactive when onClick is provided but lacks keyboard
accessibility; update the returned element (the div using avatarClasses and
onClick) to be keyboard-focusable and operable: when onClick exists add
tabIndex={0}, role="button" and an onKeyDown handler that calls the same onClick
for Enter and Space keys (or convert to a semantic <button> if styling allows),
and ensure aria-label or aria-labelledby is set to describe the avatar for
screen readers.
Comment on lines
+38
to
+86
| export const NoContent: Story = { | ||
| args: { | ||
| variant: "no-content", | ||
| primaryAction: { | ||
| label: "첫 게시물 작성하기", | ||
| onClick: () => alert("게시물 작성 모달 오픈!"), | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const NoResults: Story = { | ||
| args: { | ||
| variant: "no-results", | ||
| primaryAction: { | ||
| label: "검색어 초기화", | ||
| onClick: () => alert("검색어가 초기화되었습니다."), | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const NoNotifications: Story = { | ||
| args: { | ||
| variant: "no-notifications", | ||
| }, | ||
| }; | ||
|
|
||
| export const ErrorState: Story = { | ||
| args: { | ||
| variant: "error", | ||
| primaryAction: { | ||
| label: "다시 시도", | ||
| onClick: () => window.location.reload(), | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const AccessDenied: Story = { | ||
| args: { | ||
| variant: "access-denied", | ||
| primaryAction: { | ||
| label: "이전 페이지로", | ||
| onClick: () => history.back(), | ||
| }, | ||
| secondaryAction: { | ||
| label: "홈으로 이동", | ||
| onClick: () => alert("홈으로 이동합니다."), | ||
| }, | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Keep story actions side-effect free.
These handlers currently call alert, window.location.reload(), and history.back(). Clicking the docs will block, reload, or navigate the preview instead of just demonstrating the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/empty_states/empty-states.stories.tsx` around lines 38 - 86,
The story handlers in NoContent, NoResults, ErrorState, and AccessDenied perform
side-effects (alert, window.location.reload, history.back) which must be
side-effect free; replace those onClick implementations in the
primaryAction/secondaryAction args with non-destructive stubs (e.g., Storybook
action logger or simple no-op/console.log) so clicking in docs only demonstrates
the component without blocking, reloading, or navigating the preview; update the
onClick values for primaryAction in NoContent, NoResults, ErrorState and both
primaryAction/secondaryAction in AccessDenied accordingly.
Comment on lines
+32
to
+40
| export const ListItemWithImage: StoryObj<typeof ListItem> = { | ||
| render: (args) => <ListItem {...args} />, | ||
| args: { | ||
| id: "item-2", | ||
| title: "프로필 정보 업데이트", | ||
| description: "새로운 이미지와 프로필 상세 정보를 업데이트 하세요.", | ||
| imageUrl: "https://picsum.photos/100/100", // 더미 이미지 | ||
| hasArrow: true, | ||
| }, |
Contributor
There was a problem hiding this comment.
Avoid remote assets in Storybook.
https://picsum.photos/... makes this story nondeterministic and can fail in offline or CI environments. A checked-in asset or data URI will keep the visual docs stable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/lists/lists.stories.tsx` around lines 32 - 40, The story
ListItemWithImage uses a remote image URL in its args (args.imageUrl) which
makes Storybook nondeterministic; replace the remote picsum URL with a
checked-in static asset or a data URI and update the story args accordingly
(modify ListItemWithImage.render/args to point to the local asset or data URI)
so the ListItem story uses a deterministic, versioned image.
Comment on lines
+91
to
+112
| const getListItemClasses = (isActive: boolean, isDisabled: boolean): string => { | ||
| if (isDisabled) { | ||
| return [listItemBaseClasses, "bg-white opacity-50 cursor-not-allowed"].join(" "); | ||
| } | ||
|
|
||
| const stateClasses = isActive | ||
| ? "bg-[color:var(--color-primary-50,#F0F6FD)] border-[color:var(--color-primary-200,#B3D1F7)]" | ||
| : "bg-white hover:bg-[color:var(--color-gray-50,#F9F9F9)] cursor-pointer group"; | ||
|
|
||
| return [listItemBaseClasses, stateClasses].join(" "); | ||
| }; | ||
|
|
||
| const getTableRowClasses = (isSelected: boolean, isDisabled: boolean): string => { | ||
| if (isDisabled) { | ||
| return [tableRowBaseClasses, "bg-white opacity-50 cursor-not-allowed"].join(" "); | ||
| } | ||
|
|
||
| const stateClasses = isSelected | ||
| ? "bg-[color:var(--color-primary-50,#F0F6FD)] border-[color:var(--color-primary-200,#B3D1F7)]" | ||
| : "bg-white hover:bg-[color:var(--color-gray-50,#F9F9F9)] cursor-pointer"; | ||
|
|
||
| return [tableRowBaseClasses, stateClasses].join(" "); |
Contributor
There was a problem hiding this comment.
Only show interactive affordances when a handler exists.
onClick and onRowClick are optional, but the class builders always add cursor-pointer, hover styles, and group. Read-only items and rows therefore look clickable even when nothing happens.
Also applies to: 145-145, 295-296
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/lists/lists.tsx` around lines 91 - 112, getListItemClasses and
getTableRowClasses currently always add interactive classes (cursor-pointer,
hover, group) even when no click handler exists; update each function to accept
an additional boolean flag indicating whether a click handler is provided (e.g.,
hasOnClick for getListItemClasses and hasOnRowClick for getTableRowClasses) and
only append the interactive classes when isDisabled is false AND the new handler
flag is true; keep the active/selected styles unchanged but remove
cursor-pointer/hover/group from the non-interactive branch so read-only
items/rows no longer appear clickable.
Comment on lines
+145
to
+166
| <div className={getListItemClasses(isActive, isDisabled)} onClick={handleItemClick}> | ||
| {/* 체크박스 */} | ||
| {hasCheckbox && ( | ||
| <div | ||
| className="flex items-center justify-center w-[20px] h-[20px] flex-shrink-0" | ||
| onClick={handleCheckClick} | ||
| > | ||
| <input | ||
| type="checkbox" | ||
| checked={isChecked} | ||
| onChange={() => { | ||
| if (onCheck) { | ||
| onCheck(id, !isChecked); | ||
| } | ||
| }} | ||
| onClick={(e) => { | ||
| // 부모 <li> 나 <div>에 걸려있는 onClick 이벤트로 버블링되는 것 방지= | ||
| e.stopPropagation(); | ||
| }} | ||
| disabled={isDisabled} | ||
| className="w-4 h-4 cursor-pointer accent-[color:var(--color-primary-800,#004A9C)]" | ||
| /> |
Contributor
There was a problem hiding this comment.
Make the interactive ListItem accessible.
When onClick is passed, the action lives on a plain div, so keyboard users can't focus or activate it. The checkbox is also unlabeled, so screen readers don't know what it toggles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/lists/lists.tsx` around lines 145 - 166, The interactive area
using getListItemClasses with handleItemClick must be keyboard-accessible and
the checkbox must be labeled: make the wrapper element that currently has
onClick behave like a button by adding tabIndex={0}, role="button", and a
keyDown handler that invokes handleItemClick on Enter/Space; ensure isDisabled
prevents focus and activation. For the checkbox (hasCheckbox, onCheck, id,
isChecked, isDisabled, handleCheckClick) provide an accessible label—either wrap
the input in a <label> that includes visible or visually-hidden text describing
the item (e.g., using the list item title or id) or add
aria-label/aria-labelledby referencing the item text—so screen readers know what
the checkbox toggles, and keep the existing e.stopPropagation() to avoid
bubbling.
Comment on lines
+241
to
+267
| <div className={tableContainerClasses}> | ||
| {/* Table Header */} | ||
| <div className={tableHeaderRowClasses}> | ||
| {hasCheckbox && ( | ||
| <div className={`${columnWidthClasses.checkbox} flex items-center justify-center`}> | ||
| <input | ||
| type="checkbox" | ||
| checked={isAllChecked} | ||
| onChange={(e) => onCheckAll && onCheckAll(e.target.checked)} | ||
| className="w-4 h-4 cursor-pointer accent-[color:var(--color-primary-800,#004A9C)]" | ||
| /> | ||
| </div> | ||
| )} | ||
| {columns.map((col) => ( | ||
| <div | ||
| key={col.id} | ||
| className={`flex items-center ${columnWidthClasses[col.width || "fill"]} ${ | ||
| columnAlignClasses[col.align || "left"] | ||
| }`} | ||
| > | ||
| <span className="text-[14px] font-semibold leading-[20px] text-[color:var(--color-gray-800,#333333)] truncate"> | ||
| {col.label} | ||
| </span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
|
|
Contributor
There was a problem hiding this comment.
Expose real table semantics here.
Headers, rows, and cells are all rendered as divs, so assistive tech loses the column/header relationships entirely. The select-all and row checkboxes also need accessible names.
Also applies to: 293-317
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/lists/lists.tsx` around lines 241 - 267, The current list
rendering uses only divs (tableContainerClasses, tableHeaderRowClasses, columns
mapping, hasCheckbox/isAllChecked/onCheckAll) which breaks semantic table
relationships and accessibility; convert the header and body to real table
semantics (use <table>, <thead>, <tbody>, <tr>, <th>, <td> or at minimum add
appropriate role="table"/"rowgroup"/"row"/"columnheader"/"cell" attributes) so
screen readers can associate headers with cells, and ensure each checkbox (the
select-all checkbox and per-row checkboxes rendered alongside rows) has an
accessible name via aria-label or aria-labelledby and proper aria-checked
handling; update column header rendering (where columns.map produces header
cells) to be columnheader/th elements (or role="columnheader") and link them to
cells so assistive tech can determine column relationships.
Comment on lines
+16
to
+75
| export const ProgressBar = ({ | ||
| value, | ||
| variant = "primary", | ||
| size = "medium", | ||
| hasStripes = false, | ||
| isAnimated = false, | ||
| labelPosition = "none", | ||
| className = "", | ||
| }: ProgressBarProps) => { | ||
| const clampedValue = Math.min(100, Math.max(0, value)); | ||
|
|
||
| const variantClasses = { | ||
| primary: "bg-[color:var(--color-primary-800,#004A9C)]", | ||
| success: "bg-[color:var(--color-success-500,#10A259)]", | ||
| warning: "bg-[color:var(--color-warning-500,#F59E0B)]", | ||
| error: "bg-[color:var(--color-error-500,#EF4444)]", | ||
| }; | ||
|
|
||
| const sizeClasses = { | ||
| thin: "h-[4px] rounded-[2px]", | ||
| medium: "h-[8px] rounded-[4px]", | ||
| thick: "h-[12px] rounded-[6px]", | ||
| }; | ||
|
|
||
| const fillClasses = [ | ||
| "h-full rounded-full transition-all duration-300 ease-in-out relative overflow-hidden", | ||
| variantClasses[variant], | ||
| hasStripes ? "bg-stripes bg-[length:1rem_1rem]" : "", // 👈 bg-[length:1rem_1rem] 추가 | ||
| isAnimated && hasStripes ? "animate-progress-stripes" : "", | ||
| ] | ||
| .filter(Boolean) | ||
| .join(" "); | ||
|
|
||
| return ( | ||
| <div className={`w-full flex flex-col gap-1 ${className}`}> | ||
| {/* Top Label */} | ||
| {labelPosition === "top" && ( | ||
| <div className="flex justify-between items-center text-[12px] font-medium text-[color:var(--color-gray-600,#666666)] mb-1"> | ||
| <span>진행률</span> | ||
| <span>{clampedValue}%</span> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Track */} | ||
| <div | ||
| className={`w-full bg-[color:var(--color-gray-200,#E5E5E5)] overflow-hidden ${sizeClasses[size]}`} | ||
| > | ||
| {/* Fill */} | ||
| <div className={fillClasses} style={{ width: `${clampedValue}%` }}> | ||
| {/* Inside Label */} | ||
| {labelPosition === "inside" && size === "thick" && ( | ||
| <div className="absolute inset-0 flex items-center justify-center text-[10px] font-bold text-white"> | ||
| {clampedValue}% | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
Add ARIA attributes for screen reader accessibility.
The ProgressBar lacks semantic accessibility attributes. Screen readers won't announce the progress value or purpose.
♿ Proposed fix to add ARIA attributes
{/* Track */}
<div
- className={`w-full bg-[color:var(--color-gray-200,`#E5E5E5`)] overflow-hidden ${sizeClasses[size]}`}
+ className={`w-full bg-[color:var(--color-gray-200,`#E5E5E5`)] overflow-hidden ${sizeClasses[size]}`}
+ role="progressbar"
+ aria-valuenow={clampedValue}
+ aria-valuemin={0}
+ aria-valuemax={100}
+ aria-label="Progress"
>📝 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.
Suggested change
| export const ProgressBar = ({ | |
| value, | |
| variant = "primary", | |
| size = "medium", | |
| hasStripes = false, | |
| isAnimated = false, | |
| labelPosition = "none", | |
| className = "", | |
| }: ProgressBarProps) => { | |
| const clampedValue = Math.min(100, Math.max(0, value)); | |
| const variantClasses = { | |
| primary: "bg-[color:var(--color-primary-800,#004A9C)]", | |
| success: "bg-[color:var(--color-success-500,#10A259)]", | |
| warning: "bg-[color:var(--color-warning-500,#F59E0B)]", | |
| error: "bg-[color:var(--color-error-500,#EF4444)]", | |
| }; | |
| const sizeClasses = { | |
| thin: "h-[4px] rounded-[2px]", | |
| medium: "h-[8px] rounded-[4px]", | |
| thick: "h-[12px] rounded-[6px]", | |
| }; | |
| const fillClasses = [ | |
| "h-full rounded-full transition-all duration-300 ease-in-out relative overflow-hidden", | |
| variantClasses[variant], | |
| hasStripes ? "bg-stripes bg-[length:1rem_1rem]" : "", // 👈 bg-[length:1rem_1rem] 추가 | |
| isAnimated && hasStripes ? "animate-progress-stripes" : "", | |
| ] | |
| .filter(Boolean) | |
| .join(" "); | |
| return ( | |
| <div className={`w-full flex flex-col gap-1 ${className}`}> | |
| {/* Top Label */} | |
| {labelPosition === "top" && ( | |
| <div className="flex justify-between items-center text-[12px] font-medium text-[color:var(--color-gray-600,#666666)] mb-1"> | |
| <span>진행률</span> | |
| <span>{clampedValue}%</span> | |
| </div> | |
| )} | |
| {/* Track */} | |
| <div | |
| className={`w-full bg-[color:var(--color-gray-200,#E5E5E5)] overflow-hidden ${sizeClasses[size]}`} | |
| > | |
| {/* Fill */} | |
| <div className={fillClasses} style={{ width: `${clampedValue}%` }}> | |
| {/* Inside Label */} | |
| {labelPosition === "inside" && size === "thick" && ( | |
| <div className="absolute inset-0 flex items-center justify-center text-[10px] font-bold text-white"> | |
| {clampedValue}% | |
| </div> | |
| )} | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| }; | |
| export const ProgressBar = ({ | |
| value, | |
| variant = "primary", | |
| size = "medium", | |
| hasStripes = false, | |
| isAnimated = false, | |
| labelPosition = "none", | |
| className = "", | |
| }: ProgressBarProps) => { | |
| const clampedValue = Math.min(100, Math.max(0, value)); | |
| const variantClasses = { | |
| primary: "bg-[color:var(--color-primary-800,`#004A9C`)]", | |
| success: "bg-[color:var(--color-success-500,`#10A259`)]", | |
| warning: "bg-[color:var(--color-warning-500,`#F59E0B`)]", | |
| error: "bg-[color:var(--color-error-500,`#EF4444`)]", | |
| }; | |
| const sizeClasses = { | |
| thin: "h-[4px] rounded-[2px]", | |
| medium: "h-[8px] rounded-[4px]", | |
| thick: "h-[12px] rounded-[6px]", | |
| }; | |
| const fillClasses = [ | |
| "h-full rounded-full transition-all duration-300 ease-in-out relative overflow-hidden", | |
| variantClasses[variant], | |
| hasStripes ? "bg-stripes bg-[length:1rem_1rem]" : "", // 👈 bg-[length:1rem_1rem] 추가 | |
| isAnimated && hasStripes ? "animate-progress-stripes" : "", | |
| ] | |
| .filter(Boolean) | |
| .join(" "); | |
| return ( | |
| <div className={`w-full flex flex-col gap-1 ${className}`}> | |
| {/* Top Label */} | |
| {labelPosition === "top" && ( | |
| <div className="flex justify-between items-center text-[12px] font-medium text-[color:var(--color-gray-600,`#666666`)] mb-1"> | |
| <span>진행률</span> | |
| <span>{clampedValue}%</span> | |
| </div> | |
| )} | |
| {/* Track */} | |
| <div | |
| className={`w-full bg-[color:var(--color-gray-200,`#E5E5E5`)] overflow-hidden ${sizeClasses[size]}`} | |
| role="progressbar" | |
| aria-valuenow={clampedValue} | |
| aria-valuemin={0} | |
| aria-valuemax={100} | |
| aria-label="Progress" | |
| > | |
| {/* Fill */} | |
| <div className={fillClasses} style={{ width: `${clampedValue}%` }}> | |
| {/* Inside Label */} | |
| {labelPosition === "inside" && size === "thick" && ( | |
| <div className="absolute inset-0 flex items-center justify-center text-[10px] font-bold text-white"> | |
| {clampedValue}% | |
| </div> | |
| )} | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/progress/progress.tsx` around lines 16 - 75, The ProgressBar
component lacks ARIA semantics; update the ProgressBar JSX so the track or fill
element exposes role="progressbar" with aria-valuemin="0", aria-valuemax="100"
and aria-valuenow={clampedValue} and include either an aria-label prop or
aria-labelledby that references the top label when labelPosition === "top"
(generate a stable id if needed); ensure the accessible name reflects the
purpose (e.g., "진행률" or a passed-in label) and keep these attributes on the
element that represents the progress value (refer to ProgressBar, clampedValue,
labelPosition, and fillClasses).
Comment on lines
+87
to
+142
| export const CircularProgress = ({ | ||
| value, | ||
| size = "medium", | ||
| showLabel = true, | ||
| className = "", | ||
| }: CircularProgressProps) => { | ||
| const clampedValue = Math.min(100, Math.max(0, value)); | ||
|
|
||
| const sizeConfig = { | ||
| small: { svgSize: 32, strokeWidth: 4, fontSize: "text-[12px]" }, | ||
| medium: { svgSize: 64, strokeWidth: 6, fontSize: "text-[16px]" }, | ||
| large: { svgSize: 96, strokeWidth: 8, fontSize: "text-[20px]" }, | ||
| }; | ||
|
|
||
| const { svgSize, strokeWidth, fontSize } = sizeConfig[size]; | ||
| const radius = (svgSize - strokeWidth) / 2; | ||
| const circumference = radius * 2 * Math.PI; | ||
| const dashoffset = circumference - (clampedValue / 100) * circumference; | ||
|
|
||
| return ( | ||
| <div className={`relative inline-flex items-center justify-center ${className}`}> | ||
| <svg width={svgSize} height={svgSize} className="-rotate-90"> | ||
| {/* Background Circle */} | ||
| <circle | ||
| cx={svgSize / 2} | ||
| cy={svgSize / 2} | ||
| r={radius} | ||
| fill="none" | ||
| stroke="var(--color-gray-200, #E5E5E5)" | ||
| strokeWidth={strokeWidth} | ||
| /> | ||
| {/* Progress Circle */} | ||
| <circle | ||
| cx={svgSize / 2} | ||
| cy={svgSize / 2} | ||
| r={radius} | ||
| fill="none" | ||
| stroke="var(--color-primary-800, #004A9C)" | ||
| strokeWidth={strokeWidth} | ||
| strokeLinecap="round" | ||
| strokeDasharray={circumference} | ||
| strokeDashoffset={dashoffset} | ||
| className="transition-all duration-300 ease-in-out" | ||
| /> | ||
| </svg> | ||
| {/* Center Label */} | ||
| {showLabel && ( | ||
| <span | ||
| className={`absolute font-bold text-[color:var(--color-gray-900,#1A1A1A)] ${fontSize}`} | ||
| > | ||
| {clampedValue}% | ||
| </span> | ||
| )} | ||
| </div> | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
Add ARIA attributes to CircularProgress for accessibility.
Similar to ProgressBar, the CircularProgress should have ARIA attributes for screen reader support.
♿ Proposed fix to add ARIA attributes
- <div className={`relative inline-flex items-center justify-center ${className}`}>
+ <div
+ className={`relative inline-flex items-center justify-center ${className}`}
+ role="progressbar"
+ aria-valuenow={clampedValue}
+ aria-valuemin={0}
+ aria-valuemax={100}
+ >📝 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.
Suggested change
| export const CircularProgress = ({ | |
| value, | |
| size = "medium", | |
| showLabel = true, | |
| className = "", | |
| }: CircularProgressProps) => { | |
| const clampedValue = Math.min(100, Math.max(0, value)); | |
| const sizeConfig = { | |
| small: { svgSize: 32, strokeWidth: 4, fontSize: "text-[12px]" }, | |
| medium: { svgSize: 64, strokeWidth: 6, fontSize: "text-[16px]" }, | |
| large: { svgSize: 96, strokeWidth: 8, fontSize: "text-[20px]" }, | |
| }; | |
| const { svgSize, strokeWidth, fontSize } = sizeConfig[size]; | |
| const radius = (svgSize - strokeWidth) / 2; | |
| const circumference = radius * 2 * Math.PI; | |
| const dashoffset = circumference - (clampedValue / 100) * circumference; | |
| return ( | |
| <div className={`relative inline-flex items-center justify-center ${className}`}> | |
| <svg width={svgSize} height={svgSize} className="-rotate-90"> | |
| {/* Background Circle */} | |
| <circle | |
| cx={svgSize / 2} | |
| cy={svgSize / 2} | |
| r={radius} | |
| fill="none" | |
| stroke="var(--color-gray-200, #E5E5E5)" | |
| strokeWidth={strokeWidth} | |
| /> | |
| {/* Progress Circle */} | |
| <circle | |
| cx={svgSize / 2} | |
| cy={svgSize / 2} | |
| r={radius} | |
| fill="none" | |
| stroke="var(--color-primary-800, #004A9C)" | |
| strokeWidth={strokeWidth} | |
| strokeLinecap="round" | |
| strokeDasharray={circumference} | |
| strokeDashoffset={dashoffset} | |
| className="transition-all duration-300 ease-in-out" | |
| /> | |
| </svg> | |
| {/* Center Label */} | |
| {showLabel && ( | |
| <span | |
| className={`absolute font-bold text-[color:var(--color-gray-900,#1A1A1A)] ${fontSize}`} | |
| > | |
| {clampedValue}% | |
| </span> | |
| )} | |
| </div> | |
| ); | |
| }; | |
| export const CircularProgress = ({ | |
| value, | |
| size = "medium", | |
| showLabel = true, | |
| className = "", | |
| }: CircularProgressProps) => { | |
| const clampedValue = Math.min(100, Math.max(0, value)); | |
| const sizeConfig = { | |
| small: { svgSize: 32, strokeWidth: 4, fontSize: "text-[12px]" }, | |
| medium: { svgSize: 64, strokeWidth: 6, fontSize: "text-[16px]" }, | |
| large: { svgSize: 96, strokeWidth: 8, fontSize: "text-[20px]" }, | |
| }; | |
| const { svgSize, strokeWidth, fontSize } = sizeConfig[size]; | |
| const radius = (svgSize - strokeWidth) / 2; | |
| const circumference = radius * 2 * Math.PI; | |
| const dashoffset = circumference - (clampedValue / 100) * circumference; | |
| return ( | |
| <div | |
| className={`relative inline-flex items-center justify-center ${className}`} | |
| role="progressbar" | |
| aria-valuenow={clampedValue} | |
| aria-valuemin={0} | |
| aria-valuemax={100} | |
| > | |
| <svg width={svgSize} height={svgSize} className="-rotate-90"> | |
| {/* Background Circle */} | |
| <circle | |
| cx={svgSize / 2} | |
| cy={svgSize / 2} | |
| r={radius} | |
| fill="none" | |
| stroke="var(--color-gray-200, `#E5E5E5`)" | |
| strokeWidth={strokeWidth} | |
| /> | |
| {/* Progress Circle */} | |
| <circle | |
| cx={svgSize / 2} | |
| cy={svgSize / 2} | |
| r={radius} | |
| fill="none" | |
| stroke="var(--color-primary-800, `#004A9C`)" | |
| strokeWidth={strokeWidth} | |
| strokeLinecap="round" | |
| strokeDasharray={circumference} | |
| strokeDashoffset={dashoffset} | |
| className="transition-all duration-300 ease-in-out" | |
| /> | |
| </svg> | |
| {/* Center Label */} | |
| {showLabel && ( | |
| <span | |
| className={`absolute font-bold text-[color:var(--color-gray-900,`#1A1A1A`)] ${fontSize}`} | |
| > | |
| {clampedValue}% | |
| </span> | |
| )} | |
| </div> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/ui/progress/progress.tsx` around lines 87 - 142, The
CircularProgress component lacks ARIA attributes; update the component
(CircularProgress) to add accessibility attributes on the wrapper (the outer div
returned): set role="progressbar", aria-valuemin="0", aria-valuemax="100", and
aria-valuenow={clampedValue}; also accept an optional ariaLabel prop (or default
to a sensible string like "Progress") and add it as aria-label or use
aria-labelledby if a visible label exists (respecting showLabel) so screen
readers get a descriptive name for the progress indicator.
- icons/index.tsx: dev의 20x20 아이콘 시스템으로 통일 (UserSolidIcon, FileTextAltIcon, BellIcon 등) - tailwind.config.js: dev content 경로 사용 + progress 애니메이션 theme 유지 - avatars/empty-states/lists stories: @storybook/nextjs import로 변경 - empty-states.tsx: FileTextIcon → FileTextAltIcon (dev 네이밍) - lists.stories.tsx: isAllChecked 한 줄 포매팅 적용
- progress.stories.tsx: @storybook/nextjs import로 변경 - avatars.tsx: img → next/image (fill 방식) - lists.tsx: img → next/image (width/height 고정) - next.config.ts: images.unoptimized 추가 (static export 대응)
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
2 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.
🔎 What is this PR?
공통 inputBox 컴포넌트를 구현하고 Storybook 문서화를 완료
📝 Changes
📸 Screenshots (선택)
📚 Background / Context (선택)
✔ Checklist
pnpm build)pnpm lint)🙏 Request
fixes #90
Summary by CodeRabbit
New Features
Documentation