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

Incorporate the Paging3 library #242

Closed
gmale opened this issue Aug 20, 2021 · 2 comments
Closed

Incorporate the Paging3 library #242

gmale opened this issue Aug 20, 2021 · 2 comments
Labels
high-severity SDK Transition Issues added by gmale when transitioning out of ECC

Comments

@gmale
Copy link
Contributor

gmale commented Aug 20, 2021

As a wallet developer, I'd like paging to be easy to use in the SDK so that I can display transactions efficiently in the wallet.

Background:

Criteria:

  • leverages the new Paging 3 library
  • paging is easy and intuitive
  • paging is no longer broken
@gmale gmale added high-severity SDK Transition Issues added by gmale when transitioning out of ECC labels Aug 20, 2021
@pacu pacu added this to the Wallet Sprint 2021-34 milestone Aug 26, 2021
@ccjernigan
Copy link
Contributor

I spent some time investigating this issue.

I'm not sure how we categorize severity. The current implementation appears to limit the number of transactions that can be queried to 1000 to avoid returning an unbound number of items. If there are more than 1000 transactions, some will not be displayed. So this is broken but it doesn't crash, is consistently reproducible, and is easy to explain as a "known issue." I suspect we can limp along with this issue for a bit longer. Are we aware of end user or SDK customer impacts that might shift this priority higher?

As it comes to prioritizing this issue, some items to consider:

  • This is not a small task, it'll require a bit of effort to resolve.
  • Implementing this will be a breaking API change for clients of the SDK.
  • Implementing this will mean that we're dependent on the Android paging library, as our SDK would be emitting classes from the Paging library in our public API.

I'd like to consider whether we might be able to have a separate module in the SDK (sort of like -ktx modules in Android X) that adds the Paging extensions. This would avoid putting Paging classes directly in the public API of the core SDK. It shouldn't add much build complexity on our end, as this additional library would live in the same SDK repo and could get published at the same time as the core SDK. The paging libraries have already had API changes in the past (e.g. paging 2 to paging 3), so it would be nice to avoid future dependency hell with clients of our SDK if possible. As a separate library, we could even publish a version for paging 2, paging 3, and some future paging 4 simultaneously.

@geffenz geffenz modified the milestones: Wallet Sprint 2021-36, Wallet Sprint 2021-39 Sep 23, 2021
@r3ld3v r3ld3v removed this from the Wallet Sprint 2021-39 milestone Oct 21, 2021
ccjernigan added a commit to Electric-Coin-Company/zashi-android that referenced this issue Dec 28, 2021
This provides a very basic scaffold of the home screen and navigation to other child screens.  Additional work needs to be done in both the SDK and this app to build all of the functionality.  Specific known gaps
 - Displaying synchronization status to the user, pending full designs on the progress and animation behaviors
 - Improved APIs for listing and filtering transactions.  This is the issue in the SDK Electric-Coin-Company/zcash-android-wallet-sdk#242
 - Implement the UI for displaying transaction history
 - Display fiat currency values
 - Hook up buttons to navigate to other screens (scan #137, profile #145, send #134, request #135)
ccjernigan added a commit to Electric-Coin-Company/zashi-android that referenced this issue Dec 29, 2021
This provides a very basic scaffold of the home screen and navigation to other child screens.  Additional work needs to be done in both the SDK and this app to build all of the functionality.  Specific known gaps
 - Displaying synchronization status to the user, pending full designs on the progress and animation behaviors
 - Improved APIs for listing and filtering transactions.  This is the issue in the SDK Electric-Coin-Company/zcash-android-wallet-sdk#242
 - Implement the UI for displaying transaction history
 - Display fiat currency values
 - Hook up buttons to navigate to other screens (scan #137, profile #145, send #134, request #135)
@ccjernigan
Copy link
Contributor

If we implement paging, it will be done as paging primitives on the public API rather than using the Paging library. We don't want our public API to be tightly coupled with Google.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-severity SDK Transition Issues added by gmale when transitioning out of ECC
Projects
None yet
Development

No branches or pull requests

5 participants