Skip to content

Conversation

@Kosei805
Copy link
Contributor

@Kosei805 Kosei805 commented Nov 17, 2024

やったこと

  • やったこと (コミットのハッシュ)
  • マイページのurlを短くし、わかりやすくした。 0629d69
  • Cartにおいて、表示データとアクションで送信するデータでvolumeの値を共有できていなかったので、書き換えた 72944a3
  • マイページの実装をした d920e85

確認した方法

  • pnpm run devで確かめた
  • 借りている本を一部だけ返すなどできるか確認した

スクリーンショット

状態 画像
基本的なページ image
本を選択する image
本を全て返却する image

自動生成したコード

  • ファイル名

@Kosei805 Kosei805 requested a review from kimurash November 17, 2024 14:30
@Kosei805 Kosei805 self-assigned this Nov 17, 2024
@Kosei805 Kosei805 added the frontend frontend development label Nov 17, 2024
Comment on lines 25 to 27
<Button fz="xs" color="blue" onClick={handleReturnButtonClick}>
返却する
</Button>
Copy link
Member

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.

赤色にした

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 14 to 15
<Text size={rem(50)}>{name}</Text>
<Text>{email}</Text>
Copy link
Member

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.

プロフィール表示コンポーネント調整の時についでに直してくれい

Comment on lines 147 to 158
const handlePaginationChange = (newPage: number) => {
let url = '/home/me';
let initial = true;
if (limit) {
url =
initial === true ? `${url}?limit=${limit}` : `${url}&limit=${limit}`;
initial = false;
}
url =
initial === true ? `${url}?page=${newPage}` : `${url}&page=${newPage}`;
navigate(url);
};
Copy link
Member

Choose a reason for hiding this comment

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

URLSearchParam を使って書きたい

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.

@@ -0,0 +1,9 @@
import { atom } from 'jotai';
import { CartProps } from './cartAtom';
import { atomWithStorage, createJSONStorage } from 'jotai/utils';
Copy link
Member

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.

消しました

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const isSelected = (element: CartProps) => element.id === id;

// 該当する本のvolumeを変更する
const handleChangeVolume = (id: number, value: number) => {
Copy link
Member

Choose a reason for hiding this comment

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

僕だったら handleVolumeChange にするかな

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.

}),
);
const target = selectedCartBook.find(isSelected);
// volumeを選択されていた本がすでに選択されている場合
Copy link
Member

Choose a reason for hiding this comment

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

「volumeを選択されていた」の部分が理解できなかった

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.

Comment on lines +42 to +50
if (element.id === id) {
return {
id: element.id,
stock: element.stock,
thumbnail: element.thumbnail,
volume: value,
};
}
return element;
Copy link
Member

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.

これは、elementがvolumeが選択された本やったら、volumeを更新して、それ以外なら、elementをそのままにするって処理です。これをmapで繰り返して、配列を更新してる

</Stack>
</Dialog>
);
return <div>CartSelectedDialog</div>;
Copy link
Member

Choose a reason for hiding this comment

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

🙏

const isSelected = (element: CartProps) => element.id === id;

// 該当する本のvolumeを変更する
const handleChangeVolume = (id: number, value: number) => {
Copy link
Member

Choose a reason for hiding this comment

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

僕だったら handleVolumeChange にするかな

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.

Copy link
Member

Choose a reason for hiding this comment

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

このコンポーネントのUIは考え直したいね

Copy link
Member

Choose a reason for hiding this comment

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

一応作ってくれたんやね

// 操作する貸出履歴のvolumesを管理するAtom
export const displayLoanAtom = atom<CartProps[]>([]);
Copy link
Member

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.

volumeを操作するデータが欲しいからやね。
チェックボックスにチェックが入ってないデータもvolumeを変えたいから、selected...とは別にデータが必要だった感じ。

@kimurash
Copy link
Member

スクリーンショットありがとう
手元で起動しなくていいから時短になる

session.flash('success', 'ログインに成功しました');

return redirect('/home/mypage', {
return redirect('/home/me', {
Copy link
Member

Choose a reason for hiding this comment

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

ここを書き換えるとログインページのテストが落ちると思う

テストコードも修正しておこう

Copy link
Member

@kimurash kimurash Nov 18, 2024

Choose a reason for hiding this comment

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

修正した bbdf330

@kimurash kimurash self-assigned this Nov 19, 2024
@kimurash
Copy link
Member

kimurash commented Nov 19, 2024

プロフィールの部分を変更した

image

@Kosei805
Copy link
Contributor Author

ユーザーデータのコンポーネント
LGTMです!!

@kimurash kimurash merged commit 4842573 into main Nov 19, 2024
3 checks passed
@kimurash kimurash deleted the 68-my-page branch November 19, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend frontend development

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

マイページの実装

3 participants