Forms: remove onSplit in favor of splitting feature #38246
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
bot
added
[Block] Contact Form
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped. |
monsieur-z
force-pushed
the
fix/form-onSplit-deprecation
branch
2 times, most recently
from
8fcc86e
to
349afd5
Compare
monsieur-z
force-pushed
the
fix/form-onSplit-deprecation
branch
from
349afd5
to
1148386
Compare
| @@ -243,6 +243,7 @@ const OptionFieldDefaults = { | |||
| supports: { | |||
| reusable: false, | |||
| html: false, | |||
| splitting: true, | |||
There was a problem hiding this comment.
This has no impact on Gutenberg versions that don't support it.
| @@ -62,7 +62,7 @@ function JetpackFieldMultiple( props ) { | |||
| <InnerBlocks | |||
| allowedBlocks={ ALLOWED_BLOCKS } | |||
| template={ [ [ `jetpack/field-option-${ type }`, {} ] ] } | |||
| templateInsertUpdatesSelection={ false } | |||
| templateInsertUpdatesSelection={ true } | |||
There was a problem hiding this comment.
Ensure option input has focus initially, for all versions.
| @@ -41,21 +48,46 @@ export const JetpackFieldOptionEdit = props => { | |||
| removeBlock( clientId ); | |||
| }; | |||
|
|
|||
| const onFocus = () => { | |||
| // TODO: move cursor to end | |||
There was a problem hiding this comment.
When you delete an option, the previous one gets the focus, but the caret stays at the start. I've tried to move it at the end and select the entire text but couldn't eventually make it work, though both approaches are documented online numerous times. I don't know what's blocking here. Even requestAnimationFrame and setTimeout calls didn't work.
There was a problem hiding this comment.
Note: after removing an option with the delete key, the previous item gets the focus. but the caret doesn't move to the end. I haven't been able to fix this yet, but it doesn't break functionality, and we can ship this PR as is.
Agree - we can create a new issue for the caret issue in order to get this out as soon as possible. Seems to be an issue when just selecting an option in a dropdown too in the editor, but a second click moves the caret to the intended location.
The functionality apart from that works well, also if not using the updated Gutenberg version.
Fixes #38031
Proposed changes:
The
onSplitprop of theRichTextcomponent is no longer supported in Gutenberg. The last remaining component that uses this property in theformspackage isJetpackFieldOptionEdit, which is used by the Multiple Choice (Checkbox) and Single Choice (Radio) fields.This PR replaces the
onSplitprop with thesplittingsupport feature. This comment thread explains the change in more detail.Other information:
Jetpack product discussion
n/a
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
Regression testing
Enterkey and that you can delete items with the keyboardUpdate testing
Note: after removing an option with the
deletekey, the previous item gets the focus. but the caret doesn't move to the end. I haven't been able to fix this yet, but it doesn't break functionality, and we can ship this PR as is.