-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add: Support for other units and CSS properties in Font Size presets #26475
Add: Support for other units and CSS properties in Font Size presets #26475
Conversation
Size Change: +70 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
@jorgefilipecosta Haiii! Wow, thanks for working on this. I took it for a spin locally. There are some UI issues when creating super mega sized fonts, in particular with the preview in the font-size picker: However, it's not an issue caused by your update. In the meantime.. maybe we can add a CSS style rule for the FontSize picker to ✅ CSS Props ValidationYour update touches upon something that I've been attempting to streamline recently.. and that is Input based controls being able to handle any valid CSS property/value, not just units. Your (See the "CSS Prop Validation" section) The current FontSizePicker is able to tap into just the |
Hi @ItsJonQ,
Yes, I guess we can try a CSS min function to try to have a maximum value for font-size. But I guess these tries should not be done in this PR as we already had this problem e.g: if we set font size for a big value like 90px.
Yah these updates basically allows anything and if we can parse the value we show it otherwise we show nothing in the custom value. It seems like a good first step towards allowing everything that CSS has to offer. Having nonpixel values and calcs editable by the user would be the next step and your components looks good. Would you be able to propose a PR after this one is merged with the new component for size shown here ItsJonQ/g2#66 that allows the user to control any value and select units etc? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
Tested with an FSE theme that already uses non-px values and it works as advertised. Also tested with FSE themes that use integer values and nothing breaks.
There are some UI issues when creating super mega sized fonts, in particular with the preview in the font-size picker
There's already another ticket for that, see #25862 👍
I think this one is good to merge! ❤️
…n the font size presets.
16864c2
to
1f3514e
Compare
Hi @jorgefilipecosta I'm experiencing some errors when using this code in the site editor. This is what I see: Although the UI doesn't reflect the change, the data can be saved to the CPT but with a wrong format: {
"global": {
"styles": {
"typography": {
"fontSize": "60pxpx"
}
}
}
} I'm using https://github.com/nosolosw/global-styles-theme/ as a test demo. |
Labelling as an PR to backport for 5.6.2 as per #28738 (comment) |
* Backport #26475 for the 5.6 branch. * Backport #28604. * Revert "Backport #28604." This reverts commit 32784b6. * Backport #26583 to the 5.6 branch. * Commit lock file updates. * Committing lock file differences after updating `browserslist`. * Pin the version of NodeJS in the Compressed Size workflow. * Memoize getEntityRecords to prevent infinite re-renders (#26447) * Fix issue where gallery block requests all attachments when empty (#28621) * Return early from building attachments for galleries * Improve code clarity * Fix link control styles to prevent scrollbar (#27777) * Update package-lock * Update package-lock again Co-authored-by: Jonathan Desrosiers <desrosj@users.noreply.github.com> Co-authored-by: Kai Hao <kevin830726@gmail.com> Co-authored-by: Daniel Richards <daniel.richards@automattic.com> Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
This PR makes sure themes can use font size values on font size presets that are not in pixels.
Some examples are "2rem", "1em", or "clamp(12px, 5vw, 100px)". Any valid CSS property for font size is accepted on the preset.
The font size preset was the only preset whose value was not a valid CSS property e.g: it used "13" as a value and "13" is an invalid property while "13px" is a valid property. This causes problems on #26319 because if we try to reference a variable containing the font size preset the variable is not valid. So this PR includes functionality that if the font size preset is a number automatically adds the pixel unit e.g: "13" -> "13px". This is only done on existing font size presets added with add_theme_supports, for the font sizes passed via theme.json we assume they pass valid CSS properties.
When a font size preset is selected that is not in pixels the font size component shows the custom field as empty and correctly shows the selected preset. I guess the UI of the component should be improved to deal with other units besides pixes and to allow a free form input version where the user can type something like "clamp(12px, 5vw, 100px)" or change a little bit the advanced preset. But than can be done in a follow up PR. cc: @ItsJonQ as someone that is involved in UI work and worked with unit selection before.
Description
How has this been tested?
I tested 2019 and 2020 themes and verified the font size presets were still applied and font size selection still worked as expected.
I tested the global styles theme with the following typography settings:
I verified all the presets worked as expected.