woocommerce / woocommerce-ios

WooCommerce iOS app
https://www.woocommerce.com/mobile
GNU General Public License v2.0
321 stars 113 forks source link

[Woo POS] Improve itemlist state representation and fix glitches #14362

Closed joshheald closed 2 weeks ago

joshheald commented 2 weeks ago

Closes: #14350 Closes: #14251

Description

This PR removes uses of the items array and itemsPublisher from the ItemListViewModel. This is in preparation for architecture changes to move this state to the new Aggregated Model

The only remaining use of the items array is from the dashboard, where it's used for creating the order. We'll remove that in a future commit.

Instead, we rely on the ItemListState, which includes items when they should be shown.

As part of this work I've had to change the various pagination/loading/refresh behaviours to use the new source of truth for items, and along the way I've fixed some glitches

Steps to reproduce

  1. Launch the app and go to POS
  2. Observe that the initial loading state is shown
  3. When the items load, observe that they're shown correctly
  4. Scroll to the bottom of the list; observe that the next page is loaded
  5. Pull to refresh; observe that the list refreshes without glitches

Test with a site that has no simple products; observe that the full-screen empty state is shown.

Test with the products endpoint on a network breakpoint, and abort the request; observe that the error state is shown and can be refreshed correctly.

Testing information

This refactor should be limited to the display of the itemlist, but it's worth also checking a full checkout including order editing after the first checkout tap, because the items array is used for building the order.

I've tested this with the above scenarios on an iPad Air running iOS 17.7

Screenshots

https://github.com/user-attachments/assets/4b2001c8-c686-4648-902b-fbffd35492a3


Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

wpmobilebot commented 2 weeks ago

WooCommerce iOS📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr14362-a7194c9
Version21.1
Bundle IDcom.automattic.alpha.woocommerce
Commita7194c9207bb9e038812f7d806e0dfe9719c66db
App Center BuildWooCommerce - Prototype Builds #11508

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

joshheald commented 2 weeks ago

@iamgabrielma thanks for the review.

I couldn't replicate the case for the ghost cell eventually disappearing when we reach the end of the infinite scroll, it always remains there, but I see it working on your recording. Not really part of this PR, but from what I see the state keeps refreshing when we've reached the end of the list, so it keeps switching between setting isLoadingAfterInitialLoad to false, and then true, which renders the GhostItemCardView indefinitely. I can tackle this separately, just asking in case you've experienced a different behaviour.

Strange, but yeah I definitely see it disappear eventually. There may be some iOS differences? But mine is on iOS 17.7 so not far away from your versions...