jetpack icon indicating copy to clipboard operation
jetpack copied to clipboard

Custom CSS: add deprecation warning for Start Fresh option

Open monsieur-z opened this issue 1 year ago • 5 comments

Proposed changes:

The Custom CSS feature will soon be deprecated from Jetpack.

In this PR, we're adding warnings for sites with the Start Fresh option enabled. Since this option prevents the site from loading the theme CSS, removing it will likely break the site design.

Site admins must take action, so these warnings must be visible. We've added them in 4 different places:

  • The Customizer
  • At the top of the Jetpack Dashboard and Settings pages
  • In the Custom CSS section of the Settings page
  • At the top of the frontend of the site, for admins only

Other information:

  • [ ] Have you written new tests for your changes, if applicable?
  • [ ] Have you checked the E2E test CI results, and verified that your changes do not break them?
  • [ ] Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

pfwV0U-4M-p2

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

Prerequisites

  • Spin up a test site with this branch

  • Connect Jetpack

  • Search for the Custom CSS section in the settings page: /wp-admin/admin.php?page=jetpack#/settings?term=css

  • Enable the CSS customization panel enhancement Screenshot 2024-05-02 at 4 05 00 PM

  • Go to the Additional CSS section of the Customizer: /wp-admin/customize.php?autofocus%5Bsection%5D=custom_css

  • Enable the Start Fresh option Screenshot 2024-05-02 at 4 07 49 PM

Notices

  • Refresh the page

  • Notice the warning at the top of the Customizer sidebar Screenshot 2024-05-02 at 4 09 04 PM

  • Go back to the Custom CSS section in the settings section: /wp-admin/admin.php?page=jetpack#/settings?term=css

  • Verify it now displays a warning Screenshot 2024-05-02 at 4 10 10 PM

  • Check that a similar warning is shown at the top of the page (in the Settings and Dashboard pages) Screenshot 2024-05-02 at 4 10 46 PM

  • Visit the frontend of the site: check that you see a notice when you're authenticated as an admin only Screenshot 2024-05-14 at 10 46 57 AM

  • In the Customizer, disable the Start Fresh option

  • Check the warnings are not displayed anymore

monsieur-z avatar May 02 '24 19:05 monsieur-z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • :white_check_mark: Include a description of your PR changes.
  • :white_check_mark: Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • :white_check_mark: Add testing instructions.
  • :white_check_mark: Specify whether this PR includes any changes to data or privacy.
  • :white_check_mark: Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation :robot:


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped. Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged. If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.


Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

  • WordPress.com Simple releases happen daily.
  • WoA releases happen weekly.
  • Releases to self-hosted sites happen monthly. The next release is scheduled for July 2, 2024 (scheduled code freeze on July 1, 2024).

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

github-actions[bot] avatar May 02 '24 19:05 github-actions[bot]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the update/custom-css-start-fresh-warning branch.

  • To test on Simple, run the following command on your sandbox:

    bin/jetpack-downloader test jetpack update/custom-css-start-fresh-warning
    

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions[bot] avatar May 02 '24 19:05 github-actions[bot]

I've updated the link the notices, and added a notice in the frontend. I've updated the instructions accordingly.

monsieur-z avatar May 14 '24 14:05 monsieur-z

Thanks @coder-karen!

As for an Atomic test site though (WoA dev site using rsync, and regular AT site using the beta tester plugin), I noticed that the dashboard notices weren't showing up.

Interesting. I haven't been able to reproduce the issue on a regular AT site. Did you hit the Publish button in the Customizer after checking the option? Screenshot 2024-05-23 at 2 06 19 PM

As for WoA sites, they shouldn't be impacted by this PR since they load their own plugin instead of the Jetpack module.

monsieur-z avatar May 23 '24 18:05 monsieur-z

Interesting. I haven't been able to reproduce the issue on a regular AT site. Did you hit the Publish button in the Customizer after checking the option?

My AT sites have a Save button, not a Publish button. Presumably the same thing. Confirmed that making sure the changes are saved doesn't make a difference.

coder-karen avatar May 24 '24 12:05 coder-karen

After talking with @coder-karen, I realized I made an unnecessary distinction between AT and WoA sites. This PR is not applicable to Atomic sites, which continue to use the WordPress.com implementation of the Custom CSS feature instead of Jetpack's.

monsieur-z avatar May 27 '24 20:05 monsieur-z

After unchecking the Start Fresh box and publishing/saving, and after refreshing the page too, it still displays the notice.

2024-05-27 at 16 14

kraftbj avatar May 27 '24 21:05 kraftbj

After unchecking the Start Fresh box and publishing/saving, and after refreshing the page too, it still displays the notice.

Good catch, thanks! The Customizer page still needs a refresh, but I'd say that's good enough.

monsieur-z avatar May 28 '24 14:05 monsieur-z

When testing with multiple themes at the same time weird things start happening.

The warning at the top of the Customizer sidebar respects the Start Fresh status for the corresponding theme, but the other Warnings (Settings and Dashboard pages, and the one at the top of the customizer as well as frontend) seem to be following the last applied status in any theme.

As an example see the image below where we see the warning on top of the customizer but there is no Warning on the sidebar and Start Fresh option is not checked for this theme. (Not related to not having refreshed the page) image

Steps to reproduce.

  • Spin up a test site with this branch
  • Install a classic theme (i.e. non block-based) such as Twenty Eleven
  • Connect Jetpack
  • Search for the Custom CSS section in the settings page: /wp-admin/admin.php?page=jetpack#/settings?term=css
  • Enable the CSS customization panel enhancement
  • Go to the Additional CSS section of the Customizer: /wp-admin/customize.php?autofocus%5Bsection%5D=custom_css
  • Enable the Start Fresh option
  • Notice the warning at the top of the Customizer sidebar
  • Go back to the Custom CSS section in the settings section: /wp-admin/admin.php?page=jetpack#/settings?term=css
  • Verify it now displays a warning
  • Check that a similar warning is shown at the top of the page (in the Settings and Dashboard pages)
  • Install a different theme
  • Go to the Customizer: /wp-admin/customize.php

darssen avatar May 30 '24 20:05 darssen

Thanks @darssen!

I don't reckon this path is very likely, so I think we can proceed.

Fixing what you're mentioning doesn't seem possible (at least to me) given how we check if the start fresh option is enabled (i.e. querying custom posts in the DB), and I can't think of a better way to handle the latter. That being said, I added a condition in the function that checks if the start fresh option is enabled that returns false if the theme is a block-based theme. That should prevent users upgrading to a more recent theme from seeing the notice.

monsieur-z avatar May 31 '24 16:05 monsieur-z

Thanks for your reviews. I'm having a hard time reproducing what you mentioned, @coder-karen. But your suggestion doesn't break anything regarding self-hosted sites so I've committed it.

monsieur-z avatar Jun 03 '24 19:06 monsieur-z

Instructions updated. PR's ready for review.

monsieur-z avatar Jun 21 '24 20:06 monsieur-z