ページング機能の実装 #181

trm11tkr · 2024-04-11T02:36:14Z

概要

close: #2

レビュー観点

ページングの基盤が適切な仕様になっているか
feature/github_repository パッケージの実装が適切か

レビューレベル

Lv1: ぱっとみて違和感がないかチェックして Approve する
Lv2: 仕様レベルまで理解して、仕様通りに動くかある程度検証して Approve する
Lv3: 実際に環境で動作確認したうえで Approve する

レビュー優先度

すぐに見てもらいたい ( hotfix など ) 🚀
今日中に見てもらいたい 🚗
今日〜明日中で見てもらいたい 🚶
数日以内で見てもらいたい 🐢

画像 / 動画

ページング

Android	iOS
paging_android.webm	paging_ios.webm

初回エラー・ページングに伴うエラー

Android	iOS
paging_error_android.webm	paging_error_ios.webm

動作確認手順

エラーを確認する手順

初回エラーからの復帰の確認

機内モードにしてアプリ起動
機内モードを解除してから再読み込みボタンを押下

ページング処理に伴うエラーの確認

通常通りにアプリ起動
機内モードに設定してから、スクロールし追加読み込みを行う

備考

今回の PR ではページネーションによるエラーからの復帰対応は行なっていません。
#173 にて対応される想定です。

参考:

https://zenn.dev/k9i/articles/b8c333e1bb8b9b （今回の Paging の実装はこちらの記事を参考にしています）
https://docs.github.com/ja/rest/repos/repos?apiVersion=2022-11-28#list-organization-repositories

github-actions · 2024-04-11T02:38:45Z

Visit the preview URL for this PR (updated for commit bf5c4c6):

https://flutter-mobile-project-template-catalog--pr181-feature-2wa8lt7x.web.app

_{(expires Fri, 26 Apr 2024 04:30:28 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 9ea56735a63d07a7cfe62eb204b0528284c37c23}

…ial error.

packages/cores/core/lib/src/ui/util/common_paging_view.dart

packages/features/github_repository/lib/src/data/provider/repository.dart

github-actions · 2024-04-11T09:24:25Z

Ready for review 🚀

trm11tkr · 2024-04-11T09:25:38Z

@warahiko @takashi0602
レビューお願いいいたします。
複雑に実装になっているため 1時間はレビューに要するのではないかと考えております🙏

warahiko

めちゃ難しい実装ありがとうございます！
全体的に generics が複雑ですね 😇
kotlin みたいに star-projection が使えたりすると見やすくなりそうですが......
（PagingData は PagingDataItem の id の型は気にしていない、とか）
あと改行の仕方も気にしたいですが、linter でできるんでしょうか......

全体的に GitHub の表記揺れが見られます

packages/cores/core/lib/src/pagination/model/paging_data.dart

packages/cores/core/lib/src/pagination/provider/paging_async_notifier.dart

packages/features/github_repository/lib/src/data/provider/repository.dart

packages/features/github_repository/lib/src/ui/github_repository_list.dart

packages/cores/core/lib/src/pagination/model/paging_data.dart

packages/cores/core/lib/src/ui/util/common_paging_view.dart

packages/features/github_repository/lib/src/ui/provider/github_repository_list_notifier.dart

trm11tkr

@warahiko @takashi0602
お二方とも丁寧にレビューしていただきありがとうございました！
修正対応 & コメントさせていただきましたので再度レビューよろしくお願いいたします🙏

packages/features/github_repository/lib/src/ui/provider/github_repository_list_notifier.dart

packages/features/github_repository/lib/src/ui/github_repository_list.dart

packages/features/github_repository/lib/src/ui/provider/github_repository_list_notifier.dart

packages/features/github_repository/lib/src/data/provider/repository.dart

packages/cores/core/lib/src/pagination/model/paging_data.dart

packages/cores/core/lib/src/ui/util/common_paging_view.dart

… in VSCode

warahiko

対応ありがとうございます、返答しました！
ちなみに GitHub の H は large が正式です、コード内で揃ってる分にはまあ......という気はしますが document 類はだいたい GitHub になってますね......

trm11tkr · 2024-04-17T10:33:12Z

@warahiko

ちなみに GitHub の H は large が正式です、コード内で揃ってる分にはまあ......という気はしますが document 類はだいたい GitHub になってますね......

正式名称が GitHub (H が大文字)であることは存じ上げているのですが、ファイル名やクラス名に反映させるときは h を小文字にするのが一般的だと勝手に思っていました。
ファイル名が github_repositoryなので合わせてクラス名も GithubRepository にしていました。

他の方の実装を見てみると、半々くらいなんですかね🤔

改めて考えるとブランドネームに沿うという観点からは gitHubや GitHub の方が良いと思いましたので以下で修正いたしました。
2988860

…thout currentPage

warahiko · 2024-04-18T04:01:48Z

ファイル名が github_repositoryなので合わせてクラス名も GithubRepository にしていました。

これまさにですね、自分の中でブレていて申し訳ないです 🙏
（snake_case だというのも一つあるとは思いますが......なんか分けたくない感じが...... 😇 機械的には分けるべきなのかもですね）

変更はよさそうに思いました！

…ructor

trm11tkr · 2024-04-18T09:07:11Z

@warahiko
フィードバックいただいた点について対応いたしましたので再度レビューお願いいたします！
#181 (comment) : cores/core の extension の import を制限
#181 (comment) : PageBasedPagingAsyncNotifier に初期ページを設定

warahiko

LGTM 👍

warahiko

再度確認、よさそうです 👍

trm11tkr · 2024-04-19T05:57:16Z

丁寧にレビューしていただき本当にありがとうございました！
こちらマージさせていただきます！

github-actions bot assigned trm11tkr Apr 11, 2024

trm11tkr mentioned this pull request Apr 11, 2024

[Improve]: Riverpod を 2.5.1 に更新する #184

Closed

1 task

trm11tkr added 14 commits April 11, 2024 17:28

feat: Add visibility_detector package for list end detection

0670024

feat: Implement base pagination logic with provider for data fetching

d15dbf8

feat: Implement CommonPagingView for generic pagination support.

3034ad2

feat(github_repository): Add core package for pagination.

8a3bad4

feat: Adapt GitHubRepository for pagination as PagingDataItem<int>

33c030b

feat: Update GitHub API endpoint and integrate pagination support

19051e0

feat: Add new Notifier for pagination and integrate with existing UI

ea05760

fix: Remove an unnecessary logger.

28f2c31

fix(github_repository): invalidate provider when recovering from init…

e93a08c

…ial error.

fix(github_repository): Catch AppException with safeRequest

2cce173

feat(github_repository): Show SnackBar if paging fails.

b2c4b4a

fix: Change to private Widget

62b5a74

refactor: Extract Widgets for readability.

083cb02

fix: Remove an unnecessary parameter.

72785d9

trm11tkr commented Apr 11, 2024

View reviewed changes

packages/cores/core/lib/src/ui/util/common_paging_view.dart Show resolved Hide resolved

packages/cores/core/lib/src/ui/util/common_paging_view.dart Outdated Show resolved Hide resolved

packages/features/github_repository/lib/src/data/provider/repository.dart Outdated Show resolved Hide resolved

trm11tkr force-pushed the feature/paging branch from 9ebc87e to 72785d9 Compare April 11, 2024 09:16

trm11tkr marked this pull request as ready for review April 11, 2024 09:23

yumemi-team-review-requester bot requested review from a team, warahiko and takashi0602 and removed request for a team April 11, 2024 09:24

trm11tkr changed the title ~~[WIP] ページング機能の追加~~ ページング機能の実装 Apr 11, 2024

Merge remote-tracking branch 'origin/main' into feature/paging

e057e0c

warahiko reviewed Apr 15, 2024

View reviewed changes

refactor: Extract AsyncValue extension

c99d2de

trm11tkr added 2 commits April 16, 2024 16:30

feat: Allow specifying 'org' parameter externally.

67db104

refactor: Update docs regarding CommonPagingView

b533406

trm11tkr commented Apr 16, 2024

View reviewed changes

trm11tkr requested review from warahiko and takashi0602 April 16, 2024 08:49

trm11tkr added 5 commits April 16, 2024 18:52

Rename to github for consistency.

53ce445

Merge remote-tracking branch 'origin/main' into feature/paging

6c8ec63

refactor: Remove unused property of GithubRepository

5097736

refactor: Override build function in PageBasedPagingAsyncNotifier.

b1bcd76

refactor: Move docs to above CommonPagingView constructor for display…

039edb3

… in VSCode

warahiko reviewed Apr 17, 2024

View reviewed changes

trm11tkr added 5 commits April 17, 2024 17:51

refactor: Rename method in AsyncValueExtension

0b7656a

refactor: modify a path of logger

3801781

refactor: Narrow down imported file

a956f05

refactor: Change to named parameters

6baebcb

refactor: Rename to GitHub for consistency

2988860

refactor: Change to fetching method to return PageBasedFetchResult wi…

a905690

…thout currentPage

trm11tkr requested a review from warahiko April 17, 2024 11:22

trm11tkr added 2 commits April 18, 2024 17:05

refactor: Organize the directory to enforce import restrictions.

685b9ad

feat: Add initialPage parameter to PageBasedPagingAsyncNotifier const…

0960717

…ructor

warahiko approved these changes Apr 18, 2024

View reviewed changes

refactor: Encapsulate the implementation of extension in core package.

bf5c4c6

trm11tkr requested a review from warahiko April 19, 2024 04:31

warahiko approved these changes Apr 19, 2024

View reviewed changes

trm11tkr merged commit 5b3b0a9 into main Apr 19, 2024
6 checks passed

trm11tkr deleted the feature/paging branch April 19, 2024 05:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ページング機能の実装 #181

ページング機能の実装 #181

trm11tkr commented Apr 11, 2024 •

edited

Loading

github-actions bot commented Apr 11, 2024 •

edited

Loading

github-actions bot commented Apr 11, 2024

trm11tkr commented Apr 11, 2024

warahiko left a comment

trm11tkr left a comment

warahiko left a comment

trm11tkr commented Apr 17, 2024

warahiko commented Apr 18, 2024 •

edited

Loading

trm11tkr commented Apr 18, 2024

warahiko left a comment

warahiko left a comment

trm11tkr commented Apr 19, 2024

ページング機能の実装 #181

ページング機能の実装 #181

Conversation

trm11tkr commented Apr 11, 2024 • edited Loading

概要

レビュー観点

レビューレベル

レビュー優先度

画像 / 動画

ページング

初回エラー・ページングに伴うエラー

動作確認手順

エラーを確認する手順

初回エラーからの復帰の確認

ページング処理に伴うエラーの確認

備考

github-actions bot commented Apr 11, 2024 • edited Loading

github-actions bot commented Apr 11, 2024

trm11tkr commented Apr 11, 2024

warahiko left a comment

Choose a reason for hiding this comment

trm11tkr left a comment

Choose a reason for hiding this comment

warahiko left a comment

Choose a reason for hiding this comment

trm11tkr commented Apr 17, 2024

warahiko commented Apr 18, 2024 • edited Loading

trm11tkr commented Apr 18, 2024

warahiko left a comment

Choose a reason for hiding this comment

warahiko left a comment

Choose a reason for hiding this comment

trm11tkr commented Apr 19, 2024

trm11tkr commented Apr 11, 2024 •

edited

Loading

github-actions bot commented Apr 11, 2024 •

edited

Loading

warahiko commented Apr 18, 2024 •

edited

Loading