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

A path toward responsive, non-px font sizes #11671

Closed
chrisvanpatten opened this issue Nov 9, 2018 · 7 comments
Closed

A path toward responsive, non-px font sizes #11671

chrisvanpatten opened this issue Nov 9, 2018 · 7 comments
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Customization Issues related to Phase 2: Customization efforts [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. General Interface Parts of the UI which don't fall neatly under other labels. [Type] Discussion For issues that are high-level and not yet ready to implement.
Milestone

Comments

@chrisvanpatten
Copy link
Contributor

chrisvanpatten commented Nov 9, 2018

Due to the way font sizes are implemented, they're currently tied to a pixel value.

In the font size components, the font size object is intended to include a size property as a number value, which is the size in px. E.g. size: 16.

These sizes are required for two reasons:

  1. They're used to generate the style attribute in the font size picker and show the label at a specific size
  2. If the user inputs a font size in the custom input field, the font size picker falls back to any named font size matching that number

Further, because of #11200, in the editor font sizes are rendered with px values through inline style, because the stylesheet where we define the font size classes is less specific than our default editor styles.

I would argue that this implementation is flawed for a few reasons:

  1. It enforces an unnecessary link to pixels
    1. If your stylesheet displays font sizes with non-px values, you're still required to provide a pixel size value, because…
    2. It's not possible to accurately render your font size in the font size picker if you use non-px values
  2. If fonts are defined in CSS with non-px values, users can't actually display the pre-defined sizes in px. In other words if you define "Large" as size 22, but it's in your CSS as 2rem, but a user wants to display the font size as actually 22px, they can't. The font size picker will always reset an input of 22 to select the "Large" value. (See Font Size issue when using predefined sizes #8689, Font Size #11413)

#10687 was one attempt to solve this. It didn't solve all these problems, but did solve the main issues by…

  1. Adding an explicit "Custom" value, only showing the custom input when that value was selected
  2. Continuing to require a size parameter but only using it as an "approximate preview" size in the editor
  3. Removing the editor's attempts to convert custom sizes to named sizes — you could only get a named size if you explicitly selected it, and you could only get a custom size if you explicitly selected it

Ultimately #10687 was rejected for UI reasons.

This ticket is an attempt to ideate/brainstorm on how we could improve the UI, to truly separate and isolate custom and named sizes, and to minimise the enforced connection to "px" values.

@chrisvanpatten chrisvanpatten added this to the Future: 5.1 milestone Nov 9, 2018
@chrisvanpatten chrisvanpatten added Customization Issues related to Phase 2: Customization efforts General Interface Parts of the UI which don't fall neatly under other labels. Future [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. labels Nov 9, 2018
@chrisvanpatten
Copy link
Contributor Author

chrisvanpatten commented Nov 9, 2018

Ultimately I would like to see two outcomes:

  1. Stop trying to convert custom/numeric font sizes to named font sizes.
    • If a user is going to the deliberate effort to type a value (22, 16, whatever), that means the user explicitly desires that font size. We can't guarantee the theme will display the font at that numeric size within their font size classes, so using the numeric input should always set an inline style attribute. Every time, no exceptions.
  2. Users should have to explicitly choose to input custom sizes.
    • This relates to the first point, but ultimately users should have to explicitly choose to display the custom size field, via a dropdown option, button, toggle, or some other mechanism. Setting a custom size, with the inline style attribute, should be considered a last resort practice because inline style attribute limits the theme's ability to control the font size, and prevents the size from adapting as themes are changed. Hiding it behind a toggle, or making it an explicit dropdown option, makes it a more deliberate choice for users to consider.

@chrisvanpatten
Copy link
Contributor Author

chrisvanpatten commented Nov 9, 2018

EDIT: I might move the below into a new ticket. It relates to this issue, but is ultimately separate, and I don't want this issue to get too far off track.


I'd also like to see us do a little work to clarify the "default"/"normal" size. We currently have this size available in the size picker, mapped to 16px, however applying it really "unsets" the style (e.g. removes the className).

(So ultimately there are three classifications/types of font sizes: set via className, set via style attribute, and not set/whatever the CSS default is.)

I totally get why this is necessary, but I think we should standardise so that a "Default" size is always available in the font size picker, and make the px value that we use to display the size in the picker configurable. (As of right now, it's not really possible for a theme to define a default/unset size that doesn't correspond to a size of 16.)

In other words you'd have something like:

add_theme_support( 'editor-font-sizes', [ /* my font sizes */ ] );
add_theme_support( 'default-editor-font-preview-size', 16 );

This separation would ensure that themes always have a "unset" option available. In implementing #10687, figuring out how to handle the default was the trickiest part, and I think making it more explicit would be super helpful.

I think this is slightly less important than the other issues relating to custom sizes, but it would ultimately make it easier for users to set base font sizes to bigger or smaller values which is currently not possible.

@eyesofjeremy
Copy link

Has there been further discussion on moving away from pixel values? It's pretty surprising to me that the pixel values are baked in there as they are. Makes it impossible to map accurately to theme font sizes, if they aren't set in pixel values.

@tellthemachines tellthemachines added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Apr 23, 2020
@kjellr kjellr added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label May 20, 2020
@youknowriad youknowriad added [Type] Discussion For issues that are high-level and not yet ready to implement. and removed [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. labels Jun 4, 2020
@rastitkac
Copy link

Hi.
Is there a way to define these font-sizes responsively yet?
Given, that these are rendered inline in frontend, there should be a way to do something like:

add_theme_support('editor-font-sizes', [
    [
      'huge' => __('Huge'),
      // 'size' => '80',
      'sizes' => [
          '640' => '50', // key is breakpoint, value is size
          '768' => '68',
          '992' => '80'
        ],
      'slug' => 'huge'
    ]
]);

and then some js magic that would rewrite the style set inline.
Otherwise we must dodge using this and use blockStyles.

@youknowriad
Copy link
Contributor

I think @aristath did a lot of work on this, are we ready to close this issue now?

@aristath
Copy link
Member

I think @aristath did a lot of work on this, are we ready to close this issue now?

Yes, this has been addressed in multiple PRs and can be closed 👍

@kraftner
Copy link

@aristath If you find the time - could you be so kind and reference those PRs so this issue doesn't become a dead end? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Customization Issues related to Phase 2: Customization efforts [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. General Interface Parts of the UI which don't fall neatly under other labels. [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

No branches or pull requests

8 participants