Input Field Block: Use useblockProps hook in save function
#56507
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
t-hamano
commented
Nov 24, 2023
| <InspectorControls __experimentalGroup="advanced"> | ||
| <InspectorControls group="advanced"> |
There was a problem hiding this comment.
This is a small fix that updates a deprecated prop.
t-hamano
commented
Nov 24, 2023
Comment on lines
16
to
29
| const getNameFromLabelV1 = ( content ) => { | ||
| const dummyElement = document.createElement( 'div' ); | ||
| dummyElement.innerHTML = content; | ||
| // Get the slug. | ||
| return ( | ||
| removeAccents( dummyElement.innerText ) | ||
| // Convert anything that's not a letter or number to a hyphen. | ||
| .replace( /[^\p{L}\p{N}]+/gu, '-' ) | ||
| // Convert to lowercase | ||
| .toLowerCase() | ||
| // Remove any remaining leading or trailing hyphens. | ||
| .replace( /(^-+)|(-+$)/g, '' ) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
This is copied from the getNameFromLabel() function present in save.js and used for version 1 deprecation. This is because I think deprecations may not be handled correctly if the original function is updated and the output markup changes.
|
Size Change: -14.2 kB (-1%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
t-hamano
commented
Nov 24, 2023
| }, | ||
| }, | ||
| supports: { | ||
| className: false, |
There was a problem hiding this comment.
className support wasn't present in the v1 block, but deprecation didn't work properly unless I explicitly added it. This is probably because the v1 block did not use a hook in the save() function, meaning the block's classname itself was not output.
t-hamano
commented
Nov 24, 2023
Comment on lines
1
to
6
| <!-- wp:form-input --> | ||
| <label class="wp-block-form-input__label"> | ||
| <span class="wp-block-form-input__label-content">Label</span> | ||
| <input class="wp-block-form-input__input" type="text" name="label" aria-required="false"/> | ||
| </label> | ||
| <!-- /wp:form-input --> |
There was a problem hiding this comment.
I expected new markup to be generated here, but strangely the markup hasn't changed. This may be related to the fact that the hook was not used in the save function.
There was a problem hiding this comment.
Your expectation was right, this would appear to indicate that the deprecation migration isn't running. At least for the fixture tests.
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
t-hamano
added
[Type] Bug
There was a problem hiding this comment.
Thanks for working on this fix @t-hamano 👍
If the saved markup is being changed we also need to update the default fixture for the block to the latest state as well as introducing the new deprecated fixture. At the moment the original fixture is only covering the new deprecated version.
I would have expected this primary fixture to fail due to similar reasons as our expectation that the new deprecation's serialized HTML should have the wrapping div.
Something isn't quite right and I can't quite put my finger on it at the moment.
From my testing, the deprecation works in the editor but not in the fixture test. Could it be related to the Gutenberg experiment the form field and inputs are behind?
|
Than for the review, @aaronrobertshaw!
I was able to get a clue on this point. In the fixture files related to the form block, the value of the
In the Before this PR, we might want to ensure that form-related blocks are also registered in fixture tests and update existing fixture files correctly. I'll do some more research to see if I can resolve this issue. |
|
Another consideration might be whether to support deprecation of blocks that are experimental and unavailable unless we opt-in. For example, we might need to support deprecation in the block editor as well as stable core blocks, but blocks that are experimental and require opt-in may not need to be added to fixture tests. |
This was referenced Dec 1, 2023
|
Thanks to #56719, fixture files for form-related blocks are now generated correctly. The fixture file should now be generated correctly with this PR. |
t-hamano
commented
Dec 5, 2023
|
Flaky tests detected in b0d97b6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7178832141
|
aaronrobertshaw
approved these changes
Dec 12, 2023
There was a problem hiding this comment.
Thanks for picking this one back up @t-hamano 👍
It's looking pretty good now. There's one minor nit we'll need to tweak before merging but I'm happy to approve all the same.
✅ Tests as advertised in the editor
✅ Block successfully migrates to the latest version
✅ Fixture tests all pass
✅ getNameFromLabelV1 matches original version
✅ Deprecation's attributes and supports match original version with the exception of className as explained.
Co-authored-by: Aaron Robertshaw <[email protected]>
|
Thanks for the review, @aaronrobertshaw, @aristath! |
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.
Fixes #56458
Follow up #44214
What?
This PR adds the missing wrapper div and
blockPropsin the save function of the Input Filed (Form Input) block. This allows block support styles such as margins, borders, etc. to work on the front end.Why?
Without this hook, block support styles will not be applied. Also, the markup in the editor and frontend doesn't match.
How?
At the same time as adding a hook to the save function, I also added a block deprecation.
Testing Instructions