-
Notifications
You must be signed in to change notification settings - Fork 140
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
docs(Chip): Story を見直し #5078
docs(Chip): Story を見直し #5078
Conversation
commit: |
@@ -5,21 +5,21 @@ type Props = PropsWithChildren<VariantProps<typeof chip> & ComponentPropsWithout | |||
|
|||
const chip = tv({ | |||
base: [ | |||
'smarthr-ui-Chip shr-border-shorthand shr-rounded-full shr-leading-none', | |||
'smarthr-ui-Chip', | |||
'shr-border-shorthand shr-rounded-full shr-bg-white shr-text-black shr-leading-none', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disabled=false
のスタイルも集結(twMerge の設定見直しにより、明示的に分ける必要がなくなった)
name: 'size', | ||
render: (args) => ( | ||
<Stack align="flex-start"> | ||
{[undefined, 's'].map((size) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
現状 size="s" がデフォルトで、サイズ変更もできないので未定義で使ってもらっても問題ないんですよねぇ。
デフォルトで size="s"
がコード上に含まれないようにしてみた & radio → select に変えてみたんですがどうかしら 124d8c3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍️ 選択肢がSしかない意図が汲み取れるので良さそうです!
Storybook を見ただけだと、未指定時と挙動が変わらないのが仕様なのか不具合なのかまでは判断できないので、props 自体廃止しちゃっても良いかもですね。
関連URL
https://smarthr.atlassian.net/browse/SHRUI-1116
概要
#4949 の方針に沿って、Chip の Story を見直しました。
変更内容
確認方法
Storybook や Chromatic で確認してください。