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

Components: Render DropZone children only if dragging over element #12852

Merged
merged 5 commits into from Dec 21, 2018

Conversation

Projects
None yet
3 participants
@aduth
Copy link
Member

aduth commented Dec 13, 2018

Related: #12858
Refs #11782

This pull request seeks to optimize the rendering of the DropZone component to omit its own children in lieu of rendering them styled as display: none; when not hovered. A DropZone element is rendered for each block in the page, which can contribute significantly as the number of blocks in a post increases. In my testing for a post consisting of 20,000 words (500 paragraphs), the changes here have an impact in reducing the total number of DOM nodes by ~18%, from 15,390 to 12,614. Combined with #12858, the improvement totals a 35% reduction in total number of DOM nodes (to a final count of 9,956).

Further, it refactors the rendering to avoid rendering its static set of children as an array. This may have some negligible improvement in React's reconciliation, though it has not been measured.

Testing instructions:

Verify there are no regressions in the behavior of drag-n-drop media (default block appender, before/after block, on media placeholder).

Compare DOM nodes between this branch and master:

document.getElementsByTagName( '*' ).length
@nosolosw

This comment has been minimized.

Copy link
Member

nosolosw commented Dec 14, 2018

Have tested a couple of scenarios:

  • DnD external files
  • DnD external images onto 1) galleries and 2) as a separate block
  • DnD blocks onto themselves (their top and bottom drop zones)
  • DnD blocks onto the drop zones of the previous and consecutive blocks
  • DnD blocks around

Other than fixing some test snapshots, this works great and is ready for landing.

@nosolosw
Copy link
Member

nosolosw left a comment

At the moment Travis/phpcs/etc are acting up so CI is blocking the merge. Marking as ready for when that's fixed (I assume it doesn't have to do with this PR as I've seen the same messages in other PRs).

@youknowriad youknowriad added this to the 4.8 milestone Dec 21, 2018

@youknowriad youknowriad force-pushed the update/omit-drop-zone-children branch from d3d73ab to 767c79f Dec 21, 2018

@youknowriad youknowriad merged commit c123f1d into master Dec 21, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the update/omit-drop-zone-children branch Dec 21, 2018

youknowriad added a commit that referenced this pull request Jan 3, 2019

Components: Render DropZone children only if dragging over element (#…
…12852)

* Components: Render DropZone children only if dragging over element

* Update audio test snapshot

* Update cover test snapshot

* Update video test snapshot

* Update gallery test snapshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment