-
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
Fixes the default fluid value on the UI based on the global typography fluid value. #64803
Fixes the default fluid value on the UI based on the global typography fluid value. #64803
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: -3 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
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.
// Whether fluid is true or an object, set it to true, otherwise false. | ||
const isFluid = !! fontSize.fluid ?? false; | ||
// Whether the font size is fluid. If not defined, use the global fluid value of the theme. | ||
const isFluid = |
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.
Sorry for the late review. I noticed that it was already merged, but I'll comment just in case.
settings.typography.fluid
is not necessarily a boolean value, but can be an object. That is, if the following theme.json is defined, isFluid
will be an object:
{
"settings": {
"typography": {
"fluid": {
"minFontSize": "16px"
},
"fontSizes": [
{
"name": "Theme font size 1",
"size": "30px",
"slug": "theme-1"
}
]
}
},
"version": 3
}
We might want to change it to the following to ensure that isFluid
is always a boolean value:
fontSize.fluid !== undefined ? !! fontSize.fluid : !! globalFluid;
However, the current implementation works fine for now.
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.
Good point, thanks for catching.
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.
…fluid value (WordPress#64803) Co-authored-by: matiasbenedetto <[email protected]> Co-authored-by: ramonjd <[email protected]>
What?
Fixes the default fluid value on the UI based on the global typography fluid value.
Why?
This PR makes the UI consistent with the default behavior of the fluid typography settings. If the global fluid setting
settings.typography.fluid
is defined astrue,
all the font size presets are rendered as fluid despite not being set asfluid
explicitly. This PR reflects that in the UI.Fixes: #64765
How?
Considers the global fluid typography option to render and display the control correctly.
Testing Instructions
fluid
control should be rendered as true, and without this PR, it should render (incorrectly) as false.Screenshots or screencast
Despite the
fluid
option not being explicitly defined in the font size preset, the UI will be checked because the behavior will be true because the global option is enabled.