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

Google fonts don't load when enqueued late #3932

Closed
aaemnnosttv opened this issue Aug 25, 2021 · 2 comments
Closed

Google fonts don't load when enqueued late #3932

aaemnnosttv opened this issue Aug 25, 2021 · 2 comments
Labels
Module: Idea Hub Google Idea Hub module related issues P1 Medium priority Type: Bug Something isn't working
Milestone

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Aug 25, 2021

Bug Description

As discovered in #3625 (comment), when we try to enqueue Google fonts after (wp|admin)_head, the fonts end up not being loaded due to some odd legacy behavior around how they are enqueued.

We currently load our fonts primarily using the JS-based web font loader and secondarily using CSS if the site is using AMP. This is overly complex and not necessary for our use which is largely the reason for the problem here.

We should update our fonts to be loaded as a dependency of our main/base styles using CSS as is the recommended/primary approach in the Google font docs.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Google fonts used by Site Kit should always be loaded via a CSS stylesheet sourced from fonts.googleapis.com/css regardless of the AMP mode
  • These fonts should be registered as a dependency of the main Site Kit stylesheet used in wp-admin and the stylesheet used by the admin-bar integration – this means they should never need to be enqueued explicitly
  • The Assets::enqueue_fonts method should be deprecated for removal in the future but updated to continue to work similar to before (without the WebFontConfig variant though)
  • All calls to Assets::enqueue_fonts should be removed
  • The font families which are loaded should continue to be filterable using the current googlesitekit_font_families filter
  • The JS-based WebFontConfig used in Storybook should be replaced with a stylesheet tag for consistency

Implementation Brief

Test Coverage

  • No significant changes expected, may need to update a test or two due to enqueue_fonts becoming deprecated

Visual Regression Changes

  • No changes – this should be a clean refactor for VRT

QA Brief

  • See Add Idea Hub feature tour on WordPress posts page #3625 (comment) related to the feature tour for Idea Hub on the post list view – ensure Site Kit fonts are loaded in this situation and the font/styles used here match that used by feature tours on the SK dashboard
  • Check everywhere else where Site Kit adds an interface to ensure our fonts are used
    • SK dashboard, page dashboard, and module pages
    • WP Admin bar
    • WP dashboard?
    • Plugins page after activation
    • SK splash page

Changelog entry

  • Always load Google fonts using purely CSS rather than relying on a JavaScript snippet.
@aaemnnosttv aaemnnosttv self-assigned this Aug 25, 2021
@aaemnnosttv aaemnnosttv added P1 Medium priority Type: Bug Something isn't working Module: Idea Hub Google Idea Hub module related issues labels Aug 25, 2021
@felixarntz
Copy link
Member

IB ✅

@wpdarren
Copy link
Collaborator

wpdarren commented Sep 6, 2021

QA Update ✅

  • Checked where Site Kit adds an interface to ensure our fonts are used as per the QAB.
  • Also checked to make sure the font is displaying as expected on the post tour for idea hub.

image

@wpdarren wpdarren removed their assignment Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Idea Hub Google Idea Hub module related issues P1 Medium priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants