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

[Woo POS] Avoid triggering dashboard view redraws whenever any POS aggregate model property changes #14869

Closed
jaclync opened this issue Jan 15, 2025 · 3 comments · Fixed by #15079
Assignees
Labels
feature: POS priority: medium Planned tasks usually belonging to ongoing projects type: enhancement A request for an enhancement.

Comments

@jaclync
Copy link
Contributor

jaclync commented Jan 15, 2025

For issues #14853 and #14860, whenever PointOfSaleAggregateModel.itemsViewState is updated like for the loading state while waiting for the API request, the refresh control async task refreshable gets canceled because the top-level view PointOfSaleDashboardView is redrawn (and thus the ItemListView).

There are workarounds to preserve the async task for refreshable, but with tradeoffs. Ideally, we can avoid triggering redraws by providing more granular updates instead of the whole ObservableObject PointOfSaleAggregateModel being changed. The best candidate so far is iOS 17+ Observation framework. Some refactoring is likely required.

@jaclync jaclync added priority: medium Planned tasks usually belonging to ongoing projects type: technical debt Represents or solves tech debt of the project. labels Jan 15, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Jan 15, 2025

Thanks for reporting! 👍

@malinajirka
Copy link
Contributor

@jaclync I'm trying to understand the priority of this issue. What is the impact of this behavior?

@malinajirka malinajirka added feature: POS type: enhancement A request for an enhancement. and removed type: technical debt Represents or solves tech debt of the project. labels Jan 23, 2025
@jaclync
Copy link
Contributor Author

jaclync commented Feb 3, 2025

The impact would be to resolve a tech debt where we put a workaround in #14870 (diffs in the refreshable) so that the pull-to-refresh async task isn't canceled from unintended redraws due to other state changes in the POS aggregate model.

One potential solution is to use the iOS 17+ Observation framework so that other state changes only trigger redraws for the affected views, not the refresh control. From pdfdoF-6lP-p2#comment-7478, @joshheald is working on it this week. The granular UI updates from state changes could also bring other benefits such as performance since it no longer triggers redraws for the whole view. The main risks are the increase of minimum iOS version, and a potentially significant amount of changes to the POS code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: POS priority: medium Planned tasks usually belonging to ongoing projects type: enhancement A request for an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants