Skip to content

Breadcrumbの修正 - #136

Merged
ore0 merged 11 commits into
mainfrom
fix/breadcrumb
Aug 29, 2025
Merged

Breadcrumbの修正#136
ore0 merged 11 commits into
mainfrom
fix/breadcrumb

Conversation

@ore0

@ore0 ore0 commented Aug 21, 2025

Copy link
Copy Markdown

概要

Notionに基づく修正を実施
https://www.notion.so/goodpatch-design-div/Breadcrumb-fa6ee15111f64147ac40cca104684f34?source=copy_link

変更内容

  • スタイルの修正
  • テストの調整
  • Storybookの調整
  • Figma Connect

動作確認

  • pnpm lint を実行
  • pnpm build:package を実行してパッケージが正常にビルドされることを確認
  • pnpm test を実行してテストがすべて成功することを確認

関連 Issue

ore0 added 6 commits August 21, 2025 16:19
- Linkコンポーネントのインポート元を修正
- BreadcrumbLinkコンポーネントのスタイルを簡素化
- BreadcrumbSeparatorコンポーネントの色を更新
- テストデータ定数とヘルパー関数を追加
- 基本的なBreadcrumbのレンダリングテストを追加
- リンク機能のテストを追加
- 区切り文字の機能テストを追加
- アクセシビリティの検証テストを追加
- エッジケースのテストを追加
- BreadcrumbItemのリンクを「Components」から「Link」に変更
- BreadcrumbPageのテキストを「Breadcrumb」から「Current Page」に変更
- Figmaとの接続設定を追加
- 基本的なBreadcrumbナビゲーションの例を実装
- Breadcrumbコンポーネントの説明文を修正
- Figma Code Connectに関するコメントを更新
- `props`のマッピングに関する説明を追加
- `example`関数の更新に関する指示を追加
Copilot AI review requested due to automatic review settings August 21, 2025 07:24
@vercel

vercel Bot commented Aug 21, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
sparkle-design Ready Ready Preview Comment Aug 29, 2025 1:54am

This comment was marked as outdated.

@ore0
ore0 requested a review from Copilot August 21, 2025 07:26

This comment was marked as outdated.

@ore0
ore0 requested a review from Copilot August 21, 2025 07:27

This comment was marked as outdated.

@ore0
ore0 requested a review from Copilot August 21, 2025 07:31

This comment was marked as outdated.

@ore0
ore0 requested a review from Copilot August 21, 2025 07:32

This comment was marked as outdated.

@ore0
ore0 requested a review from Copilot August 21, 2025 07:40
@ore0
ore0 marked this pull request as draft August 21, 2025 07:40

This comment was marked as outdated.

@ore0
ore0 requested a review from Copilot August 22, 2025 03:50

This comment was marked as outdated.

@ore0 ore0 self-assigned this Aug 22, 2025

This comment was marked as outdated.

@ore0
ore0 marked this pull request as ready for review August 27, 2025 01:49
@ore0
ore0 requested a review from Copilot August 27, 2025 01:49

This comment was marked as resolved.

@ore0
ore0 requested a review from touyou August 27, 2025 01:55
@ore0
ore0 marked this pull request as draft August 27, 2025 01:55
@ore0
ore0 marked this pull request as ready for review August 27, 2025 13:48
- Breadcrumbのサポート状況を「✅」に変更

@touyou touyou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

いくつかコメント返しました🙋

Comment thread src/components/ui/breadcrumb/index.tsx
Comment thread src/components/ui/breadcrumb/index.stories.tsx
</BreadcrumbItem>
<BreadcrumbSeparator />
<BreadcrumbItem>
<BreadcrumbLink href="/components">Components</BreadcrumbLink>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

これはLinkコンポーネントも同様なのでうまくいったら横展開できればというところなんですが

onClick={(e) => {
    e.preventDefault();
    action('anchor clicked')(e);
  }}

などをして実際の遷移をしないようにしておくと動作確認がしやすそうと思いました

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

対応しようとするとStroybookのサンプルコードにもイベントハンドラが表示されますけど、これは致し方なしなんですかね。
image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

そこのコードはそもそもrender関数がそのまま出ちゃうので参考にだけしてあくまで綺麗なサンプルはドキュメントの方の使用例を見てもらう運用にする、で割と今のフォーマットのままいける気がするのでいいかと思いました🙆‍♂️

どうしても気になるようであればargsに定義できるようにしておいてそっちで指定したら出なくなるとかあるかも?ですね

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

対応しました!
eb9fa85

ore0 added 2 commits August 29, 2025 01:41
- Breadcrumbコンポーネントの使用例をドキュメントに追加しました。
- BreadcrumbコンポーネントにBreadcrumbLinkをサブコンポーネントとして追加しました。
- Breadcrumbリンクにクリックイベントを追加し、アクションをトリガーするように変更
- コードの可読性を向上させるために、リンクの構造を整理

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Updates the Breadcrumb component with style refinements, improved documentation, comprehensive test coverage, and Figma integration. The changes align with design specifications from the referenced Notion document.

  • Updated link import to use internal Link component and refined styling
  • Restructured tests with helper functions and enhanced coverage for edge cases
  • Enhanced Storybook stories with interactive actions and updated content

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/components/ui/breadcrumb/index.tsx Updated import, enhanced documentation with usage example, removed hover/focus styles from links, changed separator color
src/components/ui/breadcrumb/index.test.tsx Complete test restructure with helper functions, expanded test coverage, and improved organization
src/components/ui/breadcrumb/index.stories.tsx Added action handlers and updated story content for better interactivity
src/components/ui/breadcrumb/index.figma.tsx New Figma Connect integration file for design system alignment
README.md Updated component status to show Figma Connect completion

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@touyou touyou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTMです!

@ore0
ore0 merged commit b1b2ea2 into main Aug 29, 2025
3 checks passed
@ore0
ore0 deleted the fix/breadcrumb branch August 29, 2025 02:11
@ore0 ore0 removed their assignment Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants