JS: add visual effect when delete a blockSubWidget (#76172) #376

tjund · 2023-06-13T16:14:42+02:00

tjund commented

2023-06-13 16:14:42 +02:00

No description provided.

fpeters reviewed 2023-06-13 16:26:58 +02:00

wcs/qommon/static/js/qommon.forms.js Outdated

						
				@ -734,0 +737,4 @@

				        })

				        subwidget.style.height = subwidget.clientHeight + 'px';

				        subwidget.style.transition = 'opacity 400ms, height 300ms 400ms';

fpeters commented

2023-06-13 16:26:58 +02:00

J'ai toujours lu qu'il fallait éviter les animations sur height. (?)

tjund commented

2023-06-13 17:30:58 +02:00

Peut-être parce que par défaut ça ne marche pas avec height: auto ?
Ou pour des soucis de perf ?
(une animation sur l'opacité est plus lourde en perf que sur la hauteur).

Peut-être parce que par défaut ça ne marche pas avec `height: auto` ? Ou pour des soucis de perf ? (une animation sur l'opacité est plus lourde en perf que sur la hauteur).

fpeters commented

2023-06-13 17:38:37 +02:00

Une recherche vite fait me donne https://www.freecodecamp.org/news/animating-height-the-right-way/ :

The reason for this is of course that animating the height property in CSS causes the browser to perform expensive layout and paint operations on every frame.

Une autre recherche donne une référence côté google dans cet article, https://web.dev/animations-guide/#triggers

Before using any CSS property for animation (other than transform and opacity), determine the property's impact on the rendering pipeline. Avoid any property that triggers layout or paint unless absolutely necessary.

et donc ça répond aussi à :

(une animation sur l'opacité est plus lourde en perf que sur la hauteur).

en disant le contraire.

Une recherche vite fait me donne https://www.freecodecamp.org/news/animating-height-the-right-way/ : > The reason for this is of course that animating the height property in CSS causes the browser to perform expensive layout and paint operations on every frame. Une autre recherche donne une référence côté google dans cet article, https://web.dev/animations-guide/#triggers > Before using any CSS property for animation (other than transform and opacity), determine the property's impact on the rendering pipeline. Avoid any property that triggers layout or paint unless absolutely necessary. et donc ça répond aussi à : > (une animation sur l'opacité est plus lourde en perf que sur la hauteur). en disant le contraire.

tjund commented

2023-06-13 17:53:07 +02:00

Merci pour les liens.
Comme j'applique une opacité à 0 en premier, je peux reduire l'espace avec une transition. Je change.

Merci pour les liens. Comme j'applique une opacité à 0 en premier, je peux reduire l'espace avec une transition. Je change.

tjund commented

2023-06-13 18:03:31 +02:00

ah ben non, transform ne modifie pas l'espace d'origine.
Je ne vois pas vraiment d'alternative.

ah ben non, transform ne modifie pas l'espace d'origine. Je ne vois pas vraiment d'alternative.

tjund commented

2023-06-14 12:35:25 +02:00

J'ai fait plein de tests avec l'onglet performance.

Aucune alternative à l'animation de height pour réduire l'espace d'un element. max-height revient au même niveau perf. Tous les scripts d'accordéon animent height.
Opacity est en effet plus performant que height, beaucoup moins de reflow, utilise le GPU par défaut.
Mais franchement, rien d'alarmant, le CPU monte à 60% quelques milisecondes sur un durée de 300ms. Testé sur mon antique smartphone (un s7 qui fête ses 7 ans) et pas noté de problème de perf visible.
L'alternative perf serait de ne pas animer la disparition de l'espace laissé vide, mais c'est vraiment moche après une transition sur l'opacité d'avoir un à-coup, un saut.
et enfin, dernière piste est aussi de respecter le choix des usagers qui ne veulent pas jouer les animations (comme mode économie d'énergie ou a11y) via le `@media screen and (prefers-reduced-motion: reduce) { transition-duration: 0 !important } dans PBT et gadjo. (j'ouvre un ticket).

J'ai fait plein de tests avec l'onglet performance. * Aucune alternative à l'animation de height pour réduire l'espace d'un element. max-height revient au même niveau perf. Tous les scripts d'accordéon animent height. * Opacity est en effet plus performant que height, beaucoup moins de reflow, utilise le GPU par défaut. * Mais franchement, rien d'alarmant, le CPU monte à 60% quelques milisecondes sur un durée de 300ms. Testé sur mon antique smartphone (un s7 qui fête ses 7 ans) et pas noté de problème de perf visible. * L'alternative perf serait de ne pas animer la disparition de l'espace laissé vide, mais c'est vraiment moche après une transition sur l'opacité d'avoir un à-coup, un saut. * et enfin, dernière piste est aussi de respecter le choix des usagers qui ne veulent pas jouer les animations (comme mode économie d'énergie ou a11y) via le `@media screen and (prefers-reduced-motion: reduce) { transition-duration: 0 !important } dans PBT et gadjo. (j'ouvre un ticket).

transition opacity-height.png

216 KiB

tjund force-pushed wip/76172-animation-delete-block-row from 20f61e3d30 to 5caa38aa25

2023-06-14 15:52:01 +02:00

Compare

csechet approved these changes 2023-06-14 15:56:56 +02:00

csechet left a comment

Après discussion avec Thomas, je valide ça, @fpeters. Si on peut la désactiver pour les utilisateurs qui le demandent, ça me parait propre, et c'est pas une animation qui joue en permanence sur la page.

tjund merged commit ea62cfac46 into main

2023-06-14 16:25:27 +02:00

tjund deleted branch wip/76172-animation-delete-block-row

2023-06-14 16:25:27 +02:00

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

3 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/wcs#376