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

docs(Chip): Story を見直し #5078

Merged
merged 2 commits into from
Nov 6, 2024
Merged

docs(Chip): Story を見直し #5078

merged 2 commits into from
Nov 6, 2024

Conversation

uknmr
Copy link
Collaborator

@uknmr uknmr commented Nov 5, 2024

関連URL

https://smarthr.atlassian.net/browse/SHRUI-1116

概要

#4949 の方針に沿って、Chip の Story を見直しました。

変更内容

確認方法

Storybook や Chromatic で確認してください。

@uknmr uknmr requested a review from a team as a code owner November 5, 2024 01:57
@uknmr uknmr requested review from misako0927 and hiroki0525 and removed request for a team November 5, 2024 01:57
Copy link

pkg-pr-new bot commented Nov 5, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/kufu/smarthr-ui@5078

commit: 124d8c3

@@ -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',
Copy link
Collaborator Author

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) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

size って未定義だとデフォルト s で上書きされちゃうので、見た目上の差異がないように見えました。
Playground 上でも一度チェック入れたら外せなくて、どう指定すればサイズが変わるのかがよくわからなかったです!
CleanShot 2024-11-06 at 11 45 37

Copy link
Collaborator Author

@uknmr uknmr Nov 6, 2024

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

Copy link
Contributor

Choose a reason for hiding this comment

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

👍️ 選択肢がSしかない意図が汲み取れるので良さそうです!

Storybook を見ただけだと、未指定時と挙動が変わらないのが仕様なのか不具合なのかまでは判断できないので、props 自体廃止しちゃっても良いかもですね。

@uknmr uknmr merged commit abe4a11 into master Nov 6, 2024
14 checks passed
@uknmr uknmr deleted the improve-Chip-story branch November 6, 2024 08:54
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.

2 participants