Buttonによる完成状態の定義 - #108
Merged
Merged
Conversation
…ops mapping - Changed Button component theme from secondary to neutral. - Updated Figma connection URL and props mapping in index.figma.tsx. - Refactored Button stories to reflect theme changes. - Adjusted Button tests to validate neutral variant. - Modified Button styles for better consistency and accessibility. - Added new Breadcrumb and Tabs components with appropriate structure and styles.
- @figma/code-connectのバージョンを1.3.4にアップグレード - icon-button、icon、spinnerコンポーネントのFigma接続を更新 - 自動生成されたコメントを修正し、Figmaプロパティのマッピングを明確化
- Iconコンポーネントのインポートパスを更新 - Spinnerコンポーネントのインポートパスを更新
- IconButtonコンポーネントのインポートパスを修正 - Spinnerコンポーネントのインポートパスを修正
…s; remove disabled styling test and ref forwarding test fix(Modal): change button theme from secondary to neutral in stories; adjust modal content class for consistency fix(Spinner): format code for consistency in Figma integration
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
- Buttonコンポーネントをindex.tsからエクスポートするように変更しました。
- ボタンのテーマオプションを「secondary」から「neutral」に変更 - ストーリーのレンダリング方法を関数からオブジェクト形式に変更 - ボタンのスタイルを修正し、テキストカラーを「text-white」に更新
There was a problem hiding this comment.
Pull Request Overview
This PR finalizes the Button component implementation and syncs Figma/registry configurations across multiple UI components.
- Refactored
Buttonwith updated CVA classes, removedforwardRef, and revised prop types. - Updated Figma Code Connect integrations for Spinner, Icon, IconButton, and Button.
- Adjusted theme values (
secondary→neutral) in modal/dialog and refreshed registry metadata.
Reviewed Changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/ui/spinner/index.figma.tsx | Updated Figma integration for Spinner |
| src/components/ui/modal/index.tsx | Adjusted modal container styling (removed gap-4) |
| src/components/ui/modal/index.stories.tsx | Changed Button theme in modal stories to neutral |
| src/components/ui/icon/index.figma.tsx | Updated Figma integration for Icon |
| src/components/ui/icon-button/index.figma.tsx | Updated Figma integration for IconButton |
| src/components/ui/dialog/index.tsx | Changed DialogCancel theme to neutral |
| src/components/ui/button/item.json | Reordered registry metadata for Button component |
| src/components/ui/button/index.tsx | Refactored Button implementation and styling variants |
| src/components/ui/button/index.test.tsx | Updated tests to match new Button classes and variants |
| src/components/ui/button/index.stories.tsx | Revised Storybook meta typing and control options |
| src/components/ui/button/index.figma.tsx | Updated Figma integration for Button |
| registry.json | Reorganized registry entries and added Tabs component |
| public/r/textarea.json | Enhanced Textarea registry documentation with examples |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)
src/components/ui/button/index.tsx:306
- The Button component no longer forwards refs to its underlying element after removal of
React.forwardRef, which is a breaking change for consumers relying on ref access. Consider restoring ref forwarding or documenting this change clearly.
function Button({
src/components/ui/button/index.tsx:340
- The computed
disabledattribute is set before spreading...props, which allows an explicitdisabledprop to override the computed state. Consider spreading...propsfirst or merging props to avoid unintended overrides.
disabled={isButtonDisabled}
src/components/ui/modal/index.tsx:155
- The
gap-4utility was removed from the modal content container, which may collapse the intended spacing between child elements. Consider reintroducing a suitable gap or adjusting the layout to preserve spacing.
"z-50 flex flex-col gap-0 w-full max-h-[calc(100vh-80px)] bg-background border py-4 rounded-modal data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 fixed top-[50%] left-[50%] translate-x-[-50%] translate-y-[-50%] shadow-lg duration-200",
touyou
commented
Jul 16, 2025
Member
Author
There was a problem hiding this comment.
完全体定義にあたって基準表見ながらセルフチェック
- theme使えてるか？→使えているはず。フォーカスリングなどの扱いだけ要議論かも（もしここまで整える場合はringのトークン定義が必要？）
- cva→必要な場所なので問題なし。適宜join作戦で見やすくなったはず
- registry...一旦最小限できちんと定義できているから問題ないはず？
- Storybook...コメントを引っ張ってくる挙動などは再整理できた状態。複数コンポーネントならべるストーリーも書き方を直してControlsが使えるように。残論点もしあるとしたら各ストーリーに説明をつけるかどうか
- README更新...済んでいる
- Figma Code Connect...挙動確認までできた
- テスト...t-wada方式で生成してもらったものがある
残論点
- フォーカスリングなどの色定義について
- registry,storybookの現状に何かもう少し要求あれば
- リポジトリのリファクタリング、モノレポ→変えるイメージがあれば、なければこのままでも自分は良いかと考えてました
- registryのgitignore...そもそも可能なのかが微妙なので優先度は下げたいです
ore0
reviewed
Jul 16, 2025
There was a problem hiding this comment.
code connectでspinnerのインスタンスに使われているタイポグラフィの大きさを取得してsizepropsにタイポグラフィのサイズを反映できるとより使い勝手が良くなりそうです。
There was a problem hiding this comment.
code connectでiconのインスタンスに使われているタイポグラフィの大きさとアイコン名を取得してpropsに反映できるとより使い勝手が良くなりそうです。
| }, | ||
| example: props => ( | ||
| <IconButton | ||
| icon="edit" |
| "shrink-0 outline-none", | ||
| "focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2", | ||
| "aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", | ||
| ].join(" "), |
There was a problem hiding this comment.
Arrayをjoin()する形式ってどう思います...？
Tabsを実装するときにcopilotに言われるがまま採用しちゃったんですけど、本当にいいアプローチなのか疑問に思ってきています。
Member
Author
There was a problem hiding this comment.
自分はあまりにも長くなるので可読性の観点でいいのかなと思ってました。
何かバッドノウハウである明確な理由が見つかれば避ければいいとは思いますが、自分は好意的に捉えたので採用しています
Comment on lines
+43
to
+49
| // No matching props could be found for these Figma properties: | ||
| // "isFocused": figma.boolean('isFocused'), | ||
| // "isSuffixIcon": figma.boolean('isSuffixIcon'), | ||
| // "isPrefixIcon": figma.boolean('isPrefixIcon'), | ||
| // "label": figma.string('label'), | ||
| // "prefixIcon": figma.instance('prefixIcon'), | ||
| // "suffixIcon": figma.instance('suffixIcon') |
Member
Author
There was a problem hiding this comment.
今回AIマッチングを使ってて生成したコードだったので、改めて一回コメントアウトして動くのか確認してみますね🙆♂️
- labelプロパティの名前を修正 - suffixIconとprefixIconの設定を追加
- アイコンボタンの例にアイコンのサイズとフィルを追加 - スピナーの例にサイズを追加
- フォーカスリングの色をカスタムプロパティに変更
ore0
reviewed
Jul 17, 2025
| "cursor-pointer disabled:cursor-not-allowed", | ||
| "shrink-0 outline-none", | ||
| "focus-visible:ring-2 focus-visible:ring-[var(--color-ring-normal)] focus-visible:ring-offset-2", | ||
| "aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", |
There was a problem hiding this comment.
aria-invalidの方針が決まって方針に伴う実装ができていれば後は問題なさそうです！
- aria-invalidのリングスタイルを削除しました。
- Updated the spinner component's Figma connection to include size as a prop. - Created a new script to import icons from Figma, generating a file with connections for each icon component.
5 tasks
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
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.
概要
Buttonを改めて実装して完成形を定義すること
変更内容
動作確認
pnpm lintを実行pnpm buildを実行してプロジェクトが正常にビルドされることを確認pnpm testを実行してテストがすべて成功することを確認関連 Issue