-
Notifications
You must be signed in to change notification settings - Fork 0
カートページの実装 #133
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
カートページの実装 #133
Conversation
| title={book.title} | ||
| thumbnail={book.thumbnail} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
カートにタイトルとサムネイルを追加するのが目的だと理解した.
タイトルはモーダルで表示しないことにしたから削除してもいいのでは?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
消しました
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
| const stockList = range(0, stock); | ||
| const dataList = stock >= volume ? stockList : [...stockList, volume]; | ||
| const strList = dataList.map((data) => data.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この3つは何のための変数だろう?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
もし、volumeをstockが超えてたら0 ~ stock, volumeの要素を持つ配列が選択肢になる。
そうでないなら0 ~ stockまでを選択肢とする
そういった選択肢の配列
| setSelectedCartBook([]); | ||
| }, []); | ||
|
|
||
| // volumeがstockを超えていないかチェックする |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
volumeとstockを日本語で書いてあげた方が分かりやすいと思った
// 貸出数が蔵書数を超えていないかチェックするThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
変更しました
frontend/app/utils/cart.ts
Outdated
| export const addBookToCart = ( | ||
| cart: CartProps[], | ||
| bookId: number, | ||
| stock: number, | ||
| ) => { | ||
| const index = cart.findIndex((cartBook) => cartBook.id === bookId); | ||
| if (index !== -1) { | ||
| cart[index].volume += 1; | ||
| } else { | ||
| cart.push({ | ||
| id: bookId, | ||
| stock, | ||
| thumbnail: '', | ||
| volume: 1, | ||
| }); | ||
| } | ||
| return cart; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addBooksToCart に要素数1の配列を渡せば,同じことが実現できると思う.
そうしなかったことに何か理由がある?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
うーむ、特に意味はないかも
強いて言えば、引数に長さ1の配列を書くことに違和感を覚えただけかもしれん
addBooksToCartにまとめるわ
| interface BookCardHeaderProps { | ||
| id: number; | ||
| stock: number; | ||
| title: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title 消してもいいのでは?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
全部消したつもりやったけど残ってたわ
| }; | ||
|
|
||
| // 選択されている本のIDと表示する本のIDを比較する関数 | ||
| const selectedCheck = (element: SelectedBookProps) => element.id === id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
関数名は動詞から始めた方が機能が伝わりやすいと思う.
checkSelectedisSelected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
関数の設計思想としてはfunctionよりチェックのラベルみたいなニュアンスで使ってるからisSelectedにしたわ
| // 選択されている本のIDと表示する本のIDを比較する関数 | ||
| const selectedCheck = (element: SelectedBookProps) => element.id === id; | ||
|
|
||
| const selectedBookAdd = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switchBookSelecttoggleBookSelect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switchBookSelectにしたわ
| '1400px': 6, | ||
| '1700px': 7, | ||
| }} | ||
| spacing={{ base: 10, '300px': 'xl' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
画面幅が300ピクセル以上の場合に xl サイズの間隔が適用されると理解した.
一発で理解できなかったので,コメントを残しておきたい.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コメント残しました
| import { removeBooksFromCart } from '~/utils/cart'; | ||
|
|
||
| interface CartSelectedDialogProps { | ||
| handleLoanPatch: () => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ハンドラ名は handle<ハンドルするイベント名> という形式にした方がいいと思う
handleBorrowButtonClickとか
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleLoanPatchを書き換えたわ
他のハンドラ名は確認したけど変えてない
| w="50%" | ||
| data={strList} | ||
| value={String(volume)} | ||
| error={volume > stock} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
便利だね
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
やったあ
やったこと
確認した方法
pnpm run devで確認したスクリーンショット
自動生成したコード