募金登録の金額入力フォームにプルダウンを追加する #659

TkymHrt · 2023-11-17T14:20:18Z

対応Issue

resolve #649

概要

募金登録の際に金額の入力フォームにプルダウン(選択肢)を追加する。
datalist要素を利用して実装する。

画面スクリーンショット等

テスト項目

学内募金の学内募金登録モーダルを開き、「金額」の入力フォームをクリックすると金額の候補が表示される。
登録した項目の編集モーダルを開き、「金額」の入力フォームをクリックすると金額の候補が表示される。

備考

https://developer.mozilla.org/ja/docs/Web/HTML/Element/datalist

imaimai17468

Kubosaka

動作問題なしです。
少し短くできそうなところコメントしました。
いまいまいかのぶに見てもらって修正しなくても良いかどうか判断して欲しいです。

Kubosaka · 2023-11-18T05:34:29Z

view/next-project/src/components/common/Input/Input.tsx

@@ -11,23 +11,36 @@ interface Props {
  onChange?: (e: React.ChangeEvent<HTMLInputElement>) => void;
  children?: React.ReactNode;
  type?: string;
+  datalist?: { id: number; name: string }[];
+  listKey?: string;
+  enableDatalist?: boolean;


Suggested change

enableDatalist?: boolean;

好みですが、個人的にはdalalistがあるかどうかで判断できるかなって思いました。

datalistとlistkeyの存在を同時に確かめたい場合は、一緒にしてしまってもいいと思います

datalist?: { key: string; data: { id:number; name: string}[]; }

Kubosaka · 2023-11-18T05:35:34Z

view/next-project/src/components/common/Input/Input.tsx

+        value={props.value}
+        onChange={props.onChange}
+        type={props.type}
+        list={props.enableDatalist ? props.listKey : undefined}


Suggested change

list={props.enableDatalist ? props.listKey : undefined}

list={props.datalist && props.listKey}

listにはpropsにdatalistがあれば、props.listkeyを渡すで十分かと思います。

Kubosaka · 2023-11-18T05:36:24Z

view/next-project/src/components/common/Input/Input.tsx

+      >
+        {props.children}
+      </input>
+      {props.enableDatalist && (


Suggested change

{props.enableDatalist && (

{props.datalist && (

props.datalistがあるかどうかで判断できるかなって思います。

enableDatalistで切り替えない場合はlistkeyのチェックも入れた方が安全になります

Suggested change

{props.enableDatalist && (

{props.enableDatalist && props.listkey && (

Kubosaka · 2023-11-18T05:36:49Z

view/next-project/src/components/fund_information/AddModal.tsx

+            onChange={handler('price')}
+            datalist={DONATION_AMOUNT}
+            listKey='amoutOptions'
+            enableDatalist={true}


Suggested change

enableDatalist={true}

上のように修正すれば消せます。

ちなみにtrueを渡さなくても

Suggested change

enableDatalist={true}

enableDatalist

のみでtrueのように動作します

Kubosaka · 2023-11-18T05:37:03Z

view/next-project/src/components/fund_information/EditModal.tsx

+            onChange={handler('price')}
+            datalist={DONATION_AMOUNT}
+            listKey='amoutOptions'
+            enableDatalist={true}


Suggested change

enableDatalist={true}

ここも同様です。

Kubosaka · 2023-11-18T05:37:42Z

view/next-project/src/constants/donationAmount.ts

constantで定数を定義できてて良いと思います！

imaimai17468

2点、実装とは関係ないですが、

PRのタイトルは意味の伝わるものがいいです
- 英語でも日本語でもいいので、何をどうしたかを書くとPRの内容がよりわかりやすいと思います
commit messageをもう少し詳細に書いた方がいいと思います
- 例えばこのPRだとAdd attribute to AddModalが2個ありますが、それぞれ何がどう違う作業なのかぱっと見わかりません（同じなら1つのcommitにしましょう）
- これも別に日本語でいいので、どこで何をしたか書けるとcommitの趣旨が伝わりやすいです

imaimai17468 · 2023-11-20T13:53:25Z

view/next-project/src/components/common/Input/Input.tsx

+      {props.datalist?.key && props.datalist?.data && (
+        <datalist id={props.datalist.key}>
+          {props.datalist.data.map((option) => (
+            <option key={option.id} value={option.name} />
+          ))}
+        </datalist>
+      )}


datalistが存在すればkeyもdataも存在すると思うので、datalistの存在だけ確認すればいいと思います

Suggested change

{props.datalist?.key && props.datalist?.data && (

<datalist id={props.datalist.key}>

{props.datalist.data.map((option) => (

<option key={option.id} value={option.name} />

))}

</datalist>

)}

{props.datalist && (

<datalist id={props.datalist.key}>

{props.datalist.data.map((option) => (

<option key={option.id} value={option.name} />

))}

</datalist>

)}

imaimai17468

Kubosaka

LGTM!

TkymHrt added 4 commits November 17, 2023 22:41

[feat] Add Donation-Amount Data-List

0048bf3

[feat] Add datalist

04572cd

[feat] Add attribute to AddModal

2c34d4a

[feat] Add attribute to EditModal

9ea1599

TkymHrt requested review from imaimai17468, KazumaSun and Kubosaka November 17, 2023 14:20

formatted by workflow

02bfca5

This comment was marked as resolved.

Sign in to view

imaimai17468 approved these changes Nov 18, 2023

View reviewed changes

Kubosaka reviewed Nov 18, 2023

View reviewed changes

TkymHrt self-assigned this Nov 18, 2023

imaimai17468 reviewed Nov 18, 2023

View reviewed changes

TkymHrt changed the title ~~Feat/yama/649 add dropdown~~ Feat/yama/649 募金登録の金額入力フォームにプルダウンを追加する Nov 20, 2023

TkymHrt changed the title ~~Feat/yama/649 募金登録の金額入力フォームにプルダウンを追加する~~ 募金登録の金額入力フォームにプルダウンを追加する Nov 20, 2023

TkymHrt and others added 3 commits November 20, 2023 19:54

[fix] 一部のpropsをdatalistに統合

6fec28d

[fix] 統合されたpropsから属性値を修正

2d6851e

formatted by workflow

4e8efc8

imaimai17468 reviewed Nov 20, 2023

View reviewed changes

[fix] datalistの追加判定を修正

946c425

imaimai17468 approved these changes Nov 21, 2023

View reviewed changes

Kubosaka approved these changes Nov 23, 2023

View reviewed changes

Kubosaka merged commit c26c10e into develop Nov 23, 2023
2 checks passed

Kubosaka deleted the feat/yama/649-Add-dropdown branch November 23, 2023 13:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

募金登録の金額入力フォームにプルダウンを追加する #659

募金登録の金額入力フォームにプルダウンを追加する #659

TkymHrt commented Nov 17, 2023

This comment was marked as resolved.

imaimai17468 left a comment

Kubosaka left a comment •

edited

Loading

Kubosaka Nov 18, 2023

imaimai17468 Nov 18, 2023

Kubosaka Nov 18, 2023

Kubosaka Nov 18, 2023

imaimai17468 Nov 18, 2023

Kubosaka Nov 18, 2023

imaimai17468 Nov 18, 2023

Kubosaka Nov 18, 2023

Kubosaka Nov 18, 2023

imaimai17468 left a comment

imaimai17468 Nov 20, 2023

imaimai17468 left a comment

Kubosaka left a comment

	list={props.enableDatalist ? props.listKey : undefined}
	list={props.datalist && props.listKey}

	{props.enableDatalist && (
	{props.enableDatalist && props.listkey && (

募金登録の金額入力フォームにプルダウンを追加する #659

募金登録の金額入力フォームにプルダウンを追加する #659

Conversation

TkymHrt commented Nov 17, 2023

対応Issue

概要

画面スクリーンショット等

テスト項目

備考

This comment was marked as resolved.

imaimai17468 left a comment

Choose a reason for hiding this comment

Kubosaka left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imaimai17468 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imaimai17468 left a comment

Choose a reason for hiding this comment

Kubosaka left a comment

Choose a reason for hiding this comment

LGTM!

Kubosaka left a comment •

edited

Loading