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

PPO: View model GraphQL loading and pagination support #2139

Merged
merged 11 commits into from
Sep 4, 2024

Conversation

stevestreza-ksr
Copy link
Contributor

📲 What

This PR adds support for loading, reloading, and fetching more data for the PPO view. (Integration forthcoming)

🤔 Why

As part of PPO, we need the view to support loading paginated data from GraphQL and converting that into view models for individual cards.

🛠 How

This PR does two things:

  1. The Paginator class was updated to use an enum with associated values, rather than separate optional values. This prevents invalid states from being representable by the compiler.
  2. The PPOViewModel was updated to connect a Paginator to the GraphQL endpoint, with methods to trigger first load, pull to refresh, and loading the next page.

✅ Acceptance criteria

  • Unit tests should pass

@nativeksr
Copy link
Collaborator

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

Copy link
Contributor

@ifosli ifosli left a comment

Choose a reason for hiding this comment

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

Looks pretty good - I left a few minor comments and I'm a little confused by some of the code that got deleted from the PPOView. The "show banner" button was intended to be temporary but the ability to show a banner was not. If you have a different plan for how to show a banner, I'm okay with deleting that code, but then I think we should just delete the geometry reader entirely. Marking this as approved but please fix the banner stuff before submitting!

@stevestreza-ksr stevestreza-ksr merged commit 36e8d61 into main Sep 4, 2024
5 checks passed
@stevestreza-ksr stevestreza-ksr deleted the stevestreza/ppo/view-model-loading branch September 4, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants