Pattern Overrides: Memoize controls component #63189
Closed
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
ellatrix
reviewed
Jul 5, 2024
| { props.isSelected && isSupportedBlock && ( | ||
| <ControlsWithStoreSubscription { ...props } /> | ||
| <MemoizedControlsWithStoreSubscription |
There was a problem hiding this comment.
Can't we use createBlockEditFilter like the other hooks?
There was a problem hiding this comment.
This package is different, but I can copy the same logic here. This is still draft PR :)
There was a problem hiding this comment.
Oh, maybe we could add a private API for now?
There was a problem hiding this comment.
This is only the editor.BlockEdit filter in the editor package. Does it make sense to abstract it now?
There was a problem hiding this comment.
I think it does at some point anyway, we need to start thinking about the block supports API.
There was a problem hiding this comment.
In my case, to actually start building it 😅 Hopefully, I can share something before 6.7 betas.
There was a problem hiding this comment.
Block bindings panels should use it too, in case it helps.
There was a problem hiding this comment.
@cbravobernal, which component exactly are we talking about?
There was a problem hiding this comment.
I'm still working on it, and will move it from block-editor to editor package. But is this one:
#62880
|
Size Change: +55 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
I missed a couple of updates in the child components. This is the reason for the e2e test failures. I also noticed that the |
Mamaduka
force-pushed
the
optimize/pattern-overrides-hook
branch
from
8197d15
to
cc989c1
Compare
Mamaduka
commented
Jul 9, 2024
Comment on lines
+114
to
+118
| hasUnsupportedAttributes={ | ||
| props.name === 'core/image' && | ||
| ( !! props.attributes.caption?.length || | ||
| !! props.attributes.href?.length ) | ||
| } |
There was a problem hiding this comment.
I had to lift this condition to avoid passing the whole attribute object.
|
@cbravobernal, the UI implementation from #62880 will stay in the In that case, as @kevin940726 noted, there's probably no need for optimization here - #63146 (comment). |
|
Correct, we can close this draft PR and its issue. |
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.
What?
See #63146.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast