Implement final screen UI #18305

ovitrif · 2023-04-21T15:29:14Z

Resolves #17906

This PR:

★ Updates the final screen UI
⊕ Adds require additional info for paid domains
⊜ Fixes the url that is displayed to match the iOS implementation, showing the custom domain when following a purchase

To Test

Prerequisite

◐ Toggle the SiteCreationDomainPurchasingFeatureConfig flag

Go to Me → App Settings → Debug settings
Scroll to the Features in development section
Tap on the item corresponding to the flag
Tap RESTART THE APP button

● Treatment Variation

Verify All updates from the task are implemented. Expect:
- Title typeface updated
- Landscape view adapted accordingly
- Preview card has rounded corners and proper paddings
- Paid domains have additional info on the receipt and delay until the custom domain is guaranteed to work
- Preview card shows custom domain at the top

○ Control Variation

Verify No regression. The UI tweaks are fine, they don't have to be hidden behind the feature flag.

Previews

Portrait - Light

●	○

Landscape - Light

Portrait - Dark

●	○

Landscape - Dark

Regression Notes

Potential unintended areas of impact
- Site Creation → Large Titles because of small refactoring
- Landscape view - had to decrease title font size
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual testing and unit testing
What automated tests I added (or what prevented me from doing so)
We don't have automated tests for small UI changes and they're inefficient. We could have snapshot testing though but that's a big step not in scope of this task.

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.

UI Changes testing checklist:

Portrait and landscape orientations.
Light and dark modes.
Fonts: Larger, smaller and bold text.
High contrast.
Talkback.
Languages with large words or with letters/accents not frequently used in English.
Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
Large and small screen sizes. (Tablet and smaller phones)
Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

wpmobilebot · 2023-04-21T15:46:56Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18305-c434d59
	Commit	`c434d59`
	Direct Download	`wordpress-prototype-build-pr18305-c434d59.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2023-04-21T15:47:34Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18305-c434d59
	Commit	`c434d59`
	Direct Download	`jetpack-prototype-build-pr18305-c434d59.apk`

Note: Google Login is not supported on these builds.

mkevins · 2023-04-25T06:38:06Z

WordPress/src/main/java/org/wordpress/android/ui/sitecreation/previews/SitePreviewViewModel.kt

@@ -77,7 +83,8 @@ class SitePreviewViewModel @Inject constructor(
        require(siteCreationState.result is Created)
        siteDesign = siteCreationState.siteDesign
        result = siteCreationState.result
-        urlWithoutScheme = result.site.url
+        isFree = requireNotNull(siteCreationState.domain).isFree


I guess based on the require here that this isn't ever expected to be null here?

That's true, the only reason it's nullable is because of how we modelled the SiteCreationState 😄 . Ideally we'd have found a better, immutable way to do it instead of simply storing the value in a nullable field when advancing to the next step of the flow, imho 😄
But in real world scenarios it should be impossible to get to this screen with the domain null. If we can get here, it's better to crash 😄

mkevins

Code looks good, and this implementation works great in my testing. Nice work Ovi! I had one question about the requireNotNulls, but it is non-blocking.

mkevins · 2023-04-25T06:46:12Z

I should also note, I did observe some changes in the feature-disabled state, but assume it's ok based on your note in the description.

The font was a serif font, left-aligned, and also the preview has rounded borders. Looks good to me 👍

ovitrif · 2023-04-25T06:48:07Z

I should also note, I did observe some changes in the feature-disabled state, but assume it's ok based on your note in the description.

The font was a serif font, left-aligned, and also the preview has rounded borders. Looks good to me 👍

I'd say that it's ok, but I'm open to suggestions / feedback 👍🏻 .

Imho this screen was "left behind" in terms of title typography, all other steps in the flow have the new typeface except this. Perhaps we could align the "progress" screen as well in this regard 🤷🏻 , it's also still left with sans-serif titles.

ovitrif · 2023-04-25T08:43:45Z

Merging this as discussed 🙇🏻
Thank you @mkevins for the review and sharing your thoughts 🙏🏻

ovitrif added 2 commits April 21, 2023 17:11

Update: align title typography with the other screens

7964c3e

Update: align horizontal padding with title

1ae8fbb

ovitrif added the [Type] Task label Apr 21, 2023

ovitrif self-assigned this Apr 21, 2023

ovitrif added this to the 22.3 milestone Apr 21, 2023

ovitrif added 5 commits April 21, 2023 21:47

Update: decrease tile font size in landscape

4b70be2

Update: tweak preview card UI

5483c44

Update: remove padding under subtitle

de0cb00

Update: add texts for paid domains

199a4f8

Fix: show the custom url in the preview card for paid domains

062b0ca

ovitrif added Site Creation Domains Jetpack App labels Apr 24, 2023

ovitrif requested a review from mkevins April 24, 2023 03:20

ovitrif marked this pull request as ready for review April 24, 2023 03:51

ovitrif requested a review from antonis April 24, 2023 03:54

ovitrif added 2 commits April 24, 2023 06:01

Remove: unused resource causing lint failure

84a7290

Remove: unused resource causing lint failure… more

c434d59

mkevins reviewed Apr 25, 2023

View reviewed changes

mkevins approved these changes Apr 25, 2023

View reviewed changes

ovitrif merged commit d1f2ae6 into trunk Apr 25, 2023

ovitrif deleted the issue/17906-final-screen-ui branch April 25, 2023 08:43

This was referenced Apr 26, 2023

[Domain Purchase] Update final UI strings wordpress-mobile/WordPress-iOS#20610

Closed

[Domain Purchase] Update final UI strings wordpress-mobile/WordPress-iOS#20614

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement final screen UI #18305

Implement final screen UI #18305

ovitrif commented Apr 21, 2023 •

edited

Loading

wpmobilebot commented Apr 21, 2023 •

edited

Loading

wpmobilebot commented Apr 21, 2023 •

edited

Loading

mkevins Apr 25, 2023

ovitrif Apr 25, 2023 •

edited

Loading

mkevins left a comment

mkevins commented Apr 25, 2023

ovitrif commented Apr 25, 2023 •

edited

Loading

ovitrif commented Apr 25, 2023

Implement final screen UI #18305

Implement final screen UI #18305

Conversation

ovitrif commented Apr 21, 2023 • edited Loading

To Test

Prerequisite

● Treatment Variation

○ Control Variation

Previews

Portrait - Light

Portrait - Dark

Regression Notes

wpmobilebot commented Apr 21, 2023 • edited Loading

wpmobilebot commented Apr 21, 2023 • edited Loading

mkevins Apr 25, 2023

Choose a reason for hiding this comment

ovitrif Apr 25, 2023 • edited Loading

Choose a reason for hiding this comment

mkevins left a comment

Choose a reason for hiding this comment

mkevins commented Apr 25, 2023

ovitrif commented Apr 25, 2023 • edited Loading

ovitrif commented Apr 25, 2023

ovitrif commented Apr 21, 2023 •

edited

Loading

wpmobilebot commented Apr 21, 2023 •

edited

Loading

wpmobilebot commented Apr 21, 2023 •

edited

Loading

ovitrif Apr 25, 2023 •

edited

Loading

ovitrif commented Apr 25, 2023 •

edited

Loading