Skip to content
This repository was archived by the owner on Feb 17, 2025. It is now read-only.

Conversation

@jeyip
Copy link
Contributor

@jeyip jeyip commented May 6, 2022

Description

More context can be found here paYE8P-1B3-p2#comment-1506.

This PR displays a deprecation notice in the blockbase font customizer if gutenberg and jetpack have the correct versions and configurations to have site editor google fonts enabled.

NOTE: For dotcom simple sites

Screenshots

Before

Screen Shot 2022-05-06 at 1 25 09 PM

After

Screen Shot 2022-05-06 at 3 58 11 PM

Testing

  1. Sandbox a simple site
  2. Copy the changes made in blockbase/inc/customizer/wp-customize-fonts.php to its mirrored file in the dotcom codebase wp-content/themes/pub/blockbase/inc/customizer/wp-customize-fonts.php
  3. Activate a Blockbase theme or a theme that uses Blockbase as a parent ( Blockbase, Videomaker, etc. ).
  4. Navigate to the Customizer
  5. Navigate to the fonts panel
  6. Verify that fonts are no longer available for selection for dotcom simple sites.
  7. Repeat steps and test on an atomic site

Related issue(s):

Automattic/wp-calypso#63229

@jeyip jeyip self-assigned this May 6, 2022
$gutenberg_webfonts_api_supports_enqueueing = version_compare( GUTENBERG_VERSION, '13.0', '>=' );
}

return $jetpack_has_google_fonts_module && $gutenberg_webfonts_api_supports_enqueueing && Jetpack::is_module_active( 'google-fonts' );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: Jetpack::is_module_active always returns true for dotcom simple regardless of what value is passed to it. Unsure why at the moment

Copy link
Contributor

Choose a reason for hiding this comment

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

@jeyip yup that is by design. From a code comment on WPCOM_Site::is_module_active:

Always returns true. In Jetpack context it would actually check if a module is active or not because the module loading relies on the 'jetpack_active_modules' option. Here in WordPress.com context however, we always load everything, so in order to enable or disable something we have to check each module's settings.

@jeyip jeyip requested a review from scruffian May 6, 2022 22:59
@MaggieCabrera MaggieCabrera requested a review from a team May 9, 2022 14:50
@MaggieCabrera
Copy link
Contributor

Is this still a WIP or can the themes team review too?

@creativecoder
Copy link
Contributor

@MaggieCabrera You're welcome to review, but know this is very much a work in progress.

We need to figure out how to migrate the legacy Blockbase Customizer settings so they don't prevent other Global Style font families from displaying, like what's happening now (as tracked in Automattic/wp-calypso#63229).

Once that's figured out, users can use the Global styles options to set font families, rather than the Customizer options, so we're thinking the Customizer options aren't needed. Please do let us know if you have thoughts or ideas on this!

@kspilarski
Copy link

Hurray! Your theme supports Full Site Editing with blocks. Tell me more.

In the customizer, Tell me more opens to a .org document: https://wordpress.org/support/article/site-editor/
Can we update it to our doc instead https://wordpress.com/support/full-site-editing/ ? Thank you!

@MaggieCabrera
Copy link
Contributor

@MaggieCabrera You're welcome to review, but know this is very much a work in progress.

We need to figure out how to migrate the legacy Blockbase Customizer settings so they don't prevent other Global Style font families from displaying, like what's happening now (as tracked in Automattic/wp-calypso#63229).

Once that's figured out, users can use the Global styles options to set font families, rather than the Customizer options, so we're thinking the Customizer options aren't needed. Please do let us know if you have thoughts or ideas on this!

It is very cool to see that you all picked up on this. We built Blockbase with this moment in mind: when Blockbase was created there was no option for color or font customization on block themes, and we decided to bridge that gap using the customizer, but knowing that the future (now the present!) would bring a solution that would work directly in the FSE, we wanted our implementation to be compatible with that.

I'd have to review the code again but the idea was that the customizer options selected by the user would get saved in the user portion of theme.json, not just as a customizer variable, so that the FSE would be aware of that selection and would be able to reflect that. There might be some changes that we need to do to transition to what Jetpack now does, but our script should be GS compatible.
I'll try to have a look more closely, in any case I want to tag @pbking on this too. I believe 6.0 could be the moment when Blockbase says finally goodbye to the customizer. We could add the same message to the colors section, specially now that the style variations are a thing.

$gutenberg_webfonts_api_supports_enqueueing = false;

if ( defined( 'JETPACK__VERSION' ) ) {
$jetpack_has_google_fonts_module = JETPACK__VERSION === 'wpcom' || version_compare( JETPACK__VERSION, '10.8', '>' );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO:
Remove unnecessary reference to wpcom. We control all versioning for Jetpack, Gutenberg, and jetpack modules for simple sites.

@creativecoder
Copy link
Contributor

Logic from this PR has been incorporated into #6010

@scruffian scruffian deleted the update/blockbase-to-hide-font-customizer-if-fse-google-fonts-enabled branch May 18, 2022 08:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants