Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/components/ui/Skeleton/Skeleton.css.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { createVar, style } from "@vanilla-extract/css";

import { colors } from "@/styles";

export const widthVar = createVar();
export const heightVar = createVar();
export const radiusVar = createVar();

export const wrapper = style({
width: widthVar,
height: heightVar,
borderRadius: radiusVar,
backgroundColor: colors.coolNeutral[98],
});
Comment on lines +9 to +14
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Border-radius 미전달 시 유효하지 않은 CSS가 렌더될 가능성

borderRadius: var(--radiusVar) 형태이므로, radius prop이 넘어오지 않을 때 해당 CSS 변수 자체가 정의되지 않습니다. 이 경우 border-radius: var(--radius) 가 브라우저에서 invalid value 로 판단되어 전체 규칙이 무시될 수 있습니다.

다음과 같이 기본값을 주입해 두면 안전합니다.

-  borderRadius: radiusVar,
+  borderRadius: `var(${radiusVar}, 0)`,
📝 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 wrapper = style({
width: widthVar,
height: heightVar,
borderRadius: radiusVar,
backgroundColor: colors.coolNeutral[98],
});
export const wrapper = style({
width: widthVar,
height: heightVar,
borderRadius: `var(${radiusVar}, 0)`,
backgroundColor: colors.coolNeutral[98],
});
🤖 Prompt for AI Agents
In src/components/ui/Skeleton/Skeleton.css.ts around lines 9 to 14, the
borderRadius style uses a CSS variable that may be undefined if the radius prop
is not provided, causing invalid CSS and potential rule ignoring by browsers.
Fix this by providing a fallback default value for borderRadius in the style
definition, ensuring the CSS variable has a valid fallback to prevent invalid
CSS rendering.

63 changes: 63 additions & 0 deletions src/components/ui/Skeleton/Skeleton.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import type { Meta, StoryObj } from "@storybook/nextjs";

import { Skeleton } from "./Skeleton";

const meta: Meta<typeof Skeleton> = {
title: "Components/Skeleton",
component: Skeleton,
tags: ["autodocs"],
parameters: {
docs: {
description: {
component:
"Skeleton 컴포넌트는 로딩 상태를 시각적으로 표시하기 위한 사각형 스켈레톤 UI입니다. width, height, radius를 지정할 수 있습니다.",
},
},
},
argTypes: {
width: {
control: "number",
description: "스켈레톤의 너비 (px)",
},
height: {
control: "number",
description: "스켈레톤의 높이 (px)",
},
radius: {
control: "text",
description: "스켈레톤의 border-radius (px 또는 string)",
},
},
};

export default meta;
type Story = StoryObj<typeof Skeleton>;

export const Default: Story = {
render: args => <Skeleton {...args} />,
args: {
width: 120,
height: 24,
radius: 8,
},
};

export const CustomRadius: Story = {
render: args => <Skeleton {...args} />,
args: {
width: 120,
height: 24,
radius: "50%",
},
};

export const VariousSizes: Story = {
render: () => (
<div style={{ display: "flex", gap: 16, alignItems: "center" }}>
<Skeleton width={40} height={40} radius={20} />
<Skeleton width={80} height={16} radius={4} />
<Skeleton width={120} height={24} radius={8} />
<Skeleton width={200} height={32} radius={16} />
</div>
),
};
34 changes: 34 additions & 0 deletions src/components/ui/Skeleton/Skeleton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { assignInlineVars } from "@vanilla-extract/dynamic";

import { coerceCssRemValue } from "@/lib/utils/coerceCssRemValue";

import * as styles from "./Skeleton.css";

export type SkeletonProps = {
/** 스켈레톤 너비 */
width: number;

/** 스켈레톤 높이 */
height: number;

/** border-radius(px 또는 string) */
radius?: number | string;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: HTMLAttributes 요것 확장해놓는 것은 어떨까요?! restProps 잘 뿌리고 style prop도 잘 merge해보고요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2d76967

오 좋습니다!! 요것도 수정했습니돠~!~!


/**
* Skeleton 컴포넌트
* @example
* ```tsx
* <Skeleton width={120} height={24} radius={8} />
* ```
*/
export const Skeleton = ({ width, height, radius }: SkeletonProps) => {
const style = assignInlineVars({
[styles.widthVar]: coerceCssRemValue(width),
[styles.heightVar]: coerceCssRemValue(height),
[styles.radiusVar]:
typeof radius === "number" ? coerceCssRemValue(radius) : radius,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: 요거 타입 확인은 coerceCssRemValue 요 함수 내에서 하니까 제거해도 괜찮을 것 같아요!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2d76967

오 역시~! 수정했습니다 ✨

});

return <div className={styles.wrapper} style={style} />;
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

스크린리더 접근성 속성이 누락됨

로딩 상태를 알리기 위해 최소한 aria-busy="true" 또는 role="status" 속성을 부여해야 합니다. 시각적으로 숨긴 텍스트를 추가하는 것도 방법입니다.

-return <div className={styles.wrapper} style={style} />;
+return (
+  <div
+    className={styles.wrapper}
+    style={style}
+    role="status"
+    aria-busy="true"
+  />
+);
📝 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 Skeleton = ({ width, height, radius }: SkeletonProps) => {
const style = assignInlineVars({
[styles.widthVar]: coerceCssRemValue(width),
[styles.heightVar]: coerceCssRemValue(height),
[styles.radiusVar]:
typeof radius === "number" ? coerceCssRemValue(radius) : radius,
});
return <div className={styles.wrapper} style={style} />;
};
export const Skeleton = ({ width, height, radius }: SkeletonProps) => {
const style = assignInlineVars({
[styles.widthVar]: coerceCssRemValue(width),
[styles.heightVar]: coerceCssRemValue(height),
[styles.radiusVar]:
typeof radius === "number" ? coerceCssRemValue(radius) : radius,
});
return (
<div
className={styles.wrapper}
style={style}
role="status"
aria-busy="true"
/>
);
};
🤖 Prompt for AI Agents
In src/components/ui/Skeleton/Skeleton.tsx around lines 25 to 34, the Skeleton
component lacks screen reader accessibility attributes. Add either
aria-busy="true" or role="status" to the div to indicate loading state to
assistive technologies. Optionally, include visually hidden text inside the div
to provide additional context for screen readers.

1 change: 1 addition & 0 deletions src/components/ui/Skeleton/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { Skeleton, type SkeletonProps } from "./Skeleton";