Block: Avoid rerendering when the previous/next block updates #5025
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
|
It looks like in all places we use |
|
Nice idea! I saw these light up in React Dev Tools Highlight Updates as well 😄 Could you give the branch a rebase? Will take a look later this morning. |
youknowriad
force-pushed
the
update/optimize-previous-next-block
branch
from
b19542a
to
124ea5d
Compare
|
Rebased, thanks @aduth |
aduth
approved these changes
Feb 13, 2018
| @@ -57,8 +57,8 @@ import { | |||
| isMultiSelecting, | |||
| getBlockIndex, | |||
| getEditedPostAttribute, | |||
| getNextBlock, | |||
| getPreviousBlock, | |||
| getNextBlockUid, | |||
There was a problem hiding this comment.
We need to come to a convention on camel-case for abbreviations. Elsewhere I've used e.g. rootUID
Related: #2511
There was a problem hiding this comment.
Right! I thought we kind of agreed on always camel casing abbreviations at some point but happy to change anyway.
aduth
added a commit
that referenced
this pull request
Mar 9, 2018
Regression of #5025, where prop was changed from `previousBlock` to `previousBlockUid`, but neglected to update the instance of the prop reference in keydown handler
aduth
added a commit
that referenced
this pull request
Mar 9, 2018
* Block List: Fix select previous on backspace behavior Regression of #5025, where prop was changed from `previousBlock` to `previousBlockUid`, but neglected to update the instance of the prop reference in keydown handler * Utils: Improve spec compliancy of text field * Block: Prefer focus text field on selection, fallback to wrapper * Writing Flow: Refactor getClosestTabbable to support non-tabbable target * Block List: Extract typing monitor to separate component * Block List: Don't deselect block on escape press Escape doesn't clear focus, so causes problems that block is not selected but retains focus (since isSelected state is synced with focus) * Block List: Fix delete or insert after focused block wrapper node * Rich Text: Ensure format toolbar manages its own dismissal Previously only closed on esc when editing link, not adding new link TODO: Consolidate editing state
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.
the
getPreviousBlockandgetNextBlockselectors are causing the block to rerender if its previous/next block changes. This is unnecessary because we only need these blocks for "merge" behaviors.This PR updates the merge behavior to rely on UIDs instead of the whole object.
Testing instructions