angular / angular/components

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

Offen
#25,990 4 Kommentare 18 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
area: cdk/drag-drop needs investigation P3 perf
Vorherrschende Sprache
TypeScript
Sterne
25k
Forks
6.8k
Ø Merge
1 T. 8 Std.
Gemergte PRs (30 T.)
91

Beschreibung

### 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.

![image](https://user-images.githubusercontent.com/5865946/202159904-9e7fa386-0c2f-4b5d-94b5-78b5df3133c6.png)

![image](https://user-images.githubusercontent.com/5865946/202160378-d16a6be1-a661-4d68-87e9-d7d2af847825.png)

### There are 42 drop zones.

image

### There are 400+ drag refs.

image

###
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.

image

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**.

image

image

image

It extends styles from [toggleVisibility](https://github.com/angular/components/blob/main/src/cdk/drag-drop/drag-ref.ts#L825) (WRITE), insert placeholder, in body ` this._document.body.appendChild(parent.replaceChild(placeholder, element));` [Line no. 826](https://github.com/angular/components/blob/main/src/cdk/drag-drop/drag-ref.ts#L826) (WRITE) and getBoundingClientRect in [_cacheItemPositions](https://github.com/angular/components/blob/main/src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts#L304) (READ).

Suggested Solution

1. _cacheItemsPositions, getBoundingClientRect (READ) first, appendChild, extend Styles (WRITE) later. Not sure is it possible.
(I read it from [avoid-layout-thrashing thrashing](https://web.dev/avoid-large-complex-layouts-and-layout-thrashing/#avoid-layout-))

2. Same code, but batching read write DOM i.e. [FastDOM](https://github.com/wilsonpage/fastdom)

3. 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.

image

It first starts from **_updateActiveDropContainer** function.

In the investigation, the [enter function](https://github.com/angular/components/blob/main/src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts#L175) 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
image

Again layout thrashing.

### Suggested Solution

0. let users provide their own implementation DropList Sort Strategy Item extends [DropListSortStrategyItem](https://github.com/angular/components/blob/main/src/cdk/drag-drop/sorting/drop-list-sort-strategy.ts#LL23C35-L23C35)
implements [DropListSortStrategy](https://github.com/angular/components/blob/main/src/cdk/drag-drop/sorting/drop-list-sort-strategy.ts#L35)

Then, dropListRef sortStrategy has to be configured instead of fixed SingleAxisSortStrategy.

```
this._sortStrategy = new SingleAxisSortStrategy(this.element, _dragDropRegistry); // https://github.com/angular/components/blob/main/src/cdk/drag-drop/drop-list-ref.ts#L196
```

1. _cacheItemsPositions, getBoundingClientRect (READ) first, appendChild, extend Styles (WRITE) later. Not sure is it possible.
(I read from [avoid-layout-thrashing thrashing](https://web.dev/avoid-large-complex-layouts-and-layout-thrashing/#avoid-layout-))

2. Same code, but batching read write DOM i.e. [FastDOM](https://github.com/wilsonpage/fastdom)

3. The drag drop library mandatory insert placeholder at startDragSequence which is not optimum. I think placeholder should be optional, and also optional extend styles.

Here is similar Performance profiling in json not same as images above but same problem.
[Profile-20221117T201453.json.zip](https://github.com/angular/components/files/10031835/Profile-20221117T201453.json.zip)

### Reproduction

Steps to reproduce:
1. Production Link to be sent privately
2.

### 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.**

![image](https://user-images.githubusercontent.com/5865946/202158235-9860213a-78a7-4985-a0e0-4421f264191f.png)

### Actual Behavior

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

![image](https://user-images.githubusercontent.com/5865946/202159904-9e7fa386-0c2f-4b5d-94b5-78b5df3133c6.png)

### 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

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginne mit _toggleNativeDragInteractions und _cacheItemPositions in src/cdk/drag-drop/drag-ref.ts und src/cdk/drag-drop/sorting/single-axis-sort-strategy.ts und verwende dabei die bereitgestellten Profiling-Daten und die Reproduktion. Überprüfe enter in single-axis-sort-strategy.ts sowie drop-list-sort-strategy.ts und drop-list-ref.ts, um die vorgeschlagene Erweiterung der Sortierung zu verstehen. Als erledigt gilt die Aufgabe, wenn das Ziehen ohne die gemeldeten erzwungenen Layout-Reflows weiterhin korrekt funktioniert.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
angular, typescript
Bereich
frontend, performance
Issue-Typ
Bug
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Veraltet
Klarheit
Muss geklärt werden
Anfängerfreundlichkeit
25/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.