Don't remount the block when the 'templateLock' is set to 'contentOnly'#50292
Merged
Don't remount the block when the 'templateLock' is set to 'contentOnly'#50292
Conversation
Mamaduka
added
[Type] Bug
Mamaduka
commented
May 3, 2023
| className={ classnames( | ||
| props.className, | ||
| isEditingAsBlocks && | ||
| 'is-content-locked-editing-as-blocks' |
Member
Author
There was a problem hiding this comment.
This class isn't used in the editor. Probably a leftover from the original PR.
Member
There was a problem hiding this comment.
There is, however, a .is-content-locked-temporarily-editing-as-blocks class that has styles. Can you check whether it maybe should apply to this element? It could be a bug that it's not applied.
Member
Author
There was a problem hiding this comment.
Correct. The .is-content-locked-temporarily-editing-as-blocks use used to apply "Spotlight" styling to the block.
I double-checked this with the original PR (#43037), and the "edit mode" works as before.
Mamaduka
commented
May 3, 2023
| stopEditingAsBlock(); | ||
| } | ||
| }, [ isBlockOrDescendantSelected ] ); | ||
| }, [ isBlockOrDescendantSelected, stopEditingAsBlock ] ); |
Member
Author
There was a problem hiding this comment.
Fixes the ESLin warning. The stopEditingAsBlock callback is memoized.
Member
There was a problem hiding this comment.
The stopEditingAsBlock callback has a lot of dependencies, but they are mostly useDispatch return values which should never change. Then there's clientId, also constant, and a constant focusModeToRevert ref. I wonder if the ref really needs to be a dependency, in practice it has no effect and I think the ESLint rule will forgive an undeclared dependency that's a return value of useRef and is accessed as .current.
Member
Author
There was a problem hiding this comment.
Unfortunately, we have to declare callbacks returned by useDispatch to satisfy the linter, but as you said, they should have a stable reference.
I'll remove focusModeToRevert from dependencies - ca0dd6b.
Mamaduka
requested review from
jsnajdr,
ntsekouras and
tyxla
and removed request for
ellatrix
|
Size Change: -19 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
jsnajdr
approved these changes
May 3, 2023
| stopEditingAsBlock(); | ||
| } | ||
| }, [ isBlockOrDescendantSelected ] ); | ||
| }, [ isBlockOrDescendantSelected, stopEditingAsBlock ] ); |
Member
There was a problem hiding this comment.
The stopEditingAsBlock callback has a lot of dependencies, but they are mostly useDispatch return values which should never change. Then there's clientId, also constant, and a constant focusModeToRevert ref. I wonder if the ref really needs to be a dependency, in practice it has no effect and I think the ESLint rule will forgive an undeclared dependency that's a return value of useRef and is accessed as .current.
| className={ classnames( | ||
| props.className, | ||
| isEditingAsBlocks && | ||
| 'is-content-locked-editing-as-blocks' |
Member
There was a problem hiding this comment.
There is, however, a .is-content-locked-temporarily-editing-as-blocks class that has styles. Can you check whether it maybe should apply to this element? It could be a bug that it's not applied.
This was referenced Jan 19, 2024
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.
What?
Fixes #50163.
PR prevents remounting container
BlockEditcomponents when thetemplateLockvalue iscontentOnly.Why?
This is similar to #48209.
The
templateLockand its derived value,isContentLocked, is initiallyfalse; after synchronizing the block list settings (updateBlockListSettings), it becomescontentOnly. Like the issue above, this causes<BlockEdit />to be recreated from scratch.How?
Set
key="edit"on returnedBlockEditcomponent from the filter.Testing Instructions
Use the reproduction code from https://gist.github.com/louwie17/20328e1d2c723e34e9cb979dd156fe47, or
templaceLockattribute tocontentOnly.edit mounting.