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

[Mobile] add bookmarked boolean to game json #15773

Closed
veloce opened this issue Jul 24, 2024 · 8 comments
Closed

[Mobile] add bookmarked boolean to game json #15773

veloce opened this issue Jul 24, 2024 · 8 comments

Comments

@veloce
Copy link
Contributor

veloce commented Jul 24, 2024

Needed for lichess-org/mobile#594

Mobile PR: lichess-org/mobile#868

I see 3 places where we need this:

In the json returned by:

  • GET /api/games/user/<username>
  • GET /game/export/<gameId>

In mobile round socket:
https://github.com/lichess-org/lila/blob/master/modules/round/src/main/RoundMobile.scala

@Siderite
Copy link

Wouldn't that mean that the API would return different results based on the logged user?

@ornicar
Copy link
Collaborator

ornicar commented Jul 26, 2024

Wouldn't that mean that the API would return different results based on the logged user?

Yes, nothing wrong with that.

@ornicar
Copy link
Collaborator

ornicar commented Jul 26, 2024

mobile round socket JSON: 77ba6b0

@ornicar
Copy link
Collaborator

ornicar commented Jul 26, 2024

are we sure we want to display the bookmark status in game lists?

@veloce
Copy link
Contributor Author

veloce commented Jul 26, 2024

are we sure we want to display the bookmark status in game lists?

Not directly in the list, but I was thinking at the context menu (long press on an item), or a swipe to discover actions as we often find in mobile apps. (Un)Bookmark by swiping the game would make sense imo.

@ornicar
Copy link
Collaborator

ornicar commented Jul 26, 2024

Ah, then it's wasteful for the server to check for bookmarks for all games, all the time, when we only need to know after a user action?
Instead you could fetch /game/export when required?

@veloce
Copy link
Contributor Author

veloce commented Jul 26, 2024

I think we can do that yes. Especially for the context menu. Ideally for the swipe action we should know in advance but we're not obliged to implement swipe.

@ornicar
Copy link
Collaborator

ornicar commented Jul 26, 2024

then make sure to set the withBookmarked query parameter only when actually needed.

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

No branches or pull requests

3 participants