-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Unified Dashboard: Refactoring Part 1 #19244
Unified Dashboard: Refactoring Part 1 #19244
Conversation
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
…in the unified my site menu
…d MySite words and move to a separate package under MySite
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.
Hey @zwarm
Way to go. This is a great start. I think we can build on the changes in this PR
The UI looks good to me overall. I did a smoke test of the app
- Accessed all the menu items 🟢
- Shortcuts of the of the app - Stats especially 🟢
I have ignored the issues with quick start in this PR as we can take up those in a seperate PR
) | ||
Spacer(modifier = Modifier.height(4.dp)) | ||
|
||
// todo: eventually we can take uiStringRes out of the state, but for now it's shared, so leave it |
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.
Hey @zwarm
💡 AFAIK only plan item is using UiStringText and @ravishanker confirmed that we can safely remove plans feature from the app since they are planning a revamp for plans in site creation.
We can take up this change in a seperate PR but just letting you know.
} | ||
|
||
@Composable | ||
fun UnifiedMenuContent(uiState: MenuViewState) { |
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.
e254632
into
UI-Modernization-Remove-tabs-and-update-quick-links-layout
Part of #19270
This PR starts refactoring for unified dashboard
The Menu logic was extracted out into standalone classes with no reliance on source manager or mysiteviewmodel. The more tap will launch the new set of activities. The old classes were not removed and can be launched by removing the comment in MySiteFragment line 649.
MenuActivity
andMenuViewModel
into themenu
package under MySiteNotes
To test:
Regression Notes
Potential unintended areas of impact
The more menu is not shown or working properly
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual testing
What automated tests I added (or what prevented me from doing so)
N/A
PR submission checklist:
RELEASE-NOTES.txt
if necessary.UI Changes testing checklist: