From 91b80d50a014f48dbf02e8afa9532929d565752b Mon Sep 17 00:00:00 2001 From: Aran-Fey Date: Mon, 24 Mar 2025 08:22:24 +0100 Subject: [PATCH 1/2] fix Revealer breaking if opened/closed rapidly --- frontend/code/components/revealer.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frontend/code/components/revealer.ts b/frontend/code/components/revealer.ts index 3bb9907d..e88afc03 100644 --- a/frontend/code/components/revealer.ts +++ b/frontend/code/components/revealer.ts @@ -4,6 +4,7 @@ import { ComponentId, TextStyle } from "../dataModels"; import { commitCss } from "../utils"; import { ComponentBase, ComponentState, DeltaState } from "./componentBase"; import { RippleEffect } from "../rippleEffect"; +import { RioAnimationPlayback, RioKeyframeAnimation } from "../animations"; let HEADER_PADDING: number = 0.3; @@ -23,6 +24,7 @@ export class RevealerComponent extends ComponentBase { private contentInnerElement: HTMLElement; private rippleInstance: RippleEffect; + private currentAnimationId: number = 0; createElement(): HTMLElement { // Create the HTML @@ -171,6 +173,9 @@ export class RevealerComponent extends ComponentBase { return; } + this.currentAnimationId++; + let animationId = this.currentAnimationId; + // Update the CSS to trigger the expand animation this.contentOuterElement.style.maxHeight = "0"; this.element.classList.add("rio-revealer-open"); @@ -187,6 +192,11 @@ export class RevealerComponent extends ComponentBase { // Once the animation is finished, remove the max-height so that the // child component can freely resize itself setTimeout(() => { + // Make sure no other animation has been started in the meantime + if (animationId !== this.currentAnimationId) { + return; + } + this.contentOuterElement.style.maxHeight = "unset"; }, 1000 * 0.25); }); @@ -198,6 +208,8 @@ export class RevealerComponent extends ComponentBase { return; } + this.currentAnimationId++; + // Again, animating from `max-height: unset` doesn't work, so we have to // set it to the child's size in pixels this.setMaxHeightToChildHeight(); From 4227d439c23e346dbd6330cf040716ddfef3d803 Mon Sep 17 00:00:00 2001 From: Aran-Fey Date: Mon, 24 Mar 2025 19:02:04 +0100 Subject: [PATCH 2/2] fix the fix for the revealers --- frontend/code/components/revealer.ts | 77 +++++++++++++++++---------- frontend/css/components/revealer.scss | 4 +- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/frontend/code/components/revealer.ts b/frontend/code/components/revealer.ts index e88afc03..be8736c7 100644 --- a/frontend/code/components/revealer.ts +++ b/frontend/code/components/revealer.ts @@ -4,7 +4,11 @@ import { ComponentId, TextStyle } from "../dataModels"; import { commitCss } from "../utils"; import { ComponentBase, ComponentState, DeltaState } from "./componentBase"; import { RippleEffect } from "../rippleEffect"; -import { RioAnimationPlayback, RioKeyframeAnimation } from "../animations"; +import { + RioAnimation, + RioAnimationPlayback, + RioKeyframeAnimation, +} from "../animations"; let HEADER_PADDING: number = 0.3; @@ -24,7 +28,7 @@ export class RevealerComponent extends ComponentBase { private contentInnerElement: HTMLElement; private rippleInstance: RippleEffect; - private currentAnimationId: number = 0; + private currentAnimation: RioAnimationPlayback | null = null; createElement(): HTMLElement { // Create the HTML @@ -173,32 +177,28 @@ export class RevealerComponent extends ComponentBase { return; } - this.currentAnimationId++; - let animationId = this.currentAnimationId; - - // Update the CSS to trigger the expand animation - this.contentOuterElement.style.maxHeight = "0"; - this.element.classList.add("rio-revealer-open"); - // The components may currently be in flux due to a pending re-layout. // If that is the case, reading the `scrollHeight` would lead to an // incorrect value. Wait for the resize to finish before fetching it. requestAnimationFrame(() => { - // Animating max-height only works with fixed values (and not - // `unset`, etc), so we have to assign the child's exact height in - // pixels - this.setMaxHeightToChildHeight(); + // Cancel the current animation, if any + if (this.currentAnimation !== null) { + this.currentAnimation.cancel(); + } - // Once the animation is finished, remove the max-height so that the - // child component can freely resize itself - setTimeout(() => { - // Make sure no other animation has been started in the meantime - if (animationId !== this.currentAnimationId) { - return; - } + // Start the open animation + let animation = this.makeAnimation("open"); + this.currentAnimation = animation.animate(this.contentOuterElement); + // The animation requires a specific height to animate to, but this + // prevents the content from resizing itself. So at the end of the + // animation, remove the `max-height`. + this.currentAnimation.addEventListener("end", () => { this.contentOuterElement.style.maxHeight = "unset"; - }, 1000 * 0.25); + }); + + // Update the CSS to trigger all the other animations + this.element.classList.add("rio-revealer-open"); }); } @@ -208,23 +208,42 @@ export class RevealerComponent extends ComponentBase { return; } - this.currentAnimationId++; + // Cancel the current animation, if any + if (this.currentAnimation !== null) { + this.currentAnimation.cancel(); + } - // Again, animating from `max-height: unset` doesn't work, so we have to - // set it to the child's size in pixels - this.setMaxHeightToChildHeight(); - commitCss(this.contentOuterElement); + // Start the close animation + let animation = this.makeAnimation("close"); + this.currentAnimation = animation.animate(this.contentOuterElement); this.element.classList.remove("rio-revealer-open"); - this.contentOuterElement.style.maxHeight = "0"; } - private setMaxHeightToChildHeight(): void { + private makeAnimation(mode: "open" | "close"): RioAnimation { + let keyframes = [ + { maxHeight: "0" }, + // Animating to/from "unset" doesn't work, so we need to obtain the + // actual height + { maxHeight: this.getHeightForAnimation() }, + ]; + + if (mode === "close") { + keyframes.reverse(); + } + + return new RioKeyframeAnimation(keyframes, { + duration: 250, + easing: "ease-in-out", + }); + } + + private getHeightForAnimation(): string { let contentHeight = this.contentInnerElement.scrollHeight; let selfHeight = this.element.scrollHeight; let headerHeight = this.headerElement.scrollHeight; let targetHeight = Math.max(contentHeight, selfHeight - headerHeight); - this.contentOuterElement.style.maxHeight = `${targetHeight}px`; + return `${targetHeight}px`; } } diff --git a/frontend/css/components/revealer.scss b/frontend/css/components/revealer.scss index 74293d53..7e81c884 100644 --- a/frontend/css/components/revealer.scss +++ b/frontend/css/components/revealer.scss @@ -40,9 +40,7 @@ .rio-revealer-content-outer { flex-grow: 1; - overflow: hidden; - - transition: max-height 0.25s ease-in-out; + overflow: hidden; // Required for the open/close animation } .rio-revealer-content-inner {