Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance: Simplify return value of multi-selection selected selector #3173

Merged
merged 1 commit into from
Oct 27, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Oct 26, 2017

Related: #3171, #3170, #3172

This pull request seeks to optimize the rendering of individual blocks when selection occurs. Currently, any time a selection is made, including for a single block, the isSelected prop changes for every block such that it incurs a render on all blocks. Since this change is often from two equivalent states for most blocks (null to false, and vice-versa), the changes here seek to simplify the return value of the selector for determining whether a single block is selected to simply return false when a multi-selection exists (instead of null). This prevents renders except in the blocks included in the multi-selection.

Implementation notes:

The documented behavior of the isBlockSelected was opposite that of its implementation. Both documentation and tests have been updated to reflect what appears to be the intended behavior, but verification would be appreciated.

Testing instructions:

Verify there are no regressions in the behavior of block multi-selection.

@aduth aduth added the [Type] Performance Related to performance efforts label Oct 26, 2017
@codecov
Copy link

codecov bot commented Oct 26, 2017

Codecov Report

Merging #3173 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3173   +/-   ##
=======================================
  Coverage   31.61%   31.61%           
=======================================
  Files         217      217           
  Lines        6278     6278           
  Branches     1116     1116           
=======================================
  Hits         1985     1985           
  Misses       3609     3609           
  Partials      684      684
Impacted Files Coverage Δ
editor/selectors.js 95.65% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b444cb...41fb561. Read the comment docs.

@aduth aduth merged commit b351836 into master Oct 27, 2017
@aduth aduth deleted the update/perf-is-multi-selected branch October 27, 2017 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant