Skip to content
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

feat(grid): create grid component #118

Merged
merged 7 commits into from
Feb 6, 2025
Merged

feat(grid): create grid component #118

merged 7 commits into from
Feb 6, 2025

Conversation

synuns
Copy link
Member

@synuns synuns commented Jan 28, 2025

Changes

  • Grid #83
    • add test scenario
    • add grid component
    • add stories

Visuals

image

Checklist

  • Have you written the functional specifications?
  • Have you written the test code?

Additional Discussion Points

  • We canceled using the @sipe-team/card component to display the layout in the storybook for the following reasons:
    • We defined a separate element for displaying the layout inside the storybook due to the following two issues:
      • If you have a version as a workspace, you need to migrate it every update
      • If you have a fixed version, you need to import the css module into the preview file in the root path
  • The grid can be used as a synthetic component with the root, gridItem is keyworded as item so that it can be used as a composite component
  • When using the current css module, it is inconvenient to manage the classname when you want to support the implied css and the style that the implied css contains
    • ex) grid-area is the implied type of grid-column and grid-row
  • For the user interface, the following customized interfaces are supported for the gridItem component
    • colSpan, rowSpan, colStart, colEnd, rowStart, rowEnd

Summary by CodeRabbit

  • 새로운 기능

    • 유연하고 사용자 정의 가능한 그리드 레이아웃 컴포넌트 추가
    • Storybook을 통한 그리드 컴포넌트 문서화 및 예시 제공
  • 개선 사항

    • CSS 모듈을 사용한 동적 그리드 스타일링
    • 다양한 그리드 레이아웃 및 중첩 그리드 지원
  • 테스트

    • 그리드 컴포넌트에 대한 포괄적인 단위 테스트 추가
  • 기타

    • TypeScript 설정 및 빌드 구성 업데이트

- Added a new Grid component with customizable grid properties
- Implemented GridItem for individual grid items
- Created Storybook stories for visual testing and documentation
- Added unit tests using Vitest to ensure functionality and style application
BREAKING CHANGE: Remove sipe-team/card dependency and restructure Grid props

- Remove redundant grid props that are already supported
- Add new GridItem props (colSpan, rowSpan, colStart, colEnd, rowStart, rowEnd)
- Remove sipe-team/card library dependency
- Enhance test coverage for new GridItem props
- Add comprehensive Grid stories demonstrating new features
Copy link

coderabbitai bot commented Jan 28, 2025

워크스루

이 풀 리퀘스트는 @sipe-team/grid 패키지를 위한 새로운 Grid 컴포넌트를 소개합니다. Storybook 설정, TypeScript 구성, CSS 모듈, 테스트 설정 등을 포함하여 Grid 컴포넌트의 전체 개발 환경을 구축했습니다. 컴포넌트는 동적 그리드 레이아웃을 지원하며, 다양한 그리드 속성과 스타일링 옵션을 제공합니다.

변경 사항

파일 변경 요약
packages/grid/.storybook/main.ts Storybook 기본 구성 추가
packages/grid/.storybook/preview.ts Storybook 미리보기 설정 추가
packages/grid/global.d.ts CSS 모듈 선언 추가
packages/grid/package.json 패키지 메타데이터 및 의존성 정의
packages/grid/src/Grid.module.css 그리드 레이아웃 CSS 모듈 생성
packages/grid/src/Grid.stories.tsx Grid 컴포넌트 Storybook 스토리 추가
packages/grid/src/Grid.test.tsx Grid 컴포넌트 테스트 스위트 작성
packages/grid/src/Grid.tsx Grid 컴포넌트 및 GridItem 구현
packages/grid/src/index.ts Grid 컴포넌트 및 타입 내보내기
packages/grid/tsconfig.json TypeScript 구성 추가
packages/grid/tsup.config.ts 번들링 설정 추가
packages/grid/vitest.config.ts Vitest 구성 추가
packages/grid/vitest.setup.ts 테스트 설정 파일 추가

관련 이슈

  • sipe-team/3-2_side#83: Grid 컴포넌트의 기능 및 요구사항과 정확히 일치하는 이슈입니다.

추천 리뷰어

  • noahluftyang
  • heeji289
  • froggy1014
  • hy57in
  • kimdaeyeobbb
  • developerjhp
  • SEMIN-97
  • bae-sh

시 (토끼의 노래)

🐰 그리드의 춤, 유연한 레이아웃
컬럼과 로우가 노래하네
CSS의 마법, 리액트의 힘
자유로운 공간, 디자인의 꿈
토끼의 그리드, 세상을 담다! 🌈

Tip

🌐 Web search-backed reviews and chat
  • We have enabled web search-based reviews and chat for all users. This feature allows CodeRabbit to access the latest documentation and information on the web.
  • You can disable this feature by setting web_search: false in the knowledge_base settings.
  • Please share any feedback in the Discord discussion.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (5)
packages/grid/.storybook/main.ts (1)

1-15: Storybook 메인 설정이 잘 구성되어 있습니다.

필수 애드온들이 포함되어 있고, 스토리 파일 패턴이 적절하게 정의되어 있습니다.

성능 최적화를 위해 다음과 같은 설정 추가를 고려해보세요:

 export default {
   stories: ['../src/**/*.mdx', '../src/**/*.stories.@(js|jsx|mjs|ts|tsx)'],
   addons: [
     '@storybook/addon-onboarding',
     '@storybook/addon-links',
     '@storybook/addon-essentials',
     '@storybook/addon-interactions',
   ],
   framework: {
     name: '@storybook/react-vite',
-    options: {},
+    options: {
+      builder: {
+        viteConfigPath: 'vite.config.ts',
+      },
+    },
   },
+  docs: {
+    autodocs: true,
+  },
 } satisfies StorybookConfig;
packages/grid/src/Grid.tsx (2)

40-40: 타입 안전성 개선 필요

ForwardedRef<any> 대신 더 구체적인 타입을 사용하는 것이 좋습니다.

-  ref: ForwardedRef<any>,
+  ref: ForwardedRef<HTMLDivElement>,

109-123: 그리드 위치 계산 로직 단순화 제안

getGridColumngetGridRow 메서드의 로직이 중복되어 있습니다. 공통 함수로 추출하여 코드 중복을 제거하는 것이 좋습니다.

+  const getGridPosition = (
+    direct: string | undefined,
+    span: number | undefined,
+    start: number | 'auto' | undefined,
+    end: number | 'auto' | undefined,
+  ) => {
+    if (direct) return direct;
+    if (span) return `span ${span}`;
+    if (start || end) return `${start ?? 'auto'} / ${end ?? 'auto'}`;
+    return undefined;
+  };
+
-  const getGridColumn = () => {
-    if (column) return column;
-    if (colSpan) return `span ${colSpan}`;
-    if (colStart || colEnd)
-      return `${colStart ?? 'auto'} / ${colEnd ?? 'auto'}`;
-    return undefined;
-  };
+  const getGridColumn = () => getGridPosition(column, colSpan, colStart, colEnd);
-  const getGridRow = () => {
-    if (row) return row;
-    if (rowSpan) return `span ${rowSpan}`;
-    if (rowStart || rowEnd)
-      return `${rowStart ?? 'auto'} / ${rowEnd ?? 'auto'}`;
-    return undefined;
-  };
+  const getGridRow = () => getGridPosition(row, rowSpan, rowStart, rowEnd);
packages/grid/src/Grid.module.css (1)

1-12: CSS 변수에 대한 폴백 값 추가 필요

CSS 변수가 정의되지 않은 경우를 대비하여 기본값을 지정하는 것이 좋습니다.

 .grid {
-  display: var(--grid-display);
+  display: var(--grid-display, grid);
-  grid-template-columns: var(--grid-template-columns);
+  grid-template-columns: var(--grid-template-columns, none);
-  grid-template-rows: var(--grid-template-rows);
+  grid-template-rows: var(--grid-template-rows, none);
   /* Add fallbacks for other properties */
 }
packages/grid/src/Grid.stories.tsx (1)

4-37: 접근성 관련 스토리 추가 필요

현재 스토리북에는 기본적인 사용 예제는 잘 구현되어 있지만, 다음과 같은 접근성 관련 예제가 누락되어 있습니다:

  • 스크린 리더 사용자를 위한 그리드 레이아웃
  • 키보드 네비게이션
  • 고대비 모드에서의 표시

접근성 관련 스토리를 추가하시겠습니까? 필요한 경우 예제 코드를 제공해드릴 수 있습니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d35f52 and 305151d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (13)
  • packages/grid/.storybook/main.ts (1 hunks)
  • packages/grid/.storybook/preview.ts (1 hunks)
  • packages/grid/global.d.ts (1 hunks)
  • packages/grid/package.json (1 hunks)
  • packages/grid/src/Grid.module.css (1 hunks)
  • packages/grid/src/Grid.stories.tsx (1 hunks)
  • packages/grid/src/Grid.test.tsx (1 hunks)
  • packages/grid/src/Grid.tsx (1 hunks)
  • packages/grid/src/index.ts (1 hunks)
  • packages/grid/tsconfig.json (1 hunks)
  • packages/grid/tsup.config.ts (1 hunks)
  • packages/grid/vitest.config.ts (1 hunks)
  • packages/grid/vitest.setup.ts (1 hunks)
✅ Files skipped from review due to trivial changes (5)
  • packages/grid/vitest.setup.ts
  • packages/grid/tsconfig.json
  • packages/grid/global.d.ts
  • packages/grid/tsup.config.ts
  • packages/grid/package.json
🔇 Additional comments (4)
packages/grid/src/index.ts (1)

1-2: 내보내기 구문이 잘 구성되어 있습니다!

컴포넌트와 타입 내보내기가 깔끔하게 정리되어 있으며, 배럴(barrel) 파일 패턴을 잘 따르고 있습니다.

packages/grid/.storybook/preview.ts (1)

1-5: Storybook 설정이 적절하게 구성되어 있습니다!

autodocs 태그를 활성화하여 자동 문서화를 지원하고 있으며, satisfies 연산자를 통해 타입 안정성이 보장되어 있습니다.

packages/grid/vitest.config.ts (1)

1-11: 테스트 설정이 적절히 구성되어 있습니다.

기본 설정을 상속받아 프로젝트별 설정을 추가하는 방식이 잘 구현되어 있습니다.

설정된 setup 파일의 존재 여부를 확인해보시기 바랍니다:

✅ Verification successful

테스트 설정이 정상적으로 구성되었음을 확인했습니다

vitest.setup.ts 파일이 정상적으로 존재하며, 설정 파일에서 올바르게 참조되고 있습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of vitest.setup.ts file
test -f packages/grid/vitest.setup.ts && echo "Setup file exists" || echo "Setup file missing"

Length of output: 84

packages/grid/src/Grid.tsx (1)

158-161: 명확한 컴포넌트 내보내기 구조

RootItem 별칭을 통한 내보내기는 좋은 패턴입니다. 컴포넌트의 계층 구조를 명확하게 표현합니다.

Comment on lines +76 to +81
colSpan?: number;
rowSpan?: number;
colStart?: number | 'auto';
colEnd?: number | 'auto';
rowStart?: number | 'auto';
rowEnd?: number | 'auto';
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

숫자 prop에 대한 유효성 검사가 필요합니다

colSpan, rowSpan, colStart, colEnd, rowStart, rowEnd prop에 대한 유효성 검사가 없어 음수나 0이 전달될 경우 CSS Grid 레이아웃이 깨질 수 있습니다. 다음과 같은 개선이 필요합니다:

  • colSpanrowSpan은 1 이상의 양수만 허용
  • colStart, colEnd, rowStart, rowEnd는 'auto'이거나 0이 아닌 정수여야 함
🔗 Analysis chain

숫자 prop에 대한 유효성 검사 추가 필요

colSpan, rowSpan, colStart, colEnd, rowStart, rowEnd와 같은 숫자 prop에 대한 유효성 검사가 없습니다. 음수 값이나 0이 전달될 경우 예기치 않은 동작이 발생할 수 있습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any validation checks for these props in the codebase
rg -A 5 "colSpan|rowSpan|colStart|colEnd|rowStart|rowEnd.*validate"

Length of output: 4457


Script:

#!/bin/bash
# Check the full implementation of Grid.tsx
cat packages/grid/src/Grid.tsx

# Also check if there are any type definitions
fd "Grid.types.ts|Grid.d.ts" .

Length of output: 3653

Comment on lines +77 to +166
describe('Grid Item Properties', () => {
const gridItemProperties = [
{
prop: 'column',
style: 'gridColumn',
values: ['1 / 3', 'span 2', 'auto'],
},
{
prop: 'row',
style: 'gridRow',
values: ['1 / 3', 'span 2', 'auto'],
},
{
prop: 'area',
style: 'gridArea',
values: ['header', 'main', 'footer'],
},
{
prop: 'justifySelf',
style: 'justifySelf',
values: ['start', 'end', 'center', 'stretch'],
},
{
prop: 'alignSelf',
style: 'alignSelf',
values: ['start', 'end', 'center', 'stretch'],
},
{
prop: 'colSpan',
style: 'gridColumn',
values: [1, 2, 3],
expectedValues: ['span 1', 'span 2', 'span 3'],
},
{
prop: 'rowSpan',
style: 'gridRow',
values: [1, 2, 3],
expectedValues: ['span 1', 'span 2', 'span 3'],
},
{
prop: 'colStart',
style: 'gridColumn',
values: [1, 2, 3],
expectedValues: ['1 / auto', '2 / auto', '3 / auto'],
},
{
prop: 'colEnd',
style: 'gridColumn',
values: [1, 2, 3],
expectedValues: ['auto / 1', 'auto / 2', 'auto / 3'],
},
{
prop: 'rowStart',
style: 'gridRow',
values: [1, 2, 3],
expectedValues: ['1 / auto', '2 / auto', '3 / auto'],
},
{
prop: 'rowEnd',
style: 'gridRow',
values: [1, 2, 3],
expectedValues: ['auto / 1', 'auto / 2', 'auto / 3'],
},
];

for (const { prop, style, values, expectedValues } of gridItemProperties) {
describe(prop, () => {
it.each(
values.map((value, index) => ({
[prop]: value,
expectedValue: expectedValues?.[index] ?? value,
})),
)(
`should apply ${style} when ${prop} prop is $${prop}`,
({ [prop]: value, expectedValue }) => {
render(
<Grid.Root>
<Grid.Item data-testid="grid-item" {...{ [prop]: value }}>
item 1
</Grid.Item>
</Grid.Root>,
);

const gridItem = screen.getByTestId('grid-item');
expect(gridItem).toHaveStyle({ [style]: expectedValue });
},
);
});
}
});
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

엣지 케이스 테스트 부재 확인됨

코드베이스 분석 결과, Grid 컴포넌트에 엣지 케이스 처리를 위한 유효성 검사나 테스트가 구현되어 있지 않음이 확인되었습니다. 다음과 같은 테스트 케이스 추가를 권장드립니다:

  • colSpan, rowSpan에 대한 음수 값 테스트 (-1, -2 등)
  • area 속성에 대한 유효하지 않은 문자열 테스트 (특수문자 포함 등)
  • colStart, colEnd, rowStart, rowEnd에 대한 범위 초과 값 테스트 (매우 큰 숫자)

이러한 테스트는 컴포넌트의 안정성과 예측 가능한 동작을 보장하는 데 도움이 될 것입니다.

🔗 Analysis chain

엣지 케이스에 대한 테스트 추가 필요

그리드 아이템 속성에 대한 테스트는 잘 작성되어 있지만, 다음과 같은 엣지 케이스에 대한 테스트가 누락되어 있습니다:

  • 음수 값이 전달된 경우
  • 잘못된 형식의 문자열이 전달된 경우
  • 매우 큰 숫자가 전달된 경우

테스트 파일에서 이러한 엣지 케이스를 다루는 테스트가 있는지 확인:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
rg "invalid|negative|large.*value" packages/grid/src/Grid.test.tsx

Length of output: 68


Script:

#!/bin/bash
# Check Grid component for validation logic
echo "=== Checking Grid.tsx for validation logic ==="
rg "throw|error|warning|invalid" packages/grid/src/Grid.tsx

echo -e "\n=== Checking for prop-types or validation patterns ==="
ast-grep --pattern 'if ($$$) { throw new Error($$$) }' packages/grid/src/Grid.tsx

echo -e "\n=== Checking test file for any error scenarios ==="
ast-grep --pattern 'it($_, () => { expect($$$).toThrow($$$) })' packages/grid/src/Grid.test.tsx

Length of output: 588

@froggy1014 froggy1014 added the enhancement New feature or request label Jan 28, 2025
Copy link

changeset-bot bot commented Jan 28, 2025

🦋 Changeset detected

Latest commit: c9e7762

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sipe-team/grid Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

codecov bot commented Feb 1, 2025

Codecov Report

Attention: Patch coverage is 99.42693% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/checkbox/src/hooks/useCheckboxGroup.ts 0.00% 2 Missing ⚠️
Files with missing lines Coverage Δ
packages/avatar/src/Avatar.test.tsx 100.00% <100.00%> (ø)
packages/card/src/Card.tsx 100.00% <100.00%> (ø)
packages/checkbox/src/Checkbox.test.tsx 100.00% <100.00%> (ø)
packages/grid/src/Grid.test.tsx 100.00% <100.00%> (ø)
packages/grid/src/Grid.tsx 100.00% <100.00%> (ø)
packages/checkbox/src/hooks/useCheckboxGroup.ts 0.00% <0.00%> (ø)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 305151d and 6c315d8.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (18)
  • .changeset/unlucky-rice-grab.md (1 hunks)
  • .storybook/main.ts (1 hunks)
  • eslint.config.ts (1 hunks)
  • packages/Input/src/Input.stories.tsx (1 hunks)
  • packages/avatar/src/Avatar.module.css (1 hunks)
  • packages/avatar/src/Avatar.test.tsx (1 hunks)
  • packages/card/src/Card.tsx (3 hunks)
  • packages/checkbox/src/Checkbox.module.css (1 hunks)
  • packages/checkbox/src/Checkbox.test.tsx (1 hunks)
  • packages/checkbox/src/hooks/useCheckboxGroup.ts (1 hunks)
  • packages/flex/package.json (1 hunks)
  • packages/radio-group/src/RadioGroup.stories.tsx (1 hunks)
  • packages/reset/package.json (1 hunks)
  • packages/side/package.json (1 hunks)
  • packages/tokens/src/Tokens.stories.tsx (2 hunks)
  • packages/tooltip/src/Tooltip.stories.tsx (2 hunks)
  • packages/typography/src/Typography.stories.tsx (2 hunks)
  • www/src/HomepageFeatures/index.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (11)
  • packages/checkbox/src/Checkbox.module.css
  • .storybook/main.ts
  • packages/checkbox/src/Checkbox.test.tsx
  • packages/avatar/src/Avatar.module.css
  • packages/tooltip/src/Tooltip.stories.tsx
  • packages/avatar/src/Avatar.test.tsx
  • packages/card/src/Card.tsx
  • packages/radio-group/src/RadioGroup.stories.tsx
  • packages/reset/package.json
  • packages/tokens/src/Tokens.stories.tsx
  • packages/flex/package.json
🧰 Additional context used
🪛 GitHub Actions: CI
packages/Input/src/Input.stories.tsx

[warning] 42-42: The story should use PascalCase notation: disabled storybook/prefer-pascal-case

🪛 GitHub Check: codecov/patch
packages/checkbox/src/hooks/useCheckboxGroup.ts

[warning] 8-9: packages/checkbox/src/hooks/useCheckboxGroup.ts#L8-L9
Added lines #L8 - L9 were not covered by tests

🔇 Additional comments (12)
www/src/HomepageFeatures/index.tsx (1)

27-28: 텍스트 서식이 적절하게 개선되었습니다.

JSX 표현식에서 공백 문자를 추가하고 텍스트를 두 줄로 나누어 가독성이 향상되었습니다.

packages/typography/src/Typography.stories.tsx (2)

22-24: 코드 스타일이 개선되었습니다!

각 Typography 컴포넌트의 가중치(weight) 예제들이 한 줄로 정리되어 가독성이 향상되었습니다.


44-45: 코드 스타일이 개선되었습니다!

줄 높이(line height) 예제들도 한 줄로 정리되어 일관된 코드 스타일을 유지하고 있습니다.

.changeset/unlucky-rice-grab.md (2)

1-3: 메타데이터 구성 확인
파일 상단의 YAML 헤더와 버전 업데이트 정보 ("@sipe-team/grid": minor)가 올바르게 작성되어 있습니다. 이 정보는 grid 컴포넌트 추가에 따른 minor 업데이트를 적절히 나타내고 있습니다.


5-5: 커밋 메시지 검토
커밋 메시지 'feat(grid): create grid component'가 간결하고 명확하게 기능 추가를 설명하고 있습니다. PR 목표에 부합하며, 스토리와 테스트를 통해 추가 설명이 제공될 예정임이 확인됩니다.

eslint.config.ts (1)

20-27: ESLint 규칙 구성이 잘 되어있습니다!

규칙 구성이 명확하고 체계적으로 되어 있습니다:

  • 기본 no-unused-expressions 규칙을 비활성화하고 TypeScript 전용 버전을 사용하여 타입 인식을 개선했습니다
  • 옵션이 명시적으로 설정되어 있어 코드 품질 관리가 용이합니다
    • 단락 평가(short-circuit)는 허용하지 않음
    • 삼항 연산자는 허용
packages/side/package.json (1)

12-12: 파일 files 배열 형식 변경 확인

files 배열의 값을 한 줄로 작성하는 형식 변경은 기능적 영향은 없으나, 팀 내 코딩 스타일 가이드라인과 일관성을 유지하는지 확인해 주세요. 다른 패키지들의 package.json과의 일관성이 중요하다면 이 형식이 적절합니다.

packages/checkbox/src/hooks/useCheckboxGroup.ts (3)

5-5: 인터페이스 onChange 서명 수정 확인
인터페이스 내 onChange 함수 서명에서 마지막에 사용된 불필요한 쉼표를 제거하여 가독성이 향상되었습니다. 해당 변경은 기능상에 영향을 주지 않으며, 코드 스타일을 일관되게 만드는데 도움이 됩니다.


8-9: 테스트 커버리지 보완 필요
새롭게 추가된 훅 초기화 부분(특히 useState를 통한 상태 초기화 및 useCallback 사용)이 현재 테스트에 포함되지 않은 것으로 확인되었습니다. 이 기능에 대해 유닛 테스트를 추가하여 후속 리팩토링 시 안정성을 보장하는 것이 좋겠습니다.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 8-9: packages/checkbox/src/hooks/useCheckboxGroup.ts#L8-L9
Added lines #L8 - L9 were not covered by tests


Line range hint 8-32: 훅 전체 로직 검토 완료
전체적으로 useCheckboxGroup 훅은 체크박스 상태를 관리하고, 업데이트 및 전체 체크/인디터미넌트 상태 계산을 올바르게 수행하고 있습니다. useCallback과 상태 업데이트 로직이 적절하게 구현되어 있으며, 기능적으로 문제가 없어 보입니다.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 8-9: packages/checkbox/src/hooks/useCheckboxGroup.ts#L8-L9
Added lines #L8 - L9 were not covered by tests

packages/Input/src/Input.stories.tsx (2)

38-38: 코드가 더 간결해졌네요!

return 문을 한 줄로 단순화한 것이 가독성을 향상시켰습니다.


45-45: 간결한 코드 변경 👍

return 문을 단순화하여 코드가 더 깔끔해졌습니다.

{...args}
/>
);
return <Input value={value} onChange={(e) => setValue(e.target.value)} {...args} />;
},
};

export const disabled: Story = {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

스토리 이름을 PascalCase로 수정해주세요

Storybook 네이밍 컨벤션에 따라 'disabled'를 'Disabled'로 변경해야 합니다.

다음과 같이 수정해주세요:

-export const disabled: Story = {
+export const Disabled: Story = {
📝 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 disabled: Story = {
export const Disabled: Story = {
🧰 Tools
🪛 GitHub Actions: CI

[warning] 42-42: The story should use PascalCase notation: disabled storybook/prefer-pascal-case

- Add useMemo for memoizing grid column and row calculations
- Provide sensible default values for Grid and GridItem props
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/grid/src/Grid.tsx (2)

6-17: 인터페이스에 JSDoc 문서화가 필요합니다

컴포넌트의 재사용성과 유지보수성을 높이기 위해 GridProps 인터페이스에 JSDoc 문서를 추가하는 것이 좋습니다. 각 prop의 용도와 예상되는 값 범위를 명시해주세요.

예시:

+/**
+ * Grid 컴포넌트의 props 인터페이스
+ * @property {CSSProperties['gridTemplateColumns']} templateColumns - 그리드 열 템플릿
+ * @property {CSSProperties['gridTemplateRows']} templateRows - 그리드 행 템플릿
+ * ...
+ */
 export interface GridProps extends ComponentProps<'div'> {

95-95: ref 타입을 구체화해야 합니다

ForwardedRef<any> 대신 구체적인 타입을 사용하여 타입 안전성을 높이는 것이 좋습니다.

- ref: ForwardedRef<any>,
+ ref: ForwardedRef<HTMLDivElement>,
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c315d8 and c9e7762.

📒 Files selected for processing (2)
  • packages/grid/package.json (1 hunks)
  • packages/grid/src/Grid.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/grid/package.json
🔇 Additional comments (3)
packages/grid/src/Grid.tsx (3)

39-49: CSS 변수 상속 문제 해결 필요

현재 구현에서는 일부 CSS 변수들이 명시적인 기본값 없이 'auto'로 설정되어 있어 상속 문제가 발생할 수 있습니다.

다음과 같이 더 구체적인 기본값을 설정하는 것이 좋습니다:

 const gridStyle = {
   '--grid-display': inline ? 'inline-grid' : 'grid',
-  '--grid-template-columns': templateColumns ?? 'auto',
-  '--grid-template-rows': templateRows ?? 'auto',
+  '--grid-template-columns': templateColumns ?? 'none',
+  '--grid-template-rows': templateRows ?? 'none',
   '--grid-template-areas': templateAreas ?? 'auto',
   '--grid-auto-columns': autoColumns ?? 'auto',
   '--grid-auto-rows': autoRows ?? 'auto',
   '--grid-auto-flow': autoFlow ?? 'row',
   '--grid-gap': gap ?? '0',
   ...style,
 } as React.CSSProperties;

66-71: 숫자 prop에 대한 유효성 검사가 필요합니다

colSpan, rowSpan, colStart, colEnd, rowStart, rowEnd prop에 대한 유효성 검사가 없어 음수나 0이 전달될 경우 CSS Grid 레이아웃이 깨질 수 있습니다.

다음과 같은 유효성 검사 함수를 추가하는 것이 좋습니다:

const validateSpan = (value: number) => {
  if (value <= 0) {
    throw new Error('Span 값은 1 이상이어야 합니다');
  }
  return value;
};

const validatePosition = (value: number | 'auto') => {
  if (value !== 'auto' && value === 0) {
    throw new Error('Position 값은 0이 아닌 정수이거나 "auto"여야 합니다');
  }
  return value;
};

141-144: 컴포넌트 내보내기가 잘 구현되었습니다

RootItem 별칭을 통한 내보내기는 일반적인 컴포넌트 패턴을 잘 따르고 있습니다.

Copy link
Collaborator

@noahluftyang noahluftyang left a comment

Choose a reason for hiding this comment

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

gogo~

@synuns synuns merged commit 08121f0 into main Feb 6, 2025
4 checks passed
@synuns synuns deleted the feat/grid branch February 6, 2025 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants