[Woo POS][Products search] It's possible to pull to refresh when search is visible. Search is hidden when products arrive #13878
Conversation
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 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 #13878 +/- ##
=========================================
Coverage 38.19% 38.19%
- Complexity 9316 9322 +6
=========================================
Files 2085 2085
Lines 115154 115177 +23
Branches 14675 14680 +5
=========================================
+ Hits 43980 43989 +9
- Misses 67174 67186 +12
- Partials 4000 4002 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
📝 WalkthroughWalkthroughThe changes refactor state management across the WooCommerce POS UI. The generic Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as WooPosItemsScreen
participant List as MainItemsList
participant VM as WooPosItemsViewModel
User->>UI: Initiates pull-to-refresh gesture
UI->>List: Trigger onPullToRefreshTriggered callback
List->>VM: Execute refresh logic
VM->>VM: Update pullToRefreshState to Refreshing<br>and load products with WooPosPaginationState
VM-->>List: Emit updated state
List-->>UI: Render refreshed content based on new state
UI-->>User: Display updated product list
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchViewModelTestSelectionViewState.kt (1)
339-347: Consider adding a test forWooPosPaginationState.CanLoadMore.While the tests cover loading, none, and error states, there doesn't appear to be coverage for the
.CanLoadMorestate that likely exists in the enum. Consider adding a test case that verifies this state transition as well.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/composeui/component/WooPosPaginationErrorIndicator.kt(2 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosBaseViewState.kt(1 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt(4 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsScreen.kt(9 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsSearchHelper.kt(2 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewModel.kt(4 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewState.kt(2 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosVariationsViewState.kt(1 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchScreen.kt(2 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchViewModel.kt(4 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchViewState.kt(2 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsScreen.kt(4 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt(5 hunks)WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsSearchHelperTest.kt(1 hunks)WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewModelTestSelectionViewState.kt(3 hunks)WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchViewModelTestSelectionViewState.kt(3 hunks)WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt(5 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt (1)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsUIEvent.kt (2)
item(3-17)item(4-4)
🔇 Additional comments (49)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsScreen.kt (5)
49-49: Appropriate import added for the new state enumThe addition of the
WooPosPullToRefreshStateimport is correctly implemented to support the new state management approach replacing the boolean flag.
109-110: Good refactoring to use enum instead of boolean flagReplacing the boolean
reloadingWithPullToRefreshwith the explicit enum stateWooPosPullToRefreshState.Refreshingimproves code readability and makes the state management more robust.
116-119: Enhanced pull-to-refresh with proper state controlThe modification of the
pullRefreshmodifier to include anenabledparameter is a good improvement. This allows explicit control over when pull-to-refresh should be available based on the current state, addressing the issue mentioned in the PR where pull-to-refresh should be available during search.
180-180: Consistent state check for pull refresh indicatorThe PullRefreshIndicator is now properly using the same state enum check as the rest of the component, ensuring consistent behavior throughout the pull-to-refresh implementation.
285-285: Preview state updated to match new implementationThe preview's state initialization has been correctly updated to use the new
WooPosPullToRefreshState.Refreshingenum value, maintaining consistency with the actual implementation.WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/variations/WooPosVariationsViewModelTest.kt (1)
10-10: LGTM! Successful class rename refactoring.The import and assertion changes correctly replace the generic
PaginationStateclass with the more specificWooPosPaginationStateclass. This aligns with the PR objective of renaming incorrectly labeled high-level classes and makes the pagination state representation consistent across the codebase.Also applies to: 216-216, 284-284, 310-310, 333-333
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchViewModelTestSelectionViewState.kt (2)
13-13: Good namespacing approach.The import switch from a generic
PaginationStateto a more specificWooPosPaginationStatefollows good namespacing practices. This change is aligned with the PR objective of renaming high-level classes for better clarity.
343-343: Test assertions properly updated to use new pagination state class.The test assertions have been correctly updated to use
WooPosPaginationStateinstead of the previous genericPaginationState. This ensures the tests continue to function as expected with the new class names.Also applies to: 347-347, 368-368, 371-371
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/variations/WooPosVariationsViewModel.kt (5)
14-15: Well-structured imports for improved state managementThe introduction of domain-specific state classes
WooPosPaginationStateandWooPosPullToRefreshStatereplaces generic state management with more specific, namespaced alternatives. This aligns with the PR objective to rename incorrectly labeled high-level classes.
104-107: Consistent pagination state namingThe replacement of the generic
PaginationStatewith the domain-specificWooPosPaginationStateimproves code clarity and maintainability by using consistent naming conventions throughout the codebase.
142-154: Enhanced state management for pull-to-refreshThe refactoring from a boolean flag (
reloadingWithPullToRefresh) to an explicit state enum (WooPosPullToRefreshState.Refreshing) improves code readability and flexibility. This approach provides a more structured way to manage pull-to-refresh states across different view states and addresses the PR objective of ensuring pull-to-refresh functionality is available during search.
166-166: Consistent pagination state update in loadMoreThe pagination state update properly uses the namespaced
WooPosPaginationState.Loading, maintaining consistency with the other state changes in this file.
184-184: Consistent error state handlingThe error case now correctly references
WooPosPaginationState.Error, completing the consistent application of the new state classes throughout the pagination logic.WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewModelTestSelectionViewState.kt (3)
197-197: Good implementation of naming change.The assertion has been updated to use the new namespaced
WooPosPaginationState.Nonein place of the previous genericPaginationState.None. This aligns with the PR objective of renaming incorrectly labeled high-level classes to more specific ones.
235-235: Good implementation of naming change.The assertion has been updated to use the new namespaced
WooPosPaginationState.None, consistent with the changes made throughout the codebase.
272-273: Good implementation of namespaced classes.Both changes here are aligned with the PR objectives:
- Changed the cast to use
WooPosContentViewStateinstead of the previous genericContentViewState- Updated the assertion to use
WooPosPaginationState.Errorclass for type checkingThis provides better namespace separation and reduces the risk of name collisions with other components.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchScreen.kt (2)
23-24: Updated imports for better state management.The imports have been correctly updated to use the new namespaced state classes, which aligns with the PR objective of renaming high-level classes.
163-164: State management improvement.The preview code now uses the new enum-based state system instead of boolean flags, making the pull-to-refresh state more explicit and easier to understand.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/composeui/component/WooPosPaginationErrorIndicator.kt (2)
35-36: Updated imports to use the namespaced state classes.The imports have been correctly updated to use the new namespaced state classes, which is consistent with the changes in other files.
151-152: Improved state representation in the preview.The code now uses the more expressive enum-based state system instead of boolean flags, making it clear that the pagination is in an error state and pull-to-refresh is actively refreshing.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosBaseViewState.kt (4)
3-5: Enhanced state management with enums instead of booleans.The base view state now uses the more expressive
WooPosPullToRefreshStateenum instead of a boolean flag, allowing for more clearly defined states.
7-11: Improved interface naming and state properties.The interface has been renamed from
ContentViewStatetoWooPosPullToRefreshStatefor better namespacing, and the state properties have been updated to use the dedicated enum types instead of booleans.
13-17: Created namespaced pagination state class.The new sealed class properly namespaces the pagination states, making the code more maintainable and reducing potential naming conflicts.
19-21: Added pull-to-refresh state enum.This enum provides a clear representation of the three possible states for pull-to-refresh, which is more expressive than the previous boolean flag.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsSearchHelper.kt (2)
99-102: Added helper method to check if search is open.This new method encapsulates the logic for checking if the search is in an open state, which helps fix the issue where search state was being hidden when products arrive.
147-160: Key fix: Conditionally enable/disable pull-to-refresh based on search state.This logic directly addresses the PR objectives by:
- Enabling pull-to-refresh when search is hidden or closed
- Disabling pull-to-refresh when search is open
This ensures proper behavior during search and prevents conflicts between the two interactions.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchViewModel.kt (1)
14-14: Good refactoring to feature-specific state types.Replacing the generic
PaginationStatewith the more specificWooPosPaginationStateimproves code organization and maintainability. This namespace-based approach reduces the risk of state conflicts between different features, making the code more modular and easier to reason about.Also applies to: 147-147, 157-157, 196-196
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt (1)
71-71: Improved UI state management implementation.The changes effectively update the UI components to work with the more specific state types (
WooPosContentViewStateandWooPosPaginationState). Particularly noteworthy is the change from the booleanreloadingWithPullToRefreshto the more expressivepullToRefreshStatein theLaunchedEffect. This enhancement allows for more nuanced state handling and fixes the pull-to-refresh functionality during search, directly addressing one of the PR objectives.Also applies to: 114-126, 411-411, 425-425
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsSearchHelperTest.kt (2)
208-209: Updated test helper to use the new state types.The test helper's
createContentStatemethod has been properly updated to useWooPosPaginationState.Noneand initialize the newpullToRefreshStateproperty withWooPosPullToRefreshState.Enabled, ensuring consistency with the implementation changes.
214-254: Excellent test coverage for the pull-to-refresh behavior.These new tests provide thorough verification of the pull-to-refresh behavior in different search states:
- When search is closed, pull-to-refresh is enabled
- When search query is being changed, pull-to-refresh is disabled (preventing accidental refreshes)
- When search is hidden, pull-to-refresh is enabled
The tests directly verify the core functionality mentioned in the PR objectives around pull-to-refresh availability during search, ensuring the changes work as intended.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewState.kt (3)
7-9: Good refactoring from boolean to enum-based state management.Replacing the boolean
reloadingWithPullToRefreshwith an enum-basedpullToRefreshStateprovides more explicit state management and allows for future extension if additional states are needed. This change directly supports the PR objective of improving pull-to-refresh functionality.
14-17: Well-structured state type refactoring.The Content class has been properly updated to:
- Use the more specific
WooPosPaginationStateinstead of genericPaginationState- Add the new
pullToRefreshStateproperty with a sensible default- Implement the feature-specific
WooPosContentViewStateinterfaceThese changes create a more maintainable and organized state hierarchy that aligns with the PR objectives.
31-42: Consistent state management across all view states.The Loading, Error, and Empty state classes have been consistently updated to use the new
pullToRefreshStateproperty with appropriate defaults. This ensures that pull-to-refresh behavior is properly managed regardless of which state the UI is in.WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsViewModel.kt (5)
213-233: Improved state handling with namespaced pull-to-refresh statesThe refactoring from generic state types to specific WooPOS state types improves type safety and makes the code more self-explanatory. The conditional logic now properly maintains search state when new products arrive, addressing the bug mentioned in the PR objectives.
246-252: Consistent use of WooPosPullToRefreshState enum in reload stateThe implementation now uses the enum-based state management instead of boolean flags, providing more expressive and maintainable code.
270-288: Good extraction of conversion logic into a dedicated functionMoving the product-to-viewstate conversion logic into a separate extension function improves modularity and readability.
300-300: Consistent use of WooPosPaginationState.LoadingUpdated pagination state handling maintains consistency with the overall state management refactoring.
308-308: Consistent error handling with WooPosPaginationState.ErrorError state handling is now aligned with the namespaced pagination state system.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/search/WooPosItemsSearchViewState.kt (2)
3-7: Updated imports to use namespaced state classesThe imports have been correctly updated to use the namespaced WooPos-specific state classes, aligning with the overall refactoring effort.
17-19: Migrated to enum-based state managementThe Content class now properly implements the WooPosContentViewState interface with the new state types. This improves type safety and allows for more nuanced state control beyond simple booleans.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosVariationsViewState.kt (3)
4-5: Updated base class constructor to use WooPosPullToRefreshStateThe constructor parameter is now properly typed as WooPosPullToRefreshState rather than a boolean, improving type safety and expressiveness.
7-11: Consistent state type updates in Content classThe Content class now properly uses the namespaced state types in line with the overall refactoring, improving type safety and clarity.
13-24: Consistent pull-to-refresh state handling across all view statesAll view state classes (Loading, Error, Empty) now consistently use the WooPosPullToRefreshState type, ensuring a uniform approach to state management throughout the application.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsScreen.kt (6)
110-111: Added pull-to-refresh trigger callbackThe onPullToRefreshTriggered callback is now properly passed to the MainItemsList function, enabling the pull-to-refresh functionality during search state.
128-133: Added pull-to-refresh callback parameterThe MainItemsList function now accepts an onPullToRefreshTriggered callback, making the component more modular and reusable.
130-141: Enhanced pull-to-refresh state managementThe implementation now properly creates and configures the pull-to-refresh state using the enum-based approach. The enabled parameter ensures that pull-to-refresh is only available when appropriate, addressing the PR objective.
228-229: Updated refreshing state checkThe PullRefreshIndicator now correctly uses the WooPosPullToRefreshState.Refreshing enum value instead of a boolean flag to determine when to show the refreshing indicator.
385-487: Comprehensive updates to preview functionsAll preview functions have been updated to use the new state types, maintaining the completeness of the refactoring across production and preview code.
541-594: Continued consistent state updates in remaining previewsThe remaining preview implementations maintain consistency with the new state management approach.
samiuelson
deleted the
13771-woo-posproducts-search-it-possible-to-pull-to-refresh-when-search-is-visible
branch
Closes: #13771
Closes: #13870
Description
The PR:
PaginationStateandContentViewStateSteps to reproduce
Testing information
Make sure that there is no regression regarding PTR, including variations
The tests that have been performed
above
Images/gif
04-01--09-43.mp4
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: