-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Home Page Picker] Update the design of the “Choose a Domain” screen #15324
Conversation
Generated by 🚫 dangerJS |
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
@kyleaparker @iamthomasbishop Here's the UI changes for the Choose a Domain screen. I decided to go forward using the same collapsing header of the other screen. I felt like just making the font and UI changes resulted in a weird feel. I did try and preserve some of the existing behaviors though. When you get a chance I'd love to get your thoughts. |
@chipsnyder I will reserve final judgement until I can get my hands on a test build, but based on these screenshots, it's looking pretty sharp! The only thing that looks off to me is the spacing between navigation bar and the large title — feels a little cramped. Can't remember off the top of my head what we're using on the other sheets (I think 24?) and it might be too much spacing, but I think adding 8-16pts here might be sufficient. |
Thanks @iamthomasbishop! As for the spacing there it's actually the exact same on all instances. I'm using a reusable control where we can only modify the title and prompt for that spacing. |
Ah, fair enough! Perhaps if the spacing is that tight across all instances, we should increase it a tad? |
Yup that's easy enough. I'll add a bit of space to the build once it's working and we can tweak it across the board as needed. |
Sorry about that build failure @iamthomasbishop and @kyleaparker. A new one with that top spacing is available now. |
Conflicts: WordPress/Classes/ViewRelated/Gutenberg/Collapsable Header/CollapsableHeaderViewController.swift WordPress/Classes/ViewRelated/Site Creation/DesignSelection/SiteDesignContentCollectionViewController.swift
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.
The code LGTM and the App behaves as expected 🎉
Nice work Chip 👍
Hey @kyleaparker, @SylvesterWilmott, @iamthomasbishop I added that extra spacing mentioned here: #15324 (comment) To give you all a more comprehensive look I'm going to merge this branch and #15349 This will allow us to provide a more comprehensive build to a call for testing. I'll ping you all there once the build is avaialble |
Fixes wordpress-mobile/gutenberg-mobile#2722
To test:
📓 This doesn't include the PR to enable HPP by default so you'll still need to toggle the feature on if not already enabled
To disable or enable the development version of Home Page Picker
When enabled, the first step of the Site Creation flow will be replaced with the Home Page Picker flow.
Initial loading and Navigation
Searching for a Domain
Create Site
Searching for a Domain - No Results
google
and single letters but this seemed server-dependent)Searching for a Domain - No Network
Analytics
📓 While testing this I noticed that if you navigate away and back to any of the site creation screens then the analytics for views don't fire again. This is consistent with the production versions of these screens though so I didn't change it.
enhanced_site_creation_domains_accessed
fireenhanced_site_creation_domains_selected
fire withchosen_domain
populated with your selected domain andsearch_term
populated with the term you searched with.Screenshots:
Additional Context:
This is a continuation of the work done in the Home Page Picker Project and is part of the Round 2 requirements.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.