search

WordPress / wporg-main-2022

A block-based child theme for WordPress.org, plus local environment
62 stars 22 forks source link

Rosetta styles: Use Noto Serif JP for headings #455

Closed ryelle closed 1 week ago

ryelle commented 2 weeks ago

Noto Serif JP is being added to the available fonts with https://github.com/WordPress/wporg-mu-plugins/pull/630 & https://github.com/WordPress/wporg-parent-2021/pull/144. This PR updates the main site to use Noto Serif JP for headings.

The font preloading is also updated here, so that ja.w.org only preloads Noto Serif, not EB Garamond.

Screenshots

Before After
home-before home
download-before download
about-page-before about-page

How to test the changes in this Pull Request:

  1. Apply all three PRs across wporg-main-2022 (this one), https://github.com/WordPress/wporg-parent-2021/pull/144, and https://github.com/WordPress/wporg-mu-plugins/pull/630 (I know 😩)
  2. Follow the instructions to set up a non-English test instance, use Japanese to test with
  3. Once set up, view the frontend of the site
  4. All headings should use Noto Serif JP
  5. Noto Serif JP should be preloaded in the head tags
  6. Check on the default (english) site and/or other locales, they should still use EB Garamond

Note that EB Garamond does still load, it's hardcoded in the the global header site title. It might also appear if the font is set at the block level. We could override --wp--preset--font-family--eb-garamond with Noto Serif JP if it should be eradicated totally.

StevenDufresne commented 2 weeks ago

The code looks fine across all the Pull Requests.

I haven't tested this on my sandbox due to the complexity of setting it up, but I'm comfortable with merging this.

Thanks πŸ‘

ryelle commented 1 week ago

Except for the text "始めましょう" doesn't seem to have the new font applied (I saw in the screenshots that its font was also changed). Will there be a commit later to update this font?

Only places that currently use the "heading font" (EB Garamond) will be updated, if we need to change "始めましょう" it will need different CSS. I don't see that specifically requested in #432, though.

I couldn't test it locally as it kept showing this error on the homepage to me

I don't think I've seen that specifically, but maybe the WP version in the env is outdated? That doesn't update with node or composer packages, you can run wp-env start --update to update it.

renintw commented 1 week ago

Except for the text "始めましょう" doesn't seem to have the new font applied (I saw in the screenshots that its font was > also changed). Will there be a commit later to update this font?

Only places that currently use the "heading font" (EB Garamond) will be updated, if we need to change "始めましょう" it will need different CSS. I don't see that specifically requested in #432, though.

Ah, what I meant was that in the screenshots you provided, the font (or maybe just the size?) of "始めましょう" looks different. That's why I was wondering if there might be some changes that weren't applied in the code.

image

I couldn't test it locally as it kept showing this error on the homepage to me

I don't think I've seen that specifically, but maybe the WP version in the env is outdated? That doesn't update with node or composer packages, you can run wp-env start --update to update it.

It doesn't seem to be a WordPress version issue. After deleting gutenberg and reinstalling, it worked. The previous composer update that I've run might not have successfully updated gutenberg somehow, possibly the local environment was polluted.

I also tried cloning a new repo and building from scratch, and the error didn't show up, but t here's another error:

cURL error 35: OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to wordpress.org:443 βœ” Ran `php env/import-content.php --url https://wordpress.org/wp-json/wp/v2/posts?context=wporg_export&per_page=50` in 'cli'. (in 1s 526ms)

This caused the content to not be imported, resulting in a 404 error on the initial visit to the homepage (this can be resolved by adjusting the Reading Settings, though).

Have you run into this, possibly a proxy issue I presume?

ryelle commented 1 week ago

Ah, what I meant was that in the screenshots you provided, the font (or maybe just the size?) of "始めましょう" looks different. That's why I was wondering if there might be some changes that weren't applied in the code.

Oh, I think my before screenshot was not quite up to date, it was missing this change to shrink the CTA font size.

Have you run into this, possibly a proxy issue I presume?

Sounds like a proxy issue to me, but I don't think I've run into that exactly.