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

User full game list #572

Merged
merged 39 commits into from
May 30, 2024
Merged

User full game list #572

merged 39 commits into from
May 30, 2024

Conversation

ZTL-UwU
Copy link
Contributor

@ZTL-UwU ZTL-UwU commented Feb 24, 2024

TODO

Current status

@ZTL-UwU
Copy link
Contributor Author

ZTL-UwU commented Feb 24, 2024

Currently we are downloading all game histories at once, which takes a very long time to load on startup if the user has a large number of games. We will need a pagination api to get a few games everytime.

@veloce
Copy link
Contributor

veloce commented Feb 24, 2024

Hello! Pagination is indeed mandatory, even for a first version.

@ZTL-UwU
Copy link
Contributor Author

ZTL-UwU commented Feb 25, 2024

Pagination is now implemented! 🚀

@ijm8710
Copy link

ijm8710 commented Mar 27, 2024

Hello @ZTL-UwU, accuracy should prob be fetched as well similar to the recent game list, thoughts?

@ZTL-UwU
Copy link
Contributor Author

ZTL-UwU commented Mar 27, 2024

Hello @ZTL-UwU, accuracy should prob be fetched as well similar to the recent game list, thoughts?

The accuracy should be fetched, yes. Problem is that the server API only return a "analysed": true instead of the full game analysis like the one used recent game list. I believe that getting the accuracy would require a change in the lila server.

@veloce
Copy link
Contributor

veloce commented Apr 11, 2024

@ZTL-UwU I think we should merge this PR with its current implementation (so without filters and bookmark action).

The rest can be done later. Could you resolve the conflicts please, then I'll review asap. Thanks!

@ZTL-UwU ZTL-UwU marked this pull request as ready for review April 12, 2024 05:21
lib/src/model/game/game_repository.dart Outdated Show resolved Hide resolved
lib/src/view/user/full_games_screen.dart Outdated Show resolved Hide resolved
lib/src/view/user/full_games_screen.dart Outdated Show resolved Hide resolved
lib/src/view/user/full_games_screen.dart Outdated Show resolved Hide resolved
use default ios navigation bar previousPage
remove wrong comment
@veloce veloce added this to the Public beta milestone Apr 26, 2024
@veloce veloce removed this from the Public beta milestone May 11, 2024
@ZTL-UwU ZTL-UwU requested a review from veloce May 16, 2024 09:58
@veloce
Copy link
Contributor

veloce commented May 27, 2024

Thanks for making the change @ZTL-UwU , I just realised you updated the PR. Will take take of it now.

Copy link
Contributor

@veloce veloce left a comment

Choose a reason for hiding this comment

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

It is starting to look good now!

As you may have noticed, now the recent games section include offline games too. Pagination support is already done in the GameStorage class, so it should be pretty straightforward now to include the offline games in full game list when having no connectivity.
Using same logic as in

final recentGames = user != null

android/gradle.properties Outdated Show resolved Hide resolved
lib/src/model/game/game_repository.dart Outdated Show resolved Hide resolved
lib/src/view/game/game_list_tile.dart Show resolved Hide resolved
@veloce
Copy link
Contributor

veloce commented May 28, 2024

Thanks for your work. I pushed some refactoring, I'll continue to work a bit on it but I think we're close to merging this :)

@veloce
Copy link
Contributor

veloce commented May 28, 2024

By the way, each time I push to a remote repository branch, it messes up the diff. I don't know what I'm doing wrong...
I used this command to push the code to your repo:

git push [email protected]:ZTL-UwU/mobile.git ZTL-UwU-full-game-list:full-game-list

EDIT: oh, I suppose it is because of the merge commit. But GitHub command line instructions doesn't work without it.

@veloce veloce merged commit 231e0ab into lichess-org:main May 30, 2024
1 check passed
@ZTL-UwU ZTL-UwU deleted the full-game-list branch May 30, 2024 09:20
@veloce
Copy link
Contributor

veloce commented May 30, 2024

🎉

@ijm8710
Copy link

ijm8710 commented Jul 26, 2024

@ZTL-UwU how much effort would it be to have a similar game list view+more button for even more game history but on the perf cards themselves?

I know you were considering working on bookmarks but curious since you basically have built this already if applying to perf cards would be a quicker hit

Hoping you and Veloce agree it makes sense to have them there

@ZTL-UwU
Copy link
Contributor Author

ZTL-UwU commented Jul 26, 2024

@ijm8710 I thinks there is a merged PR for this already #830

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants