Skip to content

Conversation

@joshheald
Copy link
Contributor

@joshheald joshheald commented Nov 21, 2024

Closes: #14458

Description

This PR removes the POS Dashboard view model.

To do this, I made the connectivity banner view self-contained, relying on its connectivity observer directly. Additionally, I moved some flags used to control the display of the support modal and the exit POS confirmation modal, to @State variables on the dashboard view, with bindings where required by the floating controls view.

Testing information

Again, with this PR most of the functionality had previously been stripped out of the dashboard view model. The main areas to test are whether the support and exit POS modals still display and dismiss correctly, and to check that the no internet connection banner animates in and out as it should.

I've tested the full flow on an iPad Air running iOS 17.7.

Screenshots

remove-dashboard-view-model.mp4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

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:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

@joshheald joshheald added type: task An internally driven task. feature: POS labels Nov 21, 2024
@joshheald joshheald added this to the 21.2 milestone Nov 21, 2024
@joshheald joshheald changed the base branch from trunk to issue/14458-remove-cart-view-model November 21, 2024 14:28
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 21, 2024

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 Numberpr14483-2e5b222
Version21.1
Bundle IDcom.automattic.alpha.woocommerce
Commit2e5b222
App Center BuildWooCommerce - Prototype Builds #11729
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@joshheald joshheald marked this pull request as ready for review November 21, 2024 14:49
@wpmobilebot wpmobilebot modified the milestones: 21.2, 21.3 Nov 22, 2024
@wpmobilebot
Copy link
Collaborator

Version 21.2 has now entered code-freeze, so the milestone of this PR has been updated to 21.3.

@joshheald joshheald requested review from iamgabrielma, jaclync and staskus and removed request for iamgabrielma November 22, 2024 08:19
Base automatically changed from issue/14458-remove-cart-view-model to trunk November 22, 2024 08:20
@staskus staskus self-assigned this Nov 22, 2024
Copy link
Contributor

@staskus staskus left a comment

Choose a reason for hiding this comment

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

✅ Tested on iPad Air M2 18.1

  • Support modal
  • Connectivity notice
  • Smoke test payment and order flow

init(viewModel: PointOfSaleDashboardViewModel) {
self.viewModel = viewModel
}
@State private var showExitPOSModal: Bool = false
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Given this state var is only used to reflect user selection within the dashboard to open the modal, I think using '@State + @Binding` is a good move. Rather than using a view model as an intermediary.

@joshheald joshheald enabled auto-merge November 22, 2024 13:36
@joshheald joshheald merged commit f8c084c into trunk Nov 22, 2024
14 checks passed
@joshheald joshheald deleted the issue/14458-remove-dashboard-view-model branch November 22, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: POS type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Woo POS] Remove view models

4 participants