[Bulk Update Orders] Persist selection on configuration change and load more #13227
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
Collaborator
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #13227 +/- ##
=========================================
Coverage 40.59% 40.60%
+ Complexity 6372 6371 -1
=========================================
Files 1345 1345
Lines 77334 77325 -9
Branches 10617 10615 -2
=========================================
- Hits 31396 31394 -2
+ Misses 43177 43169 -8
- Partials 2761 2762 +1 ☔ View full report in Codecov by Sentry. |
…, using viewModel as its storage.
…gain, show list panel if on bulk selection mode.
hafizrahman
changed the title
hafizrahman
added
type: task
JorgeMucientes
approved these changes
Jan 2, 2025
Contributor
JorgeMucientes
left a comment
JorgeMucientes
left a comment
There was a problem hiding this comment.
Good job @hafizrahman. The solutions we worked on seem to be working in all cases. I'll pre-approve, while we still discuss with Hicham about possible cleaner options 👍🏼
Comment on lines
+235
to
+250
| // Some edge cases in order selection mode, like tapping the screen with 4 fingers or using TalkBack, | ||
| // cause the order's onClick listener to gain focus over the selection tracker. | ||
| // This quick fix will prevent the app from entering an unexpected status when the app is in selection mode. | ||
| private fun shouldPreventDetailNavigation(orderId: Long): Boolean { | ||
| if (tracker?.selection?.size() != 0) { | ||
| tracker?.let { selectionTracker -> | ||
| if (selectionTracker.isSelected(orderId)) { | ||
| selectionTracker.deselect(orderId) | ||
| } else { | ||
| selectionTracker.select(orderId) | ||
| } | ||
| } | ||
| return true | ||
| } | ||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
Just moving this function out of OrderListFragment to call it exactly when is needed and avoid unwanted side effects.
Contributor
Author
|
Thanks! @JorgeMucientes I'll merge this once the target is |
4 tasks
hafizrahman
added
status: do not merge
hafizrahman
removed
the
status: do not merge
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #13218, #13231
Description
This PR adds various fixes and updates to the code so that:
Steps to reproduce
TC 1: Config change
TC 2: Load more
Testing information
The tests that have been performed
Tested this both on phone and tablet simulator for both TCs.
Images/gif
Videos incoming.
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.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: