-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Address console warning for Image block inside Column block #15239
Address console warning for Image block inside Column block #15239
Conversation
There's a console warning: Warning: A component is changing an uncontrolled input of type number to be controlled. I'm not sure this is the best approach, or if it'll have side-effects. But this is modeled after a few fixes for similar warnings, like WordPress#5443
As @jorgefilipecosta mentioned, this might help in cases where the warning still appears: A component is changing an uncontrolled input of type number to be controlled. Input elements should not switch from uncontrolled to controlled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component.
As Riad suggested, 0 is different from undefined, and it's a little strange to force values to 0 instead of undefined.
I just tested the following and it seems to work: diff --git packages/block-library/src/image/edit.js packages/block-library/src/image/edit.js
index 91eeb5a40..645248bc7 100644
--- packages/block-library/src/image/edit.js
+++ packages/block-library/src/image/edit.js
@@ -482,7 +482,7 @@ class ImageEdit extends Component {
type="number"
className="block-library-image__dimensions__width"
label={ __( 'Width' ) }
- value={ width !== undefined ? width : imageWidth }
+ value={ width || imageWidth || '' }
min={ 1 }
onChange={ this.updateWidth }
/>
@@ -490,7 +490,7 @@ class ImageEdit extends Component {
type="number"
className="block-library-image__dimensions__height"
label={ __( 'Height' ) }
- value={ height !== undefined ? height : imageHeight }
+ value={ height || imageHeight || '' }
min={ 1 }
onChange={ this.updateHeight }
/>
|
Thanks, @swissspidy! Feel free to push that to this branch if possible. Otherwise, I'll push it and credit you. |
@kienstra If that change works for you, feel free to #shipit 🙂 |
This uses the || operator entirely, instead of a ternary. Props @swisspidy
Thanks, @swissspidy! I just committed your change. Though it looks like the build failed, with a failed E2E test. I'm looking at that. |
Would you be able to restart the build, to see if it'll pass? Thanks! |
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.
LGTM, Thanks for the fix
Description
There is a console warning that sometimes appears when adding an Image block inside a Column block.
We also saw this with the Image block inside custom block: ampproject/amp-wp#2194 (comment)
This PR usually prevent this error locally. But the warning still appears sometimes with it.
I'm not sure if it's the right approach, or if it has side-effects.
This is modeled after a few fixes for similar warnings, like #5443.
Steps To Reproduce:
How has this been tested?
Screenshots
Before PR:
Types of changes
Bug fix: This sets a default value of
0
forwidth
andheight
.Checklist: