-
Notifications
You must be signed in to change notification settings - Fork 4.7k
FocalPointPicker: stop using UnitControl's deprecated unit prop
#39504
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
Conversation
|
Size Change: +7 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
b04e255 to
c345820
Compare
c345820 to
9810707
Compare
stokesman
left a comment
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 does the job.
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.
Is there something I don't know about join that makes it better than template literals?
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.
I prefer the join approach because it ignores undefined values:
[ 41, undefined ].join(''); // '41'
`${ 41 }${ undefined }`; // '41undefined'And so I've taken the habit of using this syntax, especially when in need of concatenating quantity and unit in the context of UnitControl
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.
Now I get it. Thanks! I'm pretty sure in this case the values cannot be undefined but I'm not sweating these joins staying put.
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.
I prefer the join approach because it ignores undefined values
Oh, nice! I'm sure I'll borrow that approach 😀
9810707 to
29a7c0f
Compare
andrewserong
left a comment
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 is testing nicely for me too, and works the same as on trunk!
…WordPress#39504) * `FocalPointPicker`: stop using `UnitControl`'s deprecated `unit` prop * CHANGELOG
What?
Related to #39503 , this PR refactors the
FocalPointPickercomponent to avoid using the deprecatedunitprop from theUnitControlcomponent.Why?
The
unitprop is marked as deprecated, the component's docs recommend passing the unit directly through thevaluepropHow?
Refactor the code to pass the unit directly through the
valuepropTesting Instructions