Skip to content

Editor: Stop re-rendering all blocks when navigating with arrow keys#11397

Merged
gziolo merged 1 commit intomasterfrom
update/perf-optimization-block-list
Nov 5, 2018
Merged

Editor: Stop re-rendering all blocks when navigating with arrow keys#11397
gziolo merged 1 commit intomasterfrom
update/perf-optimization-block-list

Conversation

@gziolo
Copy link
Copy Markdown
Member

@gziolo gziolo commented Nov 2, 2018 •
edited
Loading

Description

How has this been tested?

I tested it using React Dev Tools by enabling Highlight Updates option in settings (see screencasts for details).

Use arrow keys (up/down) to navigate between blocks and observe whether blocks re-render less often than before. In general, only previous and current block should change.

Solution

selectionStartClientId is only used when selection is enabled (isSelectionEnabled). This PR ensure that the needed value is set only in the correct context.

Screenshots

Before

block list before

After

block list after

Types of changes

Refactoring, no changes to the logic.

@gziolo gziolo added [Type] Performance Related to performance efforts [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... labels Nov 2, 2018
@gziolo gziolo added this to the 4.3 milestone Nov 2, 2018
@gziolo gziolo self-assigned this Nov 2, 2018
@gziolo gziolo requested review from a team and aduth November 2, 2018 10:02
tofumatt
tofumatt approved these changes Nov 2, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

Niiiiiice. This is awesome; such an easy win.

@aduth
Copy link
Copy Markdown
Member

aduth commented Nov 2, 2018

selectionStartClientId is only used when multi-selection is enabled. This PR ensure that the needed value is set only in the correct context.

I'm trying to understand when and why selection is being toggled such that it would have an impact here. Do you have more information?

The way I understand it, isSelectionEnabled would always return true except in rare cases where e.g. the user is resizing an image.

@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Nov 5, 2018

The way I understand it, isSelectionEnabled would always return true except in rare cases where e.g. the user is resizing an image.

Yes, it is effectively being disabled only for Image and Spacer blocks when resizing starts by dispatching toggleSelection action which sets isSelectionEnabled to false. From what I see there are 3 event handlers here which change behavior based on that. It looks like the same mouse handlers are attached to the block here which are being handled in mentioned blocks. I'm wondering if it would be possible to stop propagating those events instead.

@gziolo gziolo merged commit 273da09 into master Nov 5, 2018
@gziolo gziolo deleted the update/perf-optimization-block-list branch November 5, 2018 07:56
@gziolo gziolo mentioned this pull request Nov 5, 2018
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tofumatt tofumatt tofumatt approved these changes

@aduth aduth Awaiting requested review from aduth

Assignees

@gziolo gziolo

Labels

[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Type] Performance Related to performance efforts

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

3 participants

@gziolo @aduth @tofumatt