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

Implement selected domain UI #18295

Merged
merged 9 commits into from
Apr 21, 2023
Merged

Conversation

ovitrif
Copy link
Contributor

@ovitrif ovitrif commented Apr 20, 2023

Resolves #17901

This PR:

  • ★ Implements the logic to highlight the selected domain on the UI
  • ⊕ Updates the global primary color in dark mode to conform with the design system
  • ⊕ Updates the leading and bottom paddings for domain title to match the design
🌑 🌞
light dark

To Test

Prerequisite

◐ Toggle the SiteCreationDomainPurchasingFeatureConfig flag
  1. Go to MeApp SettingsDebug settings
  2. Scroll to the Features in development section
  3. Tap on the item corresponding to the flag
  4. Tap RESTART THE APP button

● Treatment Variation · flag ON

  • Verify UI for selected domain matches the design specs
    1. See prerequisite
    2. Go to My Site
    3. Tap the arrow at the right of the site name
    4. Tap the + at the top right
    5. Tap the ⊕ Create WordPress.com site button
    6. Skip steps 1 and 2
    7. Enter a text to get domain suggestions
    8. ✅ Tap a few items and compare app's UI vs UI design

Regression Notes

  1. Potential unintended areas of impact
    N/a - new functionality behind feature flag

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

  3. What automated tests I added (or what prevented me from doing so)
    Unit test in SiteCreationDomainsViewModelTest

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.

n/a UI Changes testing checklist

@ovitrif ovitrif requested a review from a team April 20, 2023 18:08
@ovitrif ovitrif linked an issue Apr 20, 2023 that may be closed by this pull request
1 task
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 20, 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
Versionpr18295-51f154c
Commit51f154c
Direct Downloadwordpress-prototype-build-pr18295-51f154c.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Apr 20, 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
Versionpr18295-51f154c
Commit51f154c
Direct Downloadjetpack-prototype-build-pr18295-51f154c.apk
Note: Google Login is not supported on these builds.

@ovitrif ovitrif added this to the 22.3 milestone Apr 20, 2023
@ovitrif ovitrif mentioned this pull request Apr 20, 2023
1 task
@ovitrif ovitrif changed the title Update selected domain UI Implement selected domain UI Apr 20, 2023
Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good, and the code changes look great. Nice work Ovi!

A minor observation I had (though seemingly unrelated with this PR): When I have selected a domain, and then tap back to go back to the theme selection screen, pick a theme again, and return back to the domain search screen, the search input is empty, but the previous results (including the selected domain) are still present. Also, the "Purchase domain" button is still visible. Maybe this is expected, but it felt weird since the search input was still empty. Is this expected behavior?

@ovitrif
Copy link
Contributor Author

ovitrif commented Apr 21, 2023

When I have selected a domain, and then tap back to go back to the theme selection screen, pick a theme again, and return back to the domain search screen, the search input is empty, but the previous results (including the selected domain) are still present. Also, the "Purchase domain" button is still visible. Maybe this is expected, but it felt weird since the search input was still empty.

Thank you for finding this and reporting it @mkevins 🙇🏻 !

Is this expected behavior?

It doesn't feel "expected" to me, but upon checking in production I found it repro's:

🎬 Prod repro scenario

rec.mp4

Given the fact that it's in production for a long time, my suggestion going forward is to ignore it for now (cc. @antonis) 🙏🏻

@ovitrif ovitrif merged commit 2610f50 into trunk Apr 21, 2023
@ovitrif ovitrif deleted the issue/17901-update-selected-domain-ui branch April 21, 2023 09:49
@antonis
Copy link
Contributor

antonis commented Apr 21, 2023

my suggestion going forward is to ignore it for now

I agree that this should not be blocking 👍 Let's open an issue to track this down thought.

@mkevins
Copy link
Contributor

mkevins commented Apr 23, 2023

I agree that this should not be blocking 👍 Let's open an issue to track this down thought.

I've opened an issue to track this: #18311

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.

Update selected domain UI
4 participants