-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Site migration: add View Site button to homepage banner #39632
Conversation
@danhauk Customising the checklist on My Home turned out to be tricky – it wasn't obvious how I could adapt it to show a special checklist for our situation, and we don't currently show a checklist for Atomic sites anyway. So I've gone with this as a workaround. |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~4 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
Thanks @andfinally. Let's hold off on trying to get a customized checklist working as we're re-evaluating the usefulness of leaning on a checklist for everything. Rather than creating a custom banner like this, could we utilize the existing pattern established with the "Your site is launched" banner? Keeping the page header with "My Home" and the |
👋 @andfinally! Backing up @danhauk on keeping the View Site button and the congratulatory banner separate. Thanks for attempting the custom checklist too though, and for reminding us of that limitation pertaining to atomic sites. Seems like this is a mutually-beneficial work-around! I haven't seen the Go Mobile card show up like that before. Normally it's stacked underneath all the other content on the right-hand side. Unless this behavior is related to this PR, I'm happy with all the work 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.
Check out my last comment where I had a question I think is unrelated, I'm otherwise good!
Thanks for checking it out! I haven't moved Go Mobile in this PR – I believe that new position is intentional. I found this comment in the code:
|
{ isRecentlyMigratedSite && ( | ||
<Card className="customer-home__migrate-card" highlight="info"> | ||
<img | ||
src="/calypso/images/illustrations/fireworks.svg" |
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.
Changes proposed in this Pull Request
Testing instructions
?flags=tools/migrate
to the end of the URL to enable the feature flag).done
status hasn't been cleared yet, you may need to reset the migration first with a request to thereset-migration
endpoint.)