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

Add/editing toolkit coming soon #45167

Closed
wants to merge 12 commits into from
Closed

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Aug 25, 2020

Changes proposed in this Pull Request

This is a PR to try things out. Test our approach.

Testing instructions

D45700-code

Fixes #

@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D48549-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

@matticbot
Copy link
Contributor

matticbot commented Aug 25, 2020

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~17 bytes added 📈 [gzipped])

name        parsed_size           gzip_size
entry-main       +103 B  (+0.0%)      +17 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~1370 bytes added 📈 [gzipped])

name                  parsed_size           gzip_size
pages                     +1487 B  (+0.7%)     +263 B  (+0.4%)
settings                   +692 B  (+0.2%)     +140 B  (+0.1%)
settings-writing           +169 B  (+0.0%)      +51 B  (+0.0%)
checkout                   +166 B  (+0.0%)      +62 B  (+0.0%)
themes                     +159 B  (+0.0%)      +58 B  (+0.1%)
settings-performance       +159 B  (+0.1%)      +51 B  (+0.1%)
plugins                    +159 B  (+0.0%)      +58 B  (+0.1%)
hosting                    +159 B  (+0.1%)      +58 B  (+0.1%)
woocommerce                +152 B  (+0.0%)      +49 B  (+0.0%)
settings-security          +152 B  (+0.1%)      +49 B  (+0.1%)
settings-discussion        +152 B  (+0.1%)      +49 B  (+0.1%)
scan                       +152 B  (+0.0%)      +56 B  (+0.1%)
post-editor                +152 B  (+0.0%)      +47 B  (+0.0%)
media                      +152 B  (+0.0%)      +47 B  (+0.0%)
marketing                  +152 B  (+0.0%)      +49 B  (+0.1%)
home                       +152 B  (+0.0%)      +55 B  (+0.0%)
gutenberg-editor           +152 B  (+0.0%)      +44 B  (+0.0%)
comments                   +152 B  (+0.0%)      +50 B  (+0.0%)
backup                     +152 B  (+0.0%)      +56 B  (+0.1%)
activity                   +152 B  (+0.0%)      +59 B  (+0.1%)
jetpack-connect             +24 B  (+0.0%)      +13 B  (+0.0%)
signup                      +14 B  (+0.0%)       +3 B  (+0.0%)
accept-invite               +14 B  (+0.0%)       +3 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~181 bytes added 📈 [gzipped])

name                                                 parsed_size           gzip_size
async-load-design-blocks                                  +152 B  (+0.0%)      +67 B  (+0.0%)
async-load-signup-steps-clone-point                       +142 B  (+0.1%)      +49 B  (+0.1%)
async-load-blocks-calendar-popover                        +142 B  (+0.1%)      +40 B  (+0.1%)
async-load-blocks-inline-help-popover                      +72 B  (+0.0%)      +25 B  (+0.0%)
async-load-my-sites-site-settings-seo-settings-form        +17 B  (+0.0%)       +0 B

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

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.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@ramonjd ramonjd force-pushed the add/editing-toolkit-coming-soon branch from 9c445d3 to b99f501 Compare August 26, 2020 05:50
@ramonjd ramonjd self-assigned this Aug 30, 2020
@ramonjd ramonjd force-pushed the add/editing-toolkit-coming-soon branch from f6a6fb2 to 2af7ddc Compare September 1, 2020 00:40
@andrewserong
Copy link
Member

I don't have much to add yet, but this is coming along nicely already! I was pleasantly surprised at how easily this seems to be able to override the existing coming soon mode by just setting public to 0, and therefore none of the existing coming soon behaviour will fire. I think I might have been overthinking things in regards to supporting coming soon v1.

I'm not too sure where you're up to in the PR yet, but I was able to successfully display the fallback coming soon on my test site, but switching a page to Coming Soon from the pages view looks like it's doing the correct thing in Calypso, but doesn't appear to show the page for me yet. Calling get_blog_option in my sandbox shows that the wpcom_coming_soon_page_id setting hasn't been updated even though the response from the server with D45700-code is applied looks like it has... I might be missing something obvious here, so will give it another try tomorrow!

@ramonjd
Copy link
Member Author

ramonjd commented Sep 1, 2020

thanks for testing @andrewserong !!

Calling get_blog_option in my sandbox shows that the wpcom_coming_soon_page_id setting hasn't been updated even though the response from the server with D45700-code is applied looks like it has... I might be missing something obvious here, so will give it another try tomorrow!

I was just trying to get that working this afternoon when time ran out. Feel free to take a stab tomorrow if you dare ;)

@andrewserong
Copy link
Member

Thanks @ramonjd! Got it working, just needed a tiny update to D45700-code (the site option was set to page_for_coming_soon so I've changed it to wpcom_coming_soon_page_id to match this PR). Seems to be working okay, now!

@andrewserong
Copy link
Member

I thought I might have a go at copying over the markup for the existing Coming Soon page to this PR later on this afternoon — but let me know if I'll be stepping on toes by messing around in this PR and I can branch off this one, or look at a different area (like getting the launch / logged in coming soon banner to support the new public coming soon mode).

@ramonjd
Copy link
Member Author

ramonjd commented Sep 2, 2020

the site option was set to page_for_coming_soon so I've changed it to wpcom_coming_soon_page_id to match this PR

Awesome, thank you for catching that one I missed!

I thought I might have a go at copying over the markup for the existing Coming Soon page to this PR later on this afternoon

Go ahead to do whatever you want. Don't wanna break your stride. I'll peel off and look at something else.

@andrewserong andrewserong self-assigned this Sep 2, 2020
@andrewserong
Copy link
Member

andrewserong commented Sep 2, 2020

Fallback coming soon

I've added the current Coming Soon markup as the fallback and got it to load Recolata

  • I notice the current theme is overriding some styling to do with the buttons, which will need to be fixed up on mobile

image

Coming soon page selected

Here's what it looks like if I pick a Coming Soon page (here I've just created a simple page with a big Cover block and gradient background):

image

Other screenshots of the current state of the PR

Just adding a couple of other screenshots to show the current state of the PR for anyone else looking on :)

image

image

There's still heaps more to do of course!

@roo2
Copy link
Contributor

roo2 commented Sep 7, 2020

Amazing progress!
I tested locally, here's what I've found still to do so far

Settings Page

  • Make unlaunched sites start as coming soon, make launching a site set it to public, not coming soon
  • I think the coming soon settings section should have a link to set the coming soon page

Pages List

  • We should probably only be able to set the coming soon page if the settings is enabled? or otherwise indicate that the page won't be used if the setting is not enabled.
  • setting coming soon pages triggers a " Homepage is showing latest posts" message, even if homepage is set to an existing page 🤔
  • once coming soon page is set, the option to set homepage and latest post page disappears from the dropdown menu next to the page 🤔
  • I think sites should start with a default coming soon page (if you're on the right plan?).
  • Make the coming soon page feature only work for paid users and communication around that

Live site

  • When I had a coming soon page set, as a logged out user, I continued to see the default coming soon page.
  • When logged in, the v1 coming soon shows a site banner saying "your site is not launched yet, only you can see it until the site is launched" I think we should do something like that too.
  • Fix login link on default coming soon page, currently it just goes to #
  • a8cs can see peoples coming soon sites, is that expected?

@ramonjd
Copy link
Member Author

ramonjd commented Sep 7, 2020

Thanks for capturing these ideas!

Make unlaunched sites start as coming soon, make launching a site set it to public, not coming soon

Maybe we could start on this, putting it behind some query option in sites/new and/or feature flag? There's an issue that loosely related to this topic.

I think the coming soon settings section should have a link to set the coming soon page

To the editor version? Either way, good idea. Noted.

setting coming soon pages triggers a " Homepage is showing latest posts" message, even if homepage is set to an existing page 🤔
once coming soon page is set, the option to set homepage and latest post page disappears from the dropdown menu next to the page 🤔

This would be worth looking into now too @roo2

Not sure what the right logic would be, but I guess we should be able to switch between home and coming soon types as we wish. Noting that if a user switches a coming soon page to be the home page, they'll fallback back to the default coming soon page (if the feature is activated.

a8cs can see peoples coming soon sites, is that expected?

I guess so. I think only the user themselves sees the full site if they're logged in (and with coming soon enabled)

Fix login link on default coming soon page, currently it just goes to #

Maybe we could address that in #44888 ?

@roo2
Copy link
Contributor

roo2 commented Sep 7, 2020

setting coming soon pages triggers a " Homepage is showing latest posts" message, even if homepage is set to an existing page 🤔
once coming soon page is set, the option to set homepage and latest post page disappears from the dropdown menu next to the page 🤔
This would be worth looking into now too @roo2

Great, yea I'd quite like to take a look at this on this PR/branch

@roo2 roo2 force-pushed the add/editing-toolkit-coming-soon branch from 0fc52af to 4b266c1 Compare September 10, 2020 01:51
@roo2
Copy link
Contributor

roo2 commented Sep 10, 2020

rebased

remove_action( 'wp_head', 'global_css', 5 );
remove_action( 'wp_footer', 'wpcom_subs_js' );
remove_action( 'wp_footer', 'stats_footer', 101 );
wp_enqueue_style( 'recoleta-font', '//s1.wp.com/i/fonts/recoleta/css/400.min.css' );
Copy link
Member

@simison simison Sep 10, 2020

Choose a reason for hiding this comment

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

No strong opinion on this, but you might also look at importing in CSS from the typography package. That'll keep it more in one source of truth for fonts.

@lsl lsl mentioned this pull request Sep 11, 2020
@lsl
Copy link
Contributor

lsl commented Sep 11, 2020

Did a bit of work on this in #45581 let me know if all that is ok to push into this, I'm not sure I've done the typography import right so didn't feel too comfortable pushing into this PR directly.

@ramonjd ramonjd mentioned this pull request Sep 14, 2020
Copy link
Collaborator

@wp-desktop wp-desktop left a comment

Choose a reason for hiding this comment

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

WordPress Desktop CI Failure for job "wp-desktop-mac".

@roo2 please inspect this job's build steps for breaking changes at this link. For temporal failures, you may try to "Rerun Workflow from Failed".

Please also ensure this branch is rebased off latest Calypso.

@lsl lsl mentioned this pull request Sep 14, 2020
@ramonjd
Copy link
Member Author

ramonjd commented Sep 14, 2020

Closing this one in favour of #45581

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.

None yet

7 participants