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

ユーザー一覧の実装 #137

Merged
merged 8 commits into from
Nov 15, 2024
Merged

ユーザー一覧の実装 #137

merged 8 commits into from
Nov 15, 2024

Conversation

Kosei805
Copy link
Contributor

@Kosei805 Kosei805 commented Nov 14, 2024

やったこと

  • やったこと (コミットのハッシュ)
  • Paginationやアクションレスポンスのタイプを切り分ける 85cc374
  • ユーザー一覧ページの実装 d6c9278

確認した方法

  • pnpm run devで動かした

スクリーンショット

image

自動生成したコード

  • ファイル名

@Kosei805 Kosei805 requested a review from kimurash November 14, 2024 07:47
@Kosei805 Kosei805 self-assigned this Nov 14, 2024
@Kosei805 Kosei805 added the frontend frontend development label Nov 14, 2024
@kimurash kimurash changed the title ユーザーリストの実装 ユーザー一覧の実装 Nov 14, 2024
@Kosei805 Kosei805 marked this pull request as ready for review November 14, 2024 10:47
Copy link
Member

Choose a reason for hiding this comment

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

こういうリファクタリング大事

handleLimitChange: (newLimit: number) => void;
page?: number;
limit?: number;
totalNum: number;
Copy link
Member

Choose a reason for hiding this comment

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

total だけでもいいかなと思った

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bb

Comment on lines 117 to 118
const handleLimitChange = (newLimit: number) =>
navigate(`/home/users?limit=${newLimit}`);
Copy link
Member

Choose a reason for hiding this comment

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

個人的にこの書き方あんまり好きじゃない

関数の中身が1行だったとしてもブロックで書きたい

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bb

@Kosei805 Kosei805 merged commit 49a5744 into main Nov 15, 2024
3 checks passed
@Kosei805 Kosei805 deleted the 70-users-list-page branch November 15, 2024 12:32
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.

ページネーションにおける総ページ数のバグを治す ユーザ一覧ページの実装
2 participants