LineHeightControl: Fix application of zero values in editor#48917
Merged
aaronrobertshaw merged 3 commits intotrunkfrom May 4, 2023
Merged
LineHeightControl: Fix application of zero values in editor#48917aaronrobertshaw merged 3 commits intotrunkfrom
aaronrobertshaw merged 3 commits intotrunkfrom
Conversation
aaronrobertshaw
added
[Type] Bug
|
Size Change: +15 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
talldan
approved these changes
Apr 6, 2023
Contributor
talldan
left a comment
talldan
left a comment
There was a problem hiding this comment.
LGTM, the changes all make sense.
Left some comments, but they're mostly optional, though it'd be good to fix the types in the doc block before merging.
youknowriad
reviewed
Apr 13, 2023
youknowriad
reviewed
Apr 14, 2023
packages/block-editor/src/components/line-height-control/index.js
Outdated
Show resolved
Hide resolved
aaronrobertshaw
force-pushed
the
fix/line-height-control-with-zero-values
branch
from
9bbb0f8 to
e55615b
Compare
aaronrobertshaw
commented
Apr 18, 2023
Contributor
Author
aaronrobertshaw
left a comment
aaronrobertshaw
left a comment
There was a problem hiding this comment.
I've updated the approach in light of @youknowriad's feedback advising as to how we can avoid the need to update cleanEmptyObject.
The latest approach in this PR works for me against trunk's cleanEmptyObject or the version in #49750.
I've left a few inline comments as to the changes that remain but the PR is now more trivial.
| ); | ||
| }; | ||
| const hasLineHeight = () => !! value?.typography?.lineHeight; | ||
| const hasLineHeight = () => value?.typography?.lineHeight !== undefined; |
Contributor
Author
There was a problem hiding this comment.
I've kept this tweak to the line height value detection so it can correctly check any old numeric line-height values that might be 0 in existing blocks/styles.
Comment on lines
+88
to
+91
| if ( nextValue === '' ) { | ||
| onChange(); | ||
| return; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This is needed if we land #49750 which changes cleanEmptyObject to keep anything that !== undefined.
It also allows the following onChange calls to simply convert the value to a string improving zero value handling.
Contributor
Author
|
Now #49750 has been merged, I've re-tested this PR after rebasing on trunk and it still works well. I'll get this merged. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes: #48768
What?
Prevents LineHeightControl resetting and clearing values when
0line height selected.Why?
While zero line height might be an uncommon use case, as long as we allow zero as the minimum value for the control it should be honoured and remain when continuing to drag down, or use the down arrow key.
How?
Boolean()) or #49750 (!== undefined). The control'shandleOnChangewill then ensure any defined line-height value is passed to it's onChange handler as a string.Testing Instructions
Screenshots or screencast
Screen.Recording.2023-03-08.at.7.14.49.pm.mp4
Screen.Recording.2023-03-08.at.7.02.54.pm.mp4