Block: Avoid rerendering when the previous/next block updates#5025
Merged
youknowriad merged 2 commits intomasterfrom Feb 13, 2018
Merged
Block: Avoid rerendering when the previous/next block updates#5025youknowriad merged 2 commits intomasterfrom
youknowriad merged 2 commits intomasterfrom
Conversation
Contributor
Author
|
It looks like in all places we use |
Member
|
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. |
b19542a to
124ea5d
Compare
Contributor
Author
|
Rebased, thanks @aduth |
aduth
approved these changes
Feb 13, 2018
| getEditedPostAttribute, | ||
| getNextBlock, | ||
| getPreviousBlock, | ||
| getNextBlockUid, |
Member
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
Contributor
Author
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
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
2 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.
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