-
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
Dashboard domain card UI #18240
Dashboard domain card UI #18240
Conversation
Adds data class DashboardDomainCard
Adds - DashboardDomainCardBuilder - ViewHolder - layout - string resources - menu
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
fix detekt line length issue
@ravishanker, thanks! ✅ The UI looks good! The presentation logic seems to be mostly working. The logic for checking existing domains is still needed:
|
Thank you, coming in the next PR |
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.
It works great, passed all of my tests. Good job!
I've added minor comments and feedback. Some of them might be pointless if we want to update the UI of the card in this PR.
Updated layout and removed redundant properties
I've updated the layout with the suggestions. It might all change, if the design changes! |
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.
Thank you for considering my comments, @ravishanker! I'm approving it, although I have an extra minor suggestion. We can remove blaze_styles.xml
and use dashboard_card_styles
in promote_with_blaze_card.xml
. Since they are exactly the same.
@ravishanker, We can also add release notes in this PR.
|
Generated by 🚫 dangerJS |
|
Fixes #18178
To test:
Regression Notes
Potential unintended areas of impact
None
What I did to test those areas of impact (or what existing automated tests I relied on)
Unit tests and manual tests
What automated tests I added (or what prevented me from doing so)
Updated and added unit tests
PR submission checklist:
RELEASE-NOTES.txt
if necessary.