bug(drag-drop): Layout thrashing (forced layout reflow) when dragging between dropzones with 400+ complexed drag refs

perjerz commented 2 years ago

Is this a regression?

[ ] Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

Problem

The layout thrashing problem occurs when the user drag the complex element between drop zones.

There are 42 drop zones.

There are 400+ drag refs.

The drag refs are quite a bit complex. We have several kind of drag Refs. For example, we have a text component which including Text Editor (Quill) inside.

However, we think the number of components and drag ref complexity of root element are not the main reason for layout thrashing.

In the investigation, there are two cases make layout thrashing when dragging between drop zones.

The first is in extendStyles in _toggleNativeDragInteractions in _startDragSequence, and then call _cacheItemPositions for loop each item in a drop zone which call getMutableClientRect which call getBoundingClientRect.

It extends styles from toggleVisibility (WRITE), insert placeholder, in body this._document.body.appendChild(parent.replaceChild(placeholder, element)); Line no. 826 (WRITE) and getBoundingClientRect in _cacheItemPositions (READ).

Suggested Solution

_cacheItemsPositions, getBoundingClientRect (READ) first, appendChild, extend Styles (WRITE) later. Not sure is it possible. (I read it from avoid-layout-thrashing thrashing)
Same code, but batching read write DOM i.e. FastDOM
The drag drop library mandatory insert placeholder at _startDragSequence and _endDragSequence which is not optimum. I think placeholder should be optional, and also optional extend styles.

That's all for the first layout thrashing.

The second is almost the same as the first but a little difference. This always happen when dragging item changing active container. This layout thrashing always makes Janky user experience.

It first starts from _updateActiveDropContainer function.

In the investigation, the enter function in SingleAxisSortStrategy class has insertBefore placeholder. (WRITE)

    enter(item: T, pointerX: number, pointerY: number, index?: number): void {
    ...
     // Don't use items that are being dragged as a reference, because
        // their element has been moved down to the bottom of the body.
        if (newPositionReference && !this._dragDropRegistry.isDragging(newPositionReference)) {
            const element = newPositionReference.getRootElement();
            element.parentElement.insertBefore(placeholder, element); // <------ Here WRITE
            activeDraggables.splice(newIndex, 0, item);
        }
        else {
            coerceElement(this._element).appendChild(placeholder); // <------ Here WRITE
            activeDraggables.push(item);
        }
          // The transform needs to be cleared so it doesn't throw off the measurements.
        placeholder.style.transform = '';
        // Note that usually `start` is called together with `enter` when an item goes into a new
        // container. This will cache item positions, but we need to refresh them since the amount
        // of items has changed.
        this._cacheItemPositions(); // <----- HERE READ
    }

And then this._cacheItemPositions(); which call getMutableClientRect call getBoundingClientRect

Again layout thrashing.

Reproduction

Steps to reproduce:

Production Link to be sent privately

Expected Behavior

It should have no layout thrashing (no forced reflow). This image below shown there is no layout thrashing when I deleted the code insertBefore, extendStyles, but the behavior of dragging is bug.

Actual Behavior

There are layout thrashing every time, the placeholder has been inserted.

Environment

Angular: 14.2.9
CDK/Material: CDK
Browser(s): Chromium (Edge, Chrome), Firefox
Operating System (e.g. Windows, macOS, Ubuntu): Windows, macOS, Ubuntu

Angular CLI: 14.2.9 Node: 16.13.2 Package Manager: npm 8.5.3 OS: darwin arm64

Angular: 14.2.7 ... animations, common, compiler, compiler-cli, core, forms ... google-maps, language-service, platform-browser ... platform-browser-dynamic, router, service-worker

Package Version

@angular-devkit/architect 0.1402.6 @angular-devkit/build-angular 14.2.6 @angular-devkit/core 14.2.9 @angular-devkit/schematics 14.2.9 @angular/cdk 14.2.5 @angular/cli 14.2.9 @angular/fire 7.4.1 @angular/material 14.2.5 @angular/material-moment-adapter 14.2.5 @schematics/angular 14.2.9 rxjs 7.5.7 typescript 4.8.4 webpack 5.75.0

Related issues

angular/angular#20471 angular/components#13372

perjerz commented 1 year ago

This Stackblitz also show the same behavior of layout thrashing when dragging item with several DOMs in several drag refs.

https://stackblitz.com/edit/angular-sxsjem

Here is Performance profiling in json. Profile-20221117T201056.json.zip

nuttaponkhamkul123 commented 1 year ago

I made the huge amount of dragrefs and happend to me too. any solution for this?

perjerz commented 1 year ago

CSS runtime performance | Nolan Lawson | performance.now() 2022 nolanlawson.github.io/css-talk-2022/

e-oz commented 1 year ago

This Stackblitz also show the same behavior of layout thrashing when dragging item with several DOMs in several drag refs.

I don't understand how exactly this demo should work, but switching changeDetectionStrategy to "OnPush" helped to make it usable.

angular / components