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(Calendar): Story を見直し #5097

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

s-sasaki-0529
Copy link
Contributor

@s-sasaki-0529 s-sasaki-0529 commented Nov 12, 2024

関連URL

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

概要

  • docs: TabBar の Story を見直し #4949 の方針に沿って、Story を見直しました。
  • Calendar コンポーネントは DatePicker 内で使用する専用のコンポーネントなので、外部への export をやめて、ディレクトリも DatePicker 配下に移動しました

変更内容

確認方法

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

Copy link

pkg-pr-new bot commented Nov 12, 2024

Open in Stackblitz

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

commit: be12e42

to={args.to ? dayjs(args.to).toDate() : undefined}
onSelectDate={args.onSelectDate}
/>
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ここ、コメントの通りなんですが、argTypes を指定しないと Playground がクラッシュするし、指定すると Date でなく UnixTime (number) が入ってくる仕様みたいなので、なくなく描画時に Date に変換するようにしてます。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ドキュメントにも記載されてた。 future とは。
image

focusVisible: ['button'],
},
},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

VRT と VRTFocusVisible を一つにまとめたかったんですが、 play 関数でカレンダーのうち一つを年選択に切り替えるのと、focusVisible を適用する設定が衝突して描画結果が安定しないことがあるので分離してます。

@s-sasaki-0529 s-sasaki-0529 marked this pull request as ready for review November 12, 2024 00:57
@s-sasaki-0529 s-sasaki-0529 requested a review from a team as a code owner November 12, 2024 00:57
@s-sasaki-0529 s-sasaki-0529 requested review from misako0927 and atzzCokeK and removed request for a team November 12, 2024 00:57
},
chromatic: { forcedColors: 'active' },
},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

これ VRT うまく動かないなぁって試行錯誤してたけど、そもそも普通に強制カラーモードだとフォーカス見えないのか…。

CleanShot 2024-11-12 at 10 17 24

@uknmr
Copy link
Collaborator

uknmr commented Nov 12, 2024

ついでに src/index.ts から export をやめて DatePicker 配下に移動しても良いと思ったんですが、どうですか? @s-sasaki-0529

@s-sasaki-0529
Copy link
Contributor Author

s-sasaki-0529 commented Nov 12, 2024

ついでに src/index.ts から export をやめて DatePicker 配下に移動しても良いと思ったんですが、どうですか?

あ、Calendar って単体で提供されてるんだって驚きつつ Storybook 対応してましたけど、やっぱ DatePicker とセットで使う前提な感じなんですかね。であれば非公開コンポーネントにしちゃっても良さそうですねー。

@uknmr
Copy link
Collaborator

uknmr commented Nov 12, 2024

DatePicker のために存在してるので非公開の対応もお願いします!

@s-sasaki-0529 s-sasaki-0529 marked this pull request as draft November 12, 2024 10:26
@s-sasaki-0529 s-sasaki-0529 marked this pull request as ready for review November 12, 2024 10:41
@s-sasaki-0529
Copy link
Contributor Author

@uknmr 非公開コンポーネントに変更してディレクトリ移動もしてみました!

@@ -63,7 +63,6 @@ export { InformationPanel } from './components/InformationPanel'
export { Tooltip } from './components/Tooltip'
export { BottomFixedArea } from './components/BottomFixedArea'
export { ErrorScreen, MessageScreen } from './components/ErrorScreen'
export { Calendar } from './components/Calendar'
Copy link
Contributor

@misako0927 misako0927 Nov 14, 2024

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

まさか使ってるプロダクトがあるとは……!(sasaki さんごめんなさい🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

あれ…。Stats で利用0を確認したんですが、漏れるときは漏れるのか…。
https://github.com/kufu/smarthr-ui-stats

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そもそも Stats にこのプロダクトが反映されてないだけでした…。
というのもあるので今後はちゃんとコード検索します!

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