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

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

Conversation

misako0927
Copy link
Contributor

関連URL

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

概要

変更内容

確認方法

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

await userEvent.click(helpMessage[0]) // カーソルの点滅によるVRTのフレーキーを避けるためにフォーカスを移動する
}

VRT.play = playSingle
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pict以外にプルダウンの展開のためplayfunctionは残しています

Copy link

pkg-pr-new bot commented Nov 27, 2024

Open in Stackblitz

npm i https://pkg.pr.new/kufu/smarthr-ui@5143

commit: aea51be

@misako0927 misako0927 marked this pull request as ready for review November 27, 2024 23:46
@misako0927 misako0927 requested a review from a team as a code owner November 27, 2024 23:46
@misako0927 misako0927 requested review from oti, masa0527, Qs-F, s-sasaki-0529 and uknmr and removed request for a team, oti and masa0527 November 27, 2024 23:46
@misako0927
Copy link
Contributor Author

calendarのVRTが本日の日付の◯でflakyになっているのですが、以下の2択の修正で悩んでいます....

  1. 表示する月を過去に直す
  2. calendarに今日の日付を渡せるようにして固定する

デザインのチェック的には2が良さそうなものの、機能として今日の日付を渡すことがないと思うので、1でもいいかも、、、で甲乙つけ難く。
ご意見ありましたらいただけると幸いです。。。

@s-sasaki-0529
Copy link
Contributor

あ、しまった...。日付のことすっかり考慮してなかったです…。
前職ではテスト上では Date をモックして常に固定の日付になるようなハックをしていた覚えがありますね…。

1がベターそうかなと思いましたがシュッとできそうでしょうか…?

),
args: {
items: Object.values(defaultItems),
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

default に書いてしまってよさそう。

Suggested change
},
},
play: playSingle,

import { Stack } from '../../../Layout'
import { SingleComboBox } from '../SingleComboBox'

const defaultItems = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

imo: SingleComboBox.stories と同じであれば使いまわしてもよさそう

Copy link
Contributor Author

Choose a reason for hiding this comment

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

単純にexportするとstoryとしてカウントされてしまうので、excludeStoriesを設定しました。もしもっと良い方法があれば教えてもらえると助かります🙏
https://github.com/kufu/smarthr-ui/pull/5143/files#diff-83bd9112f2364dba079090ba55ef94e547ef3a1d6de01a36fbd5f5b1734bb2e7R86

args: {
items: Object.values(defaultItems),
selectedItem: null,
defaultItem: defaultItems['option 1'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

必須に見えてしまうので消しとくとよさそう

Suggested change
defaultItem: defaultItems['option 1'],

},
}

export const DefaultItem: StoryObj<typeof SingleComboBox> = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

視覚的に選択されてないように見える(onSelect などを使って selectedItem を埋めてあげないと駄目ってこと?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そんな感じがしているのですが、なんか変ですよね・・?(バグ・・?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

onSelectで反映するように変更しました!

render: (args) => (
<form>
<Stack gap={1}>
<FormControl title="prefixなし" dangerouslyTitleHidden>
Copy link
Collaborator

Choose a reason for hiding this comment

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

コードが複雑になってしまいコンポーネントの責務が見えにくくなってしまうので、

<Stack>
  <SingleComboBox />
  <SingleComboBox />
</Stack>

くらいの記述に収めたいです!(lint の警告はファイルごと無視しちゃって問題ないです)

export const Width: StoryObj<typeof SingleComboBox> = {
name: 'width',
args: {
width: '500px',
Copy link
Collaborator

Choose a reason for hiding this comment

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

基本的に px 固定しないので、em くらいにはしておきたいです!

export const DropdownWidth: StoryObj<typeof SingleComboBox> = {
name: 'dropdownWidth',
args: {
dropdownWidth: '300px',
Copy link
Collaborator

Choose a reason for hiding this comment

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

基本的に px 固定しないので、em くらいにはしておきたいです!

render: (args) => (
<Stack align="flex-start" gap={2}>
<form>
<FormControl className="shr-mb-[15rem]" title="デフォルト" dangerouslyTitleHidden>
Copy link
Collaborator

Choose a reason for hiding this comment

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

imo: 大外の Stack を h-screen して、最下部のコンポーネントを開くと細かい調整を消せそう

Comment on lines 102 to 103
<form>
<FormControl className="shr-mb-[15rem]" title="デフォルト" dangerouslyTitleHidden>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Story の eslint は無視で大丈夫です(form とか FormControl とか消して ok

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