Skip to content
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

Merged
merged 9 commits into from
Apr 13, 2023
Merged

Dashboard domain card UI #18240

merged 9 commits into from
Apr 13, 2023

Conversation

ravishanker
Copy link
Contributor

Fixes #18178

To test:

  1. Launch Jetpack/WordPress app
  2. Go to App Settings (Tap on Avatar at the top right hand corner on My Site -> Find App Settings)
  3. Tap Debug Settings
  4. Find dashboard_card_domain under Remote features as shown in the image above
  5. Verify domain card appears as shown above on dashboard
  6. Tap on overflow (three dots) menu
  7. Verify it pops up menu with Hide this option
  8. Tap on Hide this
  9. Verify, the card gets hidden

** Note **
The 🌐 domain icon needs to be updated, waiting for export. Will update before merging or in the next PR

Regression Notes

  1. Potential unintended areas of impact
    None

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Unit tests and manual tests

  3. What automated tests I added (or what prevented me from doing so)
    Updated and added unit tests

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Adds data class DashboardDomainCard
Adds
- DashboardDomainCardBuilder
- ViewHolder
- layout
- string resources
- menu
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 6, 2023

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr18240-461a103
Commit461a103
Direct Downloadwordpress-prototype-build-pr18240-461a103.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 6, 2023

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr18240-461a103
Commit461a103
Direct Downloadjetpack-prototype-build-pr18240-461a103.apk
Note: Google Login is not supported on these builds.

@staskus
Copy link
Contributor

staskus commented Apr 11, 2023

@ravishanker, thanks!

✅ The UI looks good!

The presentation logic seems to be mostly working. The logic for checking existing domains is still needed:

// !hasSiteDomains(siteModel) && // this may need a separate api call!

@ravishanker
Copy link
Contributor Author

The presentation logic seems to be mostly working. The logic for checking existing domains is still needed:

// !hasSiteDomains(siteModel) && // this may need a separate api call!

Thank you, coming in the next PR

Copy link
Member

@irfano irfano left a 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.

WordPress/src/main/res/layout/dashboard_card_domain.xml Outdated Show resolved Hide resolved
WordPress/src/main/res/layout/dashboard_card_domain.xml Outdated Show resolved Hide resolved
WordPress/src/main/res/layout/dashboard_card_domain.xml Outdated Show resolved Hide resolved
WordPress/src/main/res/layout/dashboard_card_domain.xml Outdated Show resolved Hide resolved
WordPress/src/main/res/layout/dashboard_card_domain.xml Outdated Show resolved Hide resolved
Updated layout and removed redundant properties
@ravishanker
Copy link
Contributor Author

ravishanker commented Apr 13, 2023

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.

I've updated the layout with the suggestions. It might all change, if the design changes!

Copy link
Member

@irfano irfano left a 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.

@irfano
Copy link
Member

irfano commented Apr 13, 2023

@ravishanker, We can also add release notes in this PR.

* [**] [Jetpack-only] Added a dashboard card for purchasing domains. [https://github.com/wordpress-mobile/WordPress-Android/pull/18240]

@peril-wordpress-mobile
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@ravishanker ravishanker merged commit 28e935d into trunk Apr 13, 2023
@ravishanker ravishanker deleted the Dashboard-Domain-Card-UI branch April 13, 2023 22:06
@ravishanker
Copy link
Contributor Author

ravishanker commented Apr 13, 2023

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.

blaze_styles.xml may need to be updated for the next phase of the project, so better leave it for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Domains Dashboard Card: Card UI
4 participants