-
Notifications
You must be signed in to change notification settings - Fork 134
[WOOMOB-1023][Woo POS][Settings] Implement 2-pane layout with Android-style UI design #14459
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
[WOOMOB-1023][Woo POS][Settings] Implement 2-pane layout with Android-style UI design #14459
Conversation
…new ViewModel for settings navigation state management
Move root destination directly into WooPosSettingsCategory enum to eliminate potential mismatches between categories and their destinations. Remove companion object function that created loose coupling. 🤖 Generated with [Claude Code](https://claude.ai/code)
…ubtitles, and proper alignment
Generated by 🚫 Danger |
📲 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 Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## woomob-1022-woo-possettings-prepare-infrastructure-for-the-settings #14459 +/- ##
=========================================================================================================
- Coverage 37.98% 37.95% -0.03%
- Complexity 9209 9210 +1
=========================================================================================================
Files 1994 1999 +5
Lines 112571 112663 +92
Branches 14852 14860 +8
=========================================================================================================
+ Hits 42761 42764 +3
- Misses 65902 65992 +90
+ Partials 3908 3907 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a 2-pane settings layout for Woo POS with Android-style UI design, featuring categories on the left and details on the right. The implementation follows a hierarchical navigation structure with visual feedback and animations.
- Restructured settings architecture to support 2-pane layout with category navigation
- Added Hardware and Store categories with subcategory navigation support
- Implemented smooth animations, ripple effects, and visual feedback for user interactions
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
strings.xml | Added string resources for new settings categories and subcategories |
WooPosHardwareSettingsViewModel.kt | Created ViewModel for hardware settings with state management |
WooPosHardwareSettingsState.kt | Defined data models for hardware settings items with icons and labels |
WooPosHardwareSettingsScreen.kt | Implemented hardware settings UI with clickable menu items and ripple effects |
WooPosSettingsDetailPaneScreen.kt | Created detail pane with animated transitions and toolbar navigation |
WooPosSettingsCategoriesViewModel.kt | Created ViewModel for category selection management |
WooPosSettingsCategoriesState.kt | Defined category data models and enum structure |
WooPosSettingsCategoriesPaneScreen.kt | Implemented category selection UI with visual selection feedback |
WooPosSettingsViewModel.kt | Enhanced main ViewModel with navigation state management |
WooPosSettingsState.kt | Defined navigation state and destination hierarchy |
WooPosSettingsScreen.kt | Restructured main screen to support 2-pane layout with category and detail views |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
data class WooPosSettingsState( | ||
val selectedCategory: WooPosSettingsCategory = WooPosSettingsCategory.HARDWARE, | ||
val currentDestination: WooPosSettingsDetailDestination = selectedCategory.rootDestination |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value for currentDestination references selectedCategory.rootDestination, but selectedCategory is also being initialized in the same constructor. This creates a circular dependency where currentDestination depends on selectedCategory's rootDestination, but selectedCategory is initialized with a default value at the same time. This should be explicitly set to WooPosSettingsCategory.HARDWARE.rootDestination to avoid potential initialization issues.
val currentDestination: WooPosSettingsDetailDestination = selectedCategory.rootDestination | |
val currentDestination: WooPosSettingsDetailDestination = WooPosSettingsCategory.HARDWARE.rootDestination |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I added a few non-blocking comments.
interactionSource = remember { MutableInteractionSource() }, | ||
indication = ripple() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interactionSource = remember { MutableInteractionSource() },
indication = ripple()
💡 n.p.: aren't these params redundant in clickable? I can't see any difference with or without them on my device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Addressed here - 8bd41fb
} | ||
|
||
@Composable | ||
private fun DetailPaneToolbar( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 What do you think about adapting and reusing WooPosToolbar
here instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree! Implemented here
WOOMOB-1023
Description
Implements a 2-pane settings layout for Woo POS following Android settings design. Categories are shown on the left, details on the right. Added visual feedback, animations, and Store category.
Focus more on the architecture then the implementation as the UI most likely is going to be changed
Steps to reproduce
Testing information
The tests that have been performed
08-12--14-29.mp4
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.