search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.51k stars 4.2k forks source link

Typography: theme.json "layout" settings cannot be custom css variables or references, must be explicit values #55070

Open ouw-jvt opened 1 year ago

ouw-jvt commented 1 year ago

Description

If you attempt to set the contentSize or wideSize property of settings.layout to a custom variable defined in settings.custom, the PHP portion of the style generation does not properly interpret the variable and map it to the real value. This causes some features to stop working, such as fluid typography.

In order to fix this, wp_get_typography_value_and_unit() from typography.php (or the methods calling it) would need to detect the presence of and interpolate the variable to its value by fetching it from wp_get_global_settings() (in the case of the below example's suggested fix, that would be $global_settings['custom'][layout][wideSize]).

While this may not be intended functionality, every other theme.json setting (that I have tried) seems to work when set to a variable. It's also useful for child theme inheritance and SASS workflows. Additional use cases are mentioned in #53525. However, if this will not be supported, a note in documentation demonstrating the formatting requirements of this particular setting would suffice.

Step-by-step reproduction instructions

In a block theme, using this example theme.json causes the fluid type (expected on the h1 style) to fail and revert to the size key. Key lines are "contentSize": "var(--wp--custom--layout--content-size)", and "wideSize": "var(--wp--custom--layout--wide-size)". CleanShot 2023-10-04 at 23 44 35@2x

{
  "$schema": "https://schemas.wp.org/trunk/theme.json",
  "version": 2,
  "settings": {
    "custom": {
      "layout": {
        "contentSize": "1692px",
        "wideSize": "1980px"
      }
    },
    "appearanceTools": true,
    "layout": {
      "contentSize": "var(--wp--custom--layout--content-size)",
      "wideSize": "var(--wp--custom--layout--wide-size)"
    },
    "typography": {
      "fluid": true,
      "fontSizes": [
        {
          "fluid": {
            "min": "1rem",
            "max": "3rem"
          },
          "name": "Large",
          "size": "3rem",
          "slug": "large"
        }
      ]
    },
    "useRootPaddingAwareAlignments": true
  },
  "styles": {
    "elements": {
      "h1": {
        "typography": {
          "fontSize": "var(--wp--preset--font-size-large)"
        }
      }
    }
  }
}

By switching back to a pixel value, things work as expected CleanShot 2023-10-04 at 23 43 13@2x

{
  "$schema": "https://schemas.wp.org/trunk/theme.json",
  "version": 2,
  "settings": {
    "custom": {
      "layout": {
        "contentSize": "1692px",
        "wideSize": "1980px"
      }
    },
    "appearanceTools": true,
    "layout": {
      "contentSize": "1692px",
      "wideSize": "1980px"
    },
    "typography": {
      "fluid": true,
      "fontSizes": [
        {
          "fluid": {
            "min": "1rem",
            "max": "3rem"
          },
          "name": "Large",
          "size": "3rem",
          "slug": "large"
        }
      ]
    },
    "useRootPaddingAwareAlignments": true
  },
  "styles": {
    "elements": {
      "h1": {
        "typography": {
          "fontSize": "var(--wp--preset--font-size-large)"
        }
      }
    }
  }
}

Potential solutions

theme.json variable detection and lookup

In typography.php wp_get_typography_value_and_unit() after the is_numeric() check (link to source here):

  1. Check the string ($raw_value) for the text "var(--wp--)"
  2. If matched, parse the variable contents by use of explode() or regex part matching.
  3. Attempt to locate the variable's value in the wp_get_global_settings() array.

A similar method could be used to handle ref entries, as well (mentioned in #53525)

Pitfalls

Could be expensive, performance-wise, to traverse the entire theme.json checking for a match (possibly multiple times)

Documentation and/or json schema update

If we can't implement this, we should note it in the layout setting's documentation and the schema description for the field. It may be appropriate to do this anywhere in which a value is expected in a specific format literally/directly by the style engine PHP.

Environment info and related issues

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

Related issue

Suggested tags

t-hamano commented 1 year ago

Hi @ouw-jvt,

This issue does not seem to reproduce in WordPress 6.4 Beta, and with the latest Gutenberg plugin.

Is it possible to use something like the Beta Tester plugin to check if this problem occurs even with the latest WordPress core?

ramonjd commented 1 year ago

Thanks for creating this issue.

What I think is happening is, since Wordpress 6.3.x, specifically https://github.com/WordPress/gutenberg/pull/53551, the typography block supports will fallback to $default_maximum_viewport_width if settings.layout.wideSize in theme.json cannot be correctly parsed to retrieve a valid value + unit, e.g., 12px or 5.4rem etc.

The fallback value is set as 1600px. Because this kicks in, we're seeing valid clamp() values despite the CSS vars. Why do we need the actual value? Because we need numerical values to calculate the correct clamp() values.

The workaround for now could be to hardcode your values into maxViewportWidth, e.g., 

        "typography": {
            "fluid": {
                "maxViewportWidth": "1980px",
            },
                  }

This was introduced in Wordpress 6.4 in https://github.com/WordPress/gutenberg/pull/53081 (it's been in the plugin since 16.5)

So CSS vars are not resolved. This is equally true of "ref" values, as mentioned in https://github.com/WordPress/gutenberg/issues/53525. Exploring "ref" resolution would be my preference in the first instance to make things consistent with how other styles values work in theme.json. As you say, it would be important to measure before and after performance.

Another idea might be to introduce filters so theme developers could use their own clamp formula using incoming values. I haven't explored that idea fully yet, but all ideas are welcome 😄

If we can't implement this, we should note it in the layout setting's documentation and the schema description for the field. It may be appropriate to do this anywhere in which a value is expected in a specific format literally/directly by the style engine PHP.

This is a very good suggestion, thank you! I can update the docs with the current reality as well as pragmatic workaround.

ramonjd commented 1 year ago

I can update the docs with the current reality as well as pragmatic workaround.

Actually I don't have access :) It looks like the docs are getting an overhaul anyway: https://github.com/WordPress/Documentation-Issue-Tracker/issues?q=is%3Aissue+is%3Aopen+theme.json

In the meantime, the most trustworthy reference is the living theme.json schema, which should always reflect the latest changes in Gutenberg:

https://github.com/WordPress/gutenberg/blob/8ef6e404f1de2ca22e3921c8e454e4335c843143/schemas/json/theme.json#L286-L286

ouw-jvt commented 1 year ago

@ramonjd Thanks for the replies! Documentation and updates to the json schema description denoting it does not accept references seems sufficient for me. Good to know this has been partially addressed in newer Gutenberg packages, too.

I'll see if I can get involved in that documentation rewrite!

github-actions[bot] commented 1 year ago

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.