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: TabBar の Story を見直し #4949

Merged
merged 7 commits into from
Sep 27, 2024
Merged

docs: TabBar の Story を見直し #4949

merged 7 commits into from
Sep 27, 2024

Conversation

uknmr
Copy link
Collaborator

@uknmr uknmr commented Sep 24, 2024

関連URL

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

概要

Story のあり方を定義し直し、他の見本となる Story を作ろうとしています。

構成は次の通りです。

Story 名 説明 Chromatic 対象
Default デザインシステムサイト用。過渡期として用意 -
Playground コンポーネントごとに必ず設置する Control -
props 系 id や onClick など汎用的なものは省略 -
VRT ペアワイズ法で props の組み合わせを網羅する

TabBar / TabItem のようにサブコンポーネントを持つ場合、コンポーネントごとに作成。

VRT を除いて、Story ごとに表示する状態は一つとしました。

ペアワイズ法の組み合わせ作成は、pict を使いました。

変更内容

確認方法

@uknmr uknmr requested a review from a team as a code owner September 24, 2024 13:56
@uknmr uknmr requested review from oti and yt-ymmt and removed request for a team September 24, 2024 13:56
Copy link

pkg-pr-new bot commented Sep 24, 2024

Open in Stackblitz

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

commit: e2ee5ca

@Qs-F Qs-F self-requested a review September 25, 2024 02:19
export const RegFocusNoBorder = All.bind({})
RegFocusNoBorder.play = () => [...Array(4)].forEach((_) => userEvent.tab())
export const Boardered = {
name: 'bordered',
Copy link
Contributor

Choose a reason for hiding this comment

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

[nits]

変数名がそのまま Story 名になってくれるので、基本的には name の明示は不要に見えます。
(変数名は大文字したいって拘りかもしれませんが)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

変数名を小文字にしたい意図でした!

@s-sasaki-0529
Copy link
Contributor

途中なのであとでまたレビューしますが、概ねの方針は良さそうです!
具体的な CSF の記法とかは今後このコードが基準になっていくのでもう少しつめたいところ。

Copy link
Contributor

@Qs-F Qs-F left a comment

Choose a reason for hiding this comment

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

LGTM

(コンテキストはここ)

Copy link
Contributor

@s-sasaki-0529 s-sasaki-0529 left a comment

Choose a reason for hiding this comment

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

👍️ CSF3になってめちゃくちゃ見やすくなった感じがします!

tags: ['!autodocs'],
}

export const True: StoryObj = {
Copy link
Contributor

Choose a reason for hiding this comment

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

[nits]

細かいですが、StoryObj は T にコンポーネントの型を渡してあげると props が型安全になるので書き心地が良くなります!

export const True: StoryObj<typeof TabBar> = {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

e2ee5ca で対応しました!
すべての Story に書くの面倒だなぁ、default に指定した型でよしなにやってほしいなぁ、と思ってました😇
雛形作る時に自動生成させても良さそうですね。

@uknmr uknmr enabled auto-merge (squash) September 27, 2024 01:35
@uknmr uknmr merged commit 3d2dc74 into master Sep 27, 2024
10 checks passed
@uknmr uknmr deleted the improve-TabBar-story branch September 27, 2024 01:39
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