Test editor styles perf #60940
Closed
Test editor styles perf #60940
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
|
Size Change: +130 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
ellatrix
reviewed
Apr 23, 2024
| const aIndex = clientIdMap[ a[ 1 ].clientId ] ?? -1; | ||
| const bIndex = clientIdMap[ b[ 1 ].clientId ] ?? -1; | ||
| return aIndex - bIndex; | ||
| } ); |
There was a problem hiding this comment.
Do overrides already have a clientId added to them?
There was a problem hiding this comment.
(I don't see the code in the PR that adds that clientId to the override object)
ellatrix
reviewed
Apr 23, 2024
| const { overrides, clientIds } = useSelect( | ||
| ( select ) => ( { | ||
| overrides: unlock( select( blockEditorStore ) ).getStyleOverrides(), | ||
| clientIds: select( blockEditorStore ).getClientIdsWithDescendants(), |
There was a problem hiding this comment.
Does this work for controlled blocks as well?
There was a problem hiding this comment.
What if we change getStyleOverrides to return the sorted overrides? 🤔
|
For me, this is good, if it solves your style dependency issues |
|
@ellatrix Just for some context, this was me pulling some of the code out of Block Styles: Extend block style variations as mechanism for achieving section styling to see what the performance impact of this particular bit of code is. That PR is causing some performance issues, and it's not clear where right now, but I don't think it's this bit of code. |
|
I did notice though that this bit of code now triggers some updates to this component whenever a block is added/moved/removed, not sure there's much impact, but having said that I don't think there's a performance test for block insertion. |
I think it's anyway good to do this separately as a preparatory PR? That PR is huge. |
|
I guess these changes were forgotten here? |
Agreed. FWIW until all the moving parts to achieve the extended block style variations feature were discovered through #57908. We weren't in a position to split this out as the order issues with style overrides were one of the last hurdles found. Now #57908 has stabilized, there's no reason not to carve out what we can into PRs like this.
Yep. the addition of |
|
I'll close this as @aaronrobertshaw indicated that he'll make some new PRs with proper descriptions 👍 |
|
I've put up an alternate PR that includes the addition of |
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?
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast