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

Only load Premium Block CSS when that block is being used #42590

Merged
merged 2 commits into from
May 25, 2020

Conversation

josephscott
Copy link
Contributor

Currently when you activate the Full Site Editing plugin the style.css for the Premium Content Block gets loaded on ever page view. This change adjusts it to only load on pages where that block is being used.

Changes proposed in this Pull Request

  • Only load the Premium Content Block CSS when that block is in use

Testing instructions

  • Activate the FSE plugin
  • Notice the Premium Content Block style.css being loaded on every page
  • Apply patch
  • Now style.css should only be loaded on pages that use the Premium Content Block

Fixes #

Currently when you activate the Full Site Editing plugin the style.css for the Premium Content Block gets loaded on ever page view.  This change adjusts it to only load on pages where that block is being used.
@matticbot
Copy link
Contributor

@gwwar gwwar added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 22, 2020
@gwwar gwwar requested review from a team May 22, 2020 17:35
@matticbot
Copy link
Contributor

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

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

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Close, but it looks like we have some shared styling we'll need to pull out of

* The following styles get applied both on the front of your site
and into https://github.com/Automattic/wp-calypso/blob/3372459cc937cb63cf968140c10af46cc0d2ebe9/apps/full-site-editing/full-site-editing-plugin/premium-content/editor.css

Let's also update the comment at the top of style.css to note that this will be added to the published view only. @sixhours @danhauk or @Automattic/serenity @Automattic/earn any interest in helping sort styling?

Before After
Screen Shot 2020-05-22 at 11 29 46 AM Screen Shot 2020-05-22 at 11 26 08 AM

@eoigal
Copy link
Contributor

eoigal commented May 22, 2020

I copied over the styles that were needed in the editor. The toolbar styles shouldn't have been in style.css at all so I moved those too.

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Thanks @josephscott for the optimization and @eoigal for the style cleanup! I verified that the editor styles look reasonable and that the published pages look good.

I did spot some minor focus changes before and after, but they both look a little off, so this is non-blocking.

Screen Shot 2020-05-22 at 1 50 56 PM

Screen Shot 2020-05-22 at 1 52 10 PM

@eoigal eoigal merged commit 292c8c3 into master May 25, 2020
@eoigal eoigal deleted the premium-block-css branch May 25, 2020 09:58
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 25, 2020
@ockham ockham mentioned this pull request May 29, 2020
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants