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

Editor: Optimize Inserter props generation and reconciliation #11243

Merged
merged 1 commit into from
Oct 30, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Oct 30, 2018

Related (potentially blocks): #11018

This pull request seeks to apply minor refactoring to the Inserter to optimize its rendering:

  • Avoid passing selectedBlock to the root component, as its reference changes frequently (i.e. every keystroke in the selected paragraph) and is only needed for the opened menu.
    • Similarly, avoid passing items array to the root component, and pass only the simple boolean reference of hasItems for use in considering whether the inserter should be rendered at all (via ifCondition)
  • Assign toggle and content renderers as a consistent bound reference on the root component class, avoiding reconciliation which would otherwise occur in the changing function references on each render.

Further optimization could be achieved in simplifying how hasItems is calculated, since getInserterItems is easily the most complex / convoluted selector we have in the core/editor store. This has not been done here, but I foresee one or more of:

  • Splitting getInserterItems into more granular memoized selectors
  • Creating a separate hasInserterItems selector which performs the more simple filtering (not mapping) operations
  • Defer the internal "build" operations of getInserterItems to the inserter component itself, passing instead the minimal raw data necessary.

Testing instructions:

Verify there are no regressions in the behavior of the inserter, with notable variations:

  • With and without a block selected
  • Inserter in header and empty-paragraph-adjacent

Using React DevTools "Highlight Updates" option, verify that the Inserter does not re-render when typing within a paragraph.

image

@aduth aduth added [Feature] Inserter The main way to insert blocks using the + button in the editing interface [Type] Performance Related to performance efforts [Type] Code Quality Issues or PRs that relate to code quality labels Oct 30, 2018
@aduth aduth force-pushed the update/inserter-perf branch from 2928106 to 8e24e8d Compare October 30, 2018 14:08
@aduth aduth added this to the 4.2 milestone Oct 30, 2018
@aduth aduth force-pushed the update/inserter-perf branch from 8e24e8d to 1ce8c5c Compare October 30, 2018 17:13
Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I no longer see re-renders when navigating between blocks. Code is very identical to my previous implementation but this one is backward compatible so this is a big plus.

Nice work! 💯

@youknowriad youknowriad merged commit 6e6c8d5 into master Oct 30, 2018
@youknowriad youknowriad deleted the update/inserter-perf branch October 30, 2018 17:28
daniloercoli added a commit that referenced this pull request Oct 31, 2018
…rnmobile/port-quote-block-step-1

* 'master' of https://github.com/WordPress/gutenberg: (21 commits)
  Fix property path on get() call (#10962)
  Fixed typos on block api documentation (#11298)
  Export `switchToBlockType` to be used mobile side when merging two blocks. (#11294)
  RichText: Remove unused `ref` assignment to RichText (#11222)
  Remove findDOMNode from Tooltip component (#11169)
  Components: Remove redundant onClickOutside handler from Dropdown (#11253)
  added myself to the contributors list (#11260)
  Add complete post type labels for Resuable Blocks (#11278)
  Increase specificity for active radio/checkbox input styling (#11290)
  Fixed "artifact" misspelling in docs. (#11291)
  Nux package: fix incorrect named deprecated import (#11283)
  Rename parentClientId to rootClientId for consistency (#11274)
  chore(release): update changelog files
  chore(release): publish
  Update plugin version to 4.2.0. (#11258)
  Data: Use turbo-combine-reducers in place of Redux (#11255)
  Revert using Icon in IconButton to avoid regression in plugin icons (pinned icons) (#11256)
  Block List: Use default Inserter for sibling insertion (#11018)
  Editor: Optimize Inserter props generation and reconciliation (#11243)
  RichText: fix format placeholder (#11102)
  ...

# Conflicts:
#	packages/block-library/src/quote/index.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Type] Code Quality Issues or PRs that relate to code quality [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants