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

Multiple Selectで複数選択する際に都度閉じないように修正 #270

Merged
merged 5 commits into from
Mar 30, 2021

Conversation

hirokikondo86
Copy link
Contributor

@hirokikondo86 hirokikondo86 commented Mar 29, 2021

Ref

close voyagegroup/fluct_XDC#124

やったこと

  • closeMenuOnSelect={false}を追加して選択時に閉じないように修正
  • StorybookのMultiple Select コンポーネントで最後(最初)の一つを削除した際に、エラーを吐いていたので修正

スクショ

Before

fb70f820a253a98ba0123563bf2c1b1b

After

15be6db4d3bd4b1c3796fcd9add99e9c

@hirokikondo86 hirokikondo86 self-assigned this Mar 29, 2021
@hirokikondo86 hirokikondo86 requested a review from a team as a code owner March 29, 2021 05:55
@hirokikondo86 hirokikondo86 changed the title Fix/#124/mult select Multiple Selectで複数選択する際に一回一回閉じないように修正 Mar 29, 2021
@hirokikondo86 hirokikondo86 changed the title Multiple Selectで複数選択する際に一回一回閉じないように修正 Multiple Selectで複数選択する際に都度閉じないように修正 Mar 29, 2021
@@ -183,6 +183,7 @@ const Select: SelectComponent = ({
<Styled.Container minWidth={minWidth} isDisabled={isDisabled}>
<ReactSelect
isClearable
closeMenuOnSelect={false}
Copy link
Contributor

Choose a reason for hiding this comment

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

外から変更できるようにした方が使いやすいので、引数で挙動変えられる(デフォルトは false)な方がいいと思いますー

Copy link
Contributor Author

Choose a reason for hiding this comment

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

確かに…!修正しました!

@noronaoki
Copy link
Contributor

@hirokikondo86 Singleセレクトの時は勝手に閉じたほうが良くない?

@hirokikondo86
Copy link
Contributor Author

hirokikondo86 commented Mar 29, 2021

@noronaoki
確かに…🤔
もう一度見直します!

@noronaoki
Copy link
Contributor

closeMenuOnSelect={false}

オプショナルなのね。そしたら実装する人が選べるからいいか。
Storybook上ではSingleの時はtrueで実装しておいてもらえると良さそう。

@ryokosuge ryokosuge self-requested a review March 29, 2021 06:29
Copy link
Contributor

@ryokosuge ryokosuge left a comment

Choose a reason for hiding this comment

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

👍

@hirokikondo86
Copy link
Contributor Author

@noronaoki

Storybook上ではSingleの時はtrueで実装しておいてもらえると良さそう。

修正しました🙇‍♂️

@hirokikondo86
Copy link
Contributor Author

シングルのSelectを使用していたCodeRecipes/Form/OverviewのSelectもcloseMenuOnSelect={true}で選択時に閉じるように実装しました。

@noronaoki
Copy link
Contributor

@hirokikondo86 LGTM

@hirokikondo86 hirokikondo86 merged commit d9cc817 into master Mar 30, 2021
@hirokikondo86 hirokikondo86 deleted the fix/#124/mult-select branch March 30, 2021 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants