Skip to content

Linkの修正 - #119

Merged
touyou merged 5 commits into
mainfrom
fix/link
Jul 30, 2025
Merged

Linkの修正#119
touyou merged 5 commits into
mainfrom
fix/link

Conversation

@touyou

@touyou touyou commented Jul 29, 2025

Copy link
Copy Markdown
Member

概要

以下の指摘事項の修正と調整
https://www.notion.so/goodpatch-design-div/Link-cf8f723bc3f04a2e84ed011773011c22?source=copy_link

変更内容

  • isUnderlineを削除し下線バージョンのみに
  • 色調整
  • Figma Connect
  • テスト修正
  • Storybook調整

動作確認

  • pnpm lint を実行
  • pnpm build を実行してプロジェクトが正常にビルドされることを確認
  • pnpm test を実行してテストがすべて成功することを確認

関連 Issue

touyou added 4 commits July 29, 2025 11:45
- isUnderlineプロパティをLinkPropsインターフェースから削除
- Linkコンポーネント内のisUnderlineに関連するコードを削除
- ストーリーからisUnderlineに関連する例を削除
- isUnderlineプロパティを削除し、デフォルトでアンダーラインスタイルを適用
- 外部リンクアイコンの表示ロジックを修正
- テストケースを簡素化し、可読性を向上
- Linkコンポーネントのストーリーにレイアウトを「centered」に設定
- Figmaとの接続を設定
- LinkコンポーネントのプロパティをFigmaにマッピング
- コード例を追加
@touyou
touyou requested a review from ore0 July 29, 2025 04:18
@touyou touyou self-assigned this Jul 29, 2025
Copilot AI review requested due to automatic review settings July 29, 2025 04:18
@vercel

vercel Bot commented Jul 29, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sparkle-design ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 30, 2025 2:25am

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

This PR simplifies the Link component by removing the underline variant option and updating the color scheme. The component now always displays with an underline and uses info color tokens instead of primary colors.

  • Removed isUnderline prop and related variant system, making underlines always visible
  • Updated color scheme from primary-600/700 to info-500/600
  • Added Figma Connect integration for design system synchronization

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/components/ui/link/index.tsx Simplified component by removing variant system and updating color tokens
src/components/ui/link/index.test.tsx Updated tests to reflect removed underline variants and new color scheme
src/components/ui/link/index.stories.tsx Removed underline-related story and controls from Storybook
src/components/ui/link/index.figma.tsx Added new Figma Connect integration file

@@ -1,27 +1,9 @@
import React from "react";
import { VariantProps, cva } from "class-variance-authority";
import { cn } from "@/lib/utils";

Copilot AI Jul 29, 2025

Copy link

Choose a reason for hiding this comment

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

The cn utility import is still present but may no longer be needed since the linkVariants function that used it has been removed. Consider removing this import if it's not used elsewhere in the component.

Copilot uses AI. Check for mistakes.
Comment thread src/components/ui/link/index.tsx Outdated
{ isUnderline, isExternalLink = false, children, className = "", ...props },
ref
) => {
({ isExternalLink = false, children, className = "", ...props }, ref) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

isExternalLinkisOpenInNewにしたいです!
内部リンクであっても新規ウィンドウないしタブで開く物はその手がかりを付けなきゃいけないという観点から命名が変更になりました🙏

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ore0 こちらで修正しました!
d780761

- isExternalLinkをisOpenInNewに変更
- Figma接続の例を更新
- ストーリーとテストでのプロパティ名を修正

@ore0 ore0 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.

LGTMです!ご対応ありがとうございます!

@touyou
touyou merged commit 8621e42 into main Jul 30, 2025
3 checks passed
@touyou
touyou deleted the fix/link branch July 30, 2025 09:38
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