-
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] Enable Home Page Picker feature #15303
Conversation
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
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.
LGTM @chip 🎉
I've re-tested all the flows described, and everything is working as expected. I've also verified the events in the debugger and in the live view, and all events are emitted correctly. Two issues I observed are described here: wordpress-mobile/WordPress-Android#13409 (comment) which were also present in the iOS app. Nice work Chip! 👍 |
Thanks for testing and confirming @mkevins. I agree with what @antonis' analysis, this is probably something we can help triage to fix on the backend. I'll do some more investigating as well on the iOS side. |
@kyleaparker @SylvesterWilmott @iamthomasbishop I merged the other changes as mentioned in #15324 (comment) and updated this branch with those changes. That way we can start a call for testing and you have a more comprehensive build to look at. Build |
Running through some test cases on different devices this is a running list of issues I'm encountering. I'll branch these off into fixes soon: (Edit) The following issue(s) are being addressed in #15360 |
Thanks for the ping @chipsnyder! This is looking really great 🎉 I have reviewed the build and have a couple of comments:
|
Thank you, @kyleaparker, for your review 😃
I see what you mean. Having a more uniform loading might help make the experience more smooth. I came up with a few scenarios while thinking about this that I fear might lead to worse overall behavior and add complexity. Let me know your thoughts on it. If you feel like there is enough value to smoothing out that load then we can continue to investigate and I'll create a GH issue to track that work.
(1) Once the image is generated we cache it and it's available so only a very small number of users will ever experience this lag for an image. Hopefully, this lag will be mostly internal team members as were testing and verifying a new site design.
I think we still need more research before we charge forward with this. I went ahead and created an issue to track this though and added my initial research. wordpress-mobile/gutenberg-mobile#2828 We were able to get some of the V2 items work for the code cut off but I feel like this is something that won't make it until next release. Let me know if you feel like it's a higher priority though.
I reused the same search control from the other flow for this which had it set to a style defined in the style guide as the default search bar text attributes. Which seems to translate to the system's preferred size of footnote so if have the default accessibility font size set this would translate to 13 points. Searching through the code this looks like this is used for other search bars as well so let me know if you'd like to see some other options and explore making this a one-off case I'm happy to take some screenshots of some other cases. If I get time today I'll come back through and post those in another comment.
Given our other discussions, I think think this is a good idea. I'll go ahead and make this change in the bug fix branch. |
Ahh yes I see how this could cause more potential problems. I think this is another one we can leave as is for V1 and possibly investigate other options for V2.
Sounds good, I don't think there is any rush on this so investigating it during V2 works for me
It seems small to me, but if we are using it throughout the app let's keep it as is for now
Thank you! |
* Prevent swipe to dismiss on site creation flow * Fix layout constraint collisions * Resolve issue where the header will sometimes jump around * Correct issue where it's possible to scroll too far offscreen * Bump gutenberg version * Updated the site design prompt wording * Add the WP custom user agent to site creation webviews (#15376) * Add Template to Preview analytics (#15365) * Resolves issue where the selection remains but the results update
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.
LGTM 👍
Fixes wordpress-mobile/gutenberg-mobile#2727
To test:
This enables the Home Page Picker for the enhanced site design selection. The old flow can still be tested for comparison reasons by toggling the feature off.
To disable or enable the development version of Home Page Picker
To Test
From a clean install:
Full Feature Testing:
Create a Site - Selecting a design
Steps
enhanced_site_creation_site_design_viewed
enhanced_site_creation_site_design_selected
with the propertytemplate
populated with the selected layout's slug.blog
subdomains all others (at the time of writing this) should not.Create a Site - Skip button
Steps
enhanced_site_creation_site_design_viewed
Skip
in the top right-hand cornerenhanced_site_creation_site_design_skipped
.blog
subdomainsPreview then Create a Site
Steps
Create a Site - Selecting a design
1-8enhanced_site_creation_site_design_preview_viewed
fire with the parametertemplate
populated with the selected design'sslug
enhanced_site_creation_site_design_preview_loading
enhanced_site_creation_site_design_preview_loaded
enhanced_site_creation_error_shown
fire with the parametererror_info
with a description of the errorCreate a Site - Selecting a design
at step 10Expect ...
Error Scenario
Steps
enhanced_site_creation_error_shown
witherror_info
populated with a description of the error that occurred.Retry
and expect to be able to completeCreate a Site - Selecting a design
Skip
and expect to be able to completeCreate a Site - Skip button
Screenshots:
Choose a design
Preview
Choose a Domain
Additional Context:
This is a continuation of the work done in the Home Page Picker Project.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.