Skip to content

Conversation

@coder-karen
Copy link
Contributor

@coder-karen coder-karen commented Mar 29, 2022

Fixes #23598

Changes proposed in this Pull Request:

  • On Jetpack plugin activation (prior to connection or toggling the Custom CSS feature within Jetpack's Writing settings page), Jetpack will add an 'Additional CSS' sub-menu link under the 'Appearance' menu item.
  • When the Custom CSS toggle is also on (via Jetpack > Settings > Writing):
    • On FSE-themed sites, no additional menu links are added - the 'Additional CSS' menu item remains (but the Customizer includes the additional Jetpack-added Custom CSS features). Prior to this change, the Customise sub-menu item was added by default when Jetpack's Custom CSS feature was enabled.
    • On non-FSE-themed sites, the Customise sub-menu item remains (both before and after the Custom CSS toggle is toggled), as this isn't added by Jetpack.
  • The 'Additional CSS' sub-menu link links to the Custom CSS section of the Customizer if Jetpack's Custom CSS feature is enabled. If not, it links to the Jetpack > Settings > Writing page, specifically highlighting the Custom CSS settings card.
  • The Custom CSS settings card is now enhanced to include a link to the Customizer and an explanation about how the Jetpack module will enhance the existing Custom CSS section.

Jetpack product discussion

p9dueE-4DZ-p2
p1HpG7-eWn-p2
p1648132696510739/1648051014.297169-slack-C029LRAU2

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

No.

Questions:

  • The function customizer_link is duplicated now - it’s in class.jetpack-admin.php and custom-css-4.7.php. Calling it from one or the other file means requiring the opposite file and generating a new instance of the class just to reach that file. I’m uncomfortable about duplicating the function however it would rarely be used in custom-css-4.7php.
  • What might I not have considered in terms of adding that sub-menu link in prior to the Custom CSS toggle being toggled on, and separately would it make sense to go back to the default when the toggle is on for non-FSE themed sites and add the Customise sub-menu back?

Testing instructions:

  • Test the scenarios mentioned under 'Changes..' above. To test block-based themes, a good option is TT1. You can also test with the most recent version of the Gutenberg plugin installed to confirm the latest block-based changes are being included.
  • If you happen to have any legacy Customizer / Additional CSS bookmarks it would be helpful to test those too.
  • Note - the Additional CSS menu link itself seems temperamental with local testing (also the case with other sub-menu items, not Jetpack related) - clicking the link via the inspector works though. Testing via something like JN/JT also works.

@coder-karen coder-karen added [Feature] Custom CSS [Status] In Progress [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ labels Mar 29, 2022
@coder-karen coder-karen self-assigned this Mar 29, 2022
@coder-karen coder-karen changed the title Custom CSS: Adding an Additional CSS menu item on JP connection Custom CSS: Adding an Additional CSS menu item on JP activation Mar 29, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2022

Thank you for your PR!

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

  • ✅ Include a description of your PR changes.
  • ⚠️ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ 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 🤖


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 (other than "Required review") 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 you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.


Jetpack plugin:

  • Next scheduled release: June 7, 2022.
  • Scheduled code freeze: May 31, 2022.

@coder-karen
Copy link
Contributor Author

Opening this up for initial reviews for now so that it doesn't sit for several weeks.

@coder-karen coder-karen added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Apr 6, 2022
@coder-karen coder-karen marked this pull request as ready for review April 6, 2022 12:29
jeherve
jeherve previously requested changes Apr 7, 2022
@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Apr 7, 2022
@coder-karen coder-karen requested a review from jeherve April 12, 2022 13:05
@coder-karen coder-karen added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Apr 12, 2022
@samiff samiff self-requested a review April 19, 2022 18:10
@coder-karen
Copy link
Contributor Author

I wonder if when the module is disabled, the "Additional CSS" links to that specific Jetpack setting to read about & enable the module. And if the module is enabled, the menu item links to the CSS customizer. What do you think about that?

I think that is a great idea! I'll look into adding that too.

@github-actions github-actions bot added the Admin Page React-powered dashboard under the Jetpack menu label May 6, 2022
@coder-karen coder-karen added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels May 6, 2022
@coder-karen
Copy link
Contributor Author

New changes ready for review (new PHPCS errors came up as well, my fix was relatively quick so may need more eyes - related to $_SERVER['REQUEST_URI'] in custom-css-4.7.php on lines 51 to 64, and similar code in class.jetpack-admin.php, lines 193 to 199).

@samiff samiff self-requested a review May 6, 2022 19:49
Copy link
Contributor

@samiff samiff left a comment

Choose a reason for hiding this comment

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

Tested this again with 2020/2022 themes and both worked great 👍 I like the redirect to the settings page to enable the module now.

@samiff samiff added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels May 6, 2022
@jeherve jeherve added this to the jetpack/11.0 milestone May 9, 2022
@jeherve jeherve merged commit eb5d63e into master May 10, 2022
@jeherve jeherve deleted the update/custom-css-menu branch May 10, 2022 10:13
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label May 10, 2022
@bindlegirl bindlegirl added the [Type] Bug When a feature is broken and / or not performing as intended label May 10, 2022
@obenland
Copy link
Member

obenland commented Jun 2, 2022

It looks like this might have introduced a regression with #19965 (and 693-gh-Automattic/wpcomsh), where on WoA sites without a plan it now displays two "Additional CSS" menu items, with the one introduced here linking to an empty search result for Custom CSS.

image

Maybe this else clause could check for the custom-css feature before adding that link?

@coder-karen
Copy link
Contributor Author

Thanks for reporting this @obenland . We're aiming to have a fix in for the next Atomic weekly release this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Admin Page React-powered dashboard under the Jetpack menu [Feature] Custom CSS [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: Make 'Additional CSS' easier to find

8 participants