Add defaultFontSizes option to theme.json #56661
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ajlende
added
[Type] Feature
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ lib/theme.json |
|
Size Change: +222 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
ajlende
force-pushed
the
add/default-font-sizes-option
branch
from
ab1cb4a
to
b7fe8e4
Compare
|
Flaky tests detected in 7ea8c73. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7135196262
|
ajlende
force-pushed
the
add/default-font-sizes-option
branch
from
b7fe8e4
to
6cf601a
Compare
ajlende
changed the title
ajlende
force-pushed
the
add/default-font-sizes-option
branch
3 times, most recently
from
1099861
to
6673c25
Compare
ajlende
requested review from
spacedmonkey,
ellatrix,
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
ajlende
force-pushed
the
add/default-font-sizes-option
branch
2 times, most recently
from
c52dc43
to
aa2e437
Compare
scruffian
reviewed
Dec 7, 2023
ajlende
force-pushed
the
add/default-font-sizes-option
branch
from
aa2e437
to
7ea8c73
Compare
This looks like it exists on trunk too. Generating the global stylesheet doesn't filter out duplicates, so the later CSS overrides the earlier. {
"name": "Display xl2",
"slug": "Display-xl",
"size": "296px"
},
{
"name": "Display xl",
"slug": "Display-xl",
"size": "96px"
},
There's a second bug on trunk. You should be able to override default values because {
"name": "Override Small",
"slug": "small",
"size": "26px"
},
{
"name": "Override Medium",
"slug": "medium",
"size": "40px"
},
{
"name": "Override Large",
"slug": "large",
"size": "72px"
},It's an issue with global styles, but fixing global styles seems like a separate task, so I recreated the second bug in this PR (and fixed the first bug where overriding yourself shows only the value that will be applied in CSS). At least that way, the experience isn't any worse than it was before. The thing you select should always match the thing in the CSS even if you still can't override the defaults. The problem is that I had to write some hacky code to match the UI with the buggy global styles. This needs more testing, and maybe some cleanup. But I'm out for a few days, so if someone wants to pick it up and bring it to the finish line while I'm away, you're more than welcome to do so. Getting the global styles |
|
Here's what I found when investigating the duplicate font sizes: #57692 |
ajlende
added a commit
to ajlende/wordpress-develop
that referenced
this pull request
Jan 18, 2024
9 tasks
|
Thank you for working on this. Once the code is merged into WP Core please could you
Thank you in advance 🙇 |
justintadlock
added a commit
to x3p0-dev/x3p0-ideas
that referenced
this pull request
Jan 25, 2024
|
I have been advised away from Github, that merging the backport is delayed here due to some concerns regarding compatibility with existing Themes. @ajlende will elaborate when he has more details. |
|
This PR changed the default value for The value for So both the ordering bug and the legacy behavior both need to be added to fix this issue which is why there are delays. |
|
Thank you for the update 👍 Let either of the Editor Tech Leads know if you need any support to land the fixes. |
cbravobernal
added a commit
that referenced
this pull request
Jan 30, 2024
This reverts commit 940f0fe.
cbravobernal
added a commit
that referenced
this pull request
Jan 30, 2024
ajlende
added a commit
that referenced
this pull request
Jan 30, 2024
* Add defaultFontSizes option for theme.json * Fix defaultFontSizes causing errors when missing * Simplify useMergedSettings * Simplify more * Remove the memo because it's probably fine * Try ignoring how overrides are handled in the dropdown * Minor performance improvement using reverse instead of toReversed * Link to a new issue with a more complete description * Refactor hack into its own funtion
cbravobernal
added a commit
that referenced
this pull request
Jan 30, 2024
youknowriad
pushed a commit
that referenced
this pull request
Jan 31, 2024
ajlende
added a commit
that referenced
this pull request
Feb 1, 2024
* Add defaultFontSizes option for theme.json * Fix defaultFontSizes causing errors when missing * Simplify useMergedSettings * Simplify more * Remove the memo because it's probably fine * Try ignoring how overrides are handled in the dropdown * Minor performance improvement using reverse instead of toReversed * Link to a new issue with a more complete description * Refactor hack into its own funtion
ajlende
added a commit
that referenced
this pull request
Feb 6, 2024
* Add defaultFontSizes option for theme.json * Fix defaultFontSizes causing errors when missing * Simplify useMergedSettings * Simplify more * Remove the memo because it's probably fine * Try ignoring how overrides are handled in the dropdown * Minor performance improvement using reverse instead of toReversed * Link to a new issue with a more complete description * Refactor hack into its own funtion
ajlende
added a commit
that referenced
this pull request
Feb 7, 2024
* Add defaultFontSizes option for theme.json * Fix defaultFontSizes causing errors when missing * Simplify useMergedSettings * Simplify more * Remove the memo because it's probably fine * Try ignoring how overrides are handled in the dropdown * Minor performance improvement using reverse instead of toReversed * Link to a new issue with a more complete description * Refactor hack into its own funtion
ajlende
added a commit
that referenced
this pull request
Feb 9, 2024
* Add defaultFontSizes option for theme.json * Fix defaultFontSizes causing errors when missing * Simplify useMergedSettings * Simplify more * Remove the memo because it's probably fine * Try ignoring how overrides are handled in the dropdown * Minor performance improvement using reverse instead of toReversed * Link to a new issue with a more complete description * Refactor hack into its own funtion
youknowriad
removed
Needs PHP backport
|
Removing the labels as this has been reverted. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds a
defaultFontSizesoption to disable showing the default font sizes.When
defaultFontSizes === falsethe options are removed from the UI.By default
defaultFontSizesistrue.Why?
Resolves #52200
How?
TypographyPanelto remove defaults when the option was set to falseTesting Instructions
Test the new
defaultFontSizes.settings.typography.defaultFontSizes = falseto your theme.json file.Test that duplicate font sizes are removed.
settings.typography.fontSizesto include duplicate slugs.Testing Instructions for Keyboard
N/A
Screenshots or screencast
Tested with the theme.json from the example in #52200 and
defaultFontSizesset tofalse.