From 046fb9c817c9fe58e47e2ad0876a581404c1c683 Mon Sep 17 00:00:00 2001 From: Gondragos Date: Tue, 4 Mar 2025 11:57:10 +0000 Subject: [PATCH 1/8] feat: LEAP-1896: Add scrollbar support to Visualizer Introduced a scroll filler element to improve handling of browser scroll size inconsistencies in the Visualizer. Updated relevant methods and styles to adjust layout dynamically based on the calculated scroll size. Added safeguards to mouse events. --- .../editor/src/lib/AudioUltra/Common/Utils.ts | 24 +++++++++ .../src/lib/AudioUltra/Visual/Visualizer.ts | 50 ++++++++++++++++--- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts b/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts index 7e8c13ce1256..cf6d7382a1fa 100644 --- a/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts +++ b/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts @@ -212,3 +212,27 @@ export const getCursorTime = (e: MouseEvent, visualizer: Visualizer, duration: n export const isTimeSimilar = (a: number, b: number) => Math.abs(a - b) < TIME_TOLERANCE; export const isTimeRelativelySimilar = (a: number, b: number, observedDuration: number) => isTimeSimilar(a / observedDuration, b / observedDuration); + +/** + * A constant representing the width of the browser's scrollbar in pixels. + * This value is calculated dynamically by creating a temporary DOM element + * with a scrollable area and comparing its offset width to its client width. + * Useful for making precise layout adjustments that depend on the scrollbar size. + * + * Note: The calculation is performed immediately when the variable is defined + * and retains its value for the duration of runtime. + * + * @constant {number} + */ +export const BROWSER_SCROLL_SIZE = ((): number => { + const scrollDiv = document.createElement("div"); + scrollDiv.style.width = "100px"; + scrollDiv.style.height = "100px"; + scrollDiv.style.overflow = "scroll"; + scrollDiv.style.position = "absolute"; + scrollDiv.style.top = "-9999px"; + document.body.appendChild(scrollDiv); + const scrollSize = scrollDiv.offsetWidth - scrollDiv.clientWidth; + document.body.removeChild(scrollDiv); + return scrollSize; +})(); diff --git a/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts b/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts index e0846fdd7950..4a6d0137ff40 100644 --- a/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts +++ b/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts @@ -1,5 +1,5 @@ import type { WaveformAudio } from "../Media/WaveformAudio"; -import { averageMinMax, clamp, debounce, defaults, warn } from "../Common/Utils"; +import { averageMinMax, BROWSER_SCROLL_SIZE, clamp, debounce, defaults, warn } from "../Common/Utils"; import type { Waveform, WaveformOptions } from "../Waveform"; import { type CanvasCompositeOperation, Layer, type RenderingContext } from "./Layer"; import { Events } from "../Common/Events"; @@ -48,6 +48,7 @@ export type VisualizerOptions = Pick< export class Visualizer extends Events { private wrapper!: HTMLElement; + private scrollFiller!: HTMLElement; private layers = new Map(); private observer!: ResizeObserver; private currentTime = 0; @@ -183,6 +184,7 @@ export class Visualizer extends Events { } this.getSamplesPerPx(); + this.updateScrollFiller(); this.wf.invoke("zoom", [this.zoom]); this.draw(); @@ -193,6 +195,11 @@ export class Visualizer extends Events { } setScrollLeft(value: number, redraw = true, forceDraw = false) { + this.wrapper.scrollLeft = value * this.fullWidth; + this._setScrollLeft(value, redraw, forceDraw); + } + + _setScrollLeft(value: number, redraw = true, forceDraw = false) { this.scrollLeft = value; if (redraw) { @@ -274,13 +281,13 @@ export class Visualizer extends Events { centerToCurrentTime() { if (this.zoom === 1) { - this.scrollLeft = 0; + this.setScrollLeft(0); return; } const offset = this.width / 2 / this.zoomedWidth; - this.scrollLeft = clamp(this.currentTime - offset, 0, 1); + this.setScrollLeft(clamp(this.currentTime - offset, 0, 1)); } /** @@ -638,18 +645,38 @@ export class Visualizer extends Events { this.wrapper = document.createElement("div"); this.wrapper.style.height = "100%"; + this.wrapper.style.position = "relative"; + this.wrapper.style.overflow = "scroll hidden"; - this.createLayer({ name: "main" }); + const mainLayer = this.createLayer({ name: "main" }); this.createLayer({ name: "background", offscreen: true, zIndex: 0, isVisible: false }); this.createLayer({ name: "waveform", offscreen: true, zIndex: 100 }); this.createLayerGroup({ name: "regions", offscreen: true, zIndex: 101, compositeOperation: "source-over" }); const controlsLayer = this.createLayer({ name: "controls", offscreen: true, zIndex: 1000 }); this.playhead.setLayer(controlsLayer); - this.layers.get("main")?.appendTo(this.wrapper); + + mainLayer.canvas.style.position = "sticky"; + mainLayer.canvas.style.top = "0"; + mainLayer.canvas.style.left = "0"; + mainLayer.canvas.style.zIndex = "2"; + mainLayer?.appendTo(this.wrapper); + + this.scrollFiller = document.createElement("div"); + this.scrollFiller.style.position = "absolute"; + this.scrollFiller.style.width = "100%"; + this.scrollFiller.style.height = `${BROWSER_SCROLL_SIZE}px`; + this.scrollFiller.style.top = "100%"; + this.wrapper.appendChild(this.scrollFiller); + container.appendChild(this.wrapper); } + updateScrollFiller() { + const { fullWidth } = this; + this.scrollFiller.style.width = `${fullWidth}px`; + } + reserveSpace({ height }: { height: number }) { this.reservedSpace = height; } @@ -792,6 +819,12 @@ export class Visualizer extends Events { this.wrapper.addEventListener("click", this.handleSeek); this.wrapper.addEventListener("mousedown", this.handleMouseDown); + this.wrapper.addEventListener("scroll", (e) => { + const scrollLeft = this.wrapper.scrollLeft / this.fullWidth; + this.wf.invoke("scroll", [scrollLeft]); + this._setScrollLeft(scrollLeft); + }); + // Cursor events this.on("mouseMove", this.playHeadMove); @@ -850,6 +883,8 @@ export class Visualizer extends Events { }; private handleSeek = (e: MouseEvent) => { + if (e.offsetY > this.height) return; + const mainLayer = this.getLayer("main"); if (!this.wf.loaded || this.seekLocked || !(e.target && mainLayer?.canvas?.contains(e.target))) return; @@ -864,6 +899,7 @@ export class Visualizer extends Events { }; private handleMouseDown = (e: MouseEvent) => { + if (e.offsetY > this.height) return; if (!this.wf.loaded) return; this.playhead.invoke("mouseDown", [e]); }; @@ -938,7 +974,7 @@ export class Visualizer extends Events { }; private setContainerHeight() { - this.container.style.height = `${this.height}px`; + this.container.style.height = `${this.height + BROWSER_SCROLL_SIZE}px`; } private updateSize() { @@ -958,6 +994,8 @@ export class Visualizer extends Events { this.wf.renderTimeline(); this.resetWaveformRender(); this.draw(false, true); + this.updateScrollFiller(); + this.setScrollLeft(this.scrollLeft); }); }; From f315f8b74df967ba08f15a76b6dce384afdbbd49 Mon Sep 17 00:00:00 2001 From: Gondragos Date: Tue, 4 Mar 2025 15:58:59 +0000 Subject: [PATCH 2/8] fix: Rename scrollbar size constant for clarity Renamed `BROWSER_SCROLL_SIZE` to `BROWSER_SCROLLBAR_WIDTH` to provide a clearer description of its purpose. Updated related references and comments to reflect this change, ensuring consistency and maintaining accurate documentation. --- web/libs/editor/src/lib/AudioUltra/Common/Utils.ts | 6 +++--- web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts b/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts index cf6d7382a1fa..6a5911825298 100644 --- a/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts +++ b/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts @@ -214,17 +214,17 @@ export const isTimeRelativelySimilar = (a: number, b: number, observedDuration: isTimeSimilar(a / observedDuration, b / observedDuration); /** - * A constant representing the width of the browser's scrollbar in pixels. + * A constant representing the thickness of the scrollbar's handle in pixels. * This value is calculated dynamically by creating a temporary DOM element * with a scrollable area and comparing its offset width to its client width. - * Useful for making precise layout adjustments that depend on the scrollbar size. + * Useful for making precise layout adjustments that depend on the width of the scrollbar. * * Note: The calculation is performed immediately when the variable is defined * and retains its value for the duration of runtime. * * @constant {number} */ -export const BROWSER_SCROLL_SIZE = ((): number => { +export const BROWSER_SCROLLBAR_WIDTH = ((): number => { const scrollDiv = document.createElement("div"); scrollDiv.style.width = "100px"; scrollDiv.style.height = "100px"; diff --git a/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts b/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts index 4a6d0137ff40..c8df2a23c82c 100644 --- a/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts +++ b/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts @@ -1,5 +1,5 @@ import type { WaveformAudio } from "../Media/WaveformAudio"; -import { averageMinMax, BROWSER_SCROLL_SIZE, clamp, debounce, defaults, warn } from "../Common/Utils"; +import { averageMinMax, BROWSER_SCROLLBAR_WIDTH, clamp, debounce, defaults, warn } from "../Common/Utils"; import type { Waveform, WaveformOptions } from "../Waveform"; import { type CanvasCompositeOperation, Layer, type RenderingContext } from "./Layer"; import { Events } from "../Common/Events"; @@ -665,7 +665,7 @@ export class Visualizer extends Events { this.scrollFiller = document.createElement("div"); this.scrollFiller.style.position = "absolute"; this.scrollFiller.style.width = "100%"; - this.scrollFiller.style.height = `${BROWSER_SCROLL_SIZE}px`; + this.scrollFiller.style.height = `${BROWSER_SCROLLBAR_WIDTH}px`; this.scrollFiller.style.top = "100%"; this.wrapper.appendChild(this.scrollFiller); @@ -974,7 +974,7 @@ export class Visualizer extends Events { }; private setContainerHeight() { - this.container.style.height = `${this.height + BROWSER_SCROLL_SIZE}px`; + this.container.style.height = `${this.height + BROWSER_SCROLLBAR_WIDTH}px`; } private updateSize() { From 8be60456534d45e5cfd2edbe54cc202889e5441b Mon Sep 17 00:00:00 2001 From: Gondragos Date: Wed, 5 Mar 2025 07:21:56 +0000 Subject: [PATCH 3/8] Improve scrollbar handling for audio visualizer Refactored scrollbar logic into a new `initScrollBar` method for better clarity and functionality. Added `scrollbarGutter: stable` for potential consistent scrollbar behavior across browsers. --- .../editor/src/lib/AudioUltra/Common/Utils.ts | 1 + .../src/lib/AudioUltra/Visual/Visualizer.ts | 22 ++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts b/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts index 6a5911825298..5936a7aaef5a 100644 --- a/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts +++ b/web/libs/editor/src/lib/AudioUltra/Common/Utils.ts @@ -231,6 +231,7 @@ export const BROWSER_SCROLLBAR_WIDTH = ((): number => { scrollDiv.style.overflow = "scroll"; scrollDiv.style.position = "absolute"; scrollDiv.style.top = "-9999px"; + scrollDiv.style.scrollbarGutter = "stable"; document.body.appendChild(scrollDiv); const scrollSize = scrollDiv.offsetWidth - scrollDiv.clientWidth; document.body.removeChild(scrollDiv); diff --git a/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts b/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts index c8df2a23c82c..e563f6958b90 100644 --- a/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts +++ b/web/libs/editor/src/lib/AudioUltra/Visual/Visualizer.ts @@ -645,8 +645,6 @@ export class Visualizer extends Events { this.wrapper = document.createElement("div"); this.wrapper.style.height = "100%"; - this.wrapper.style.position = "relative"; - this.wrapper.style.overflow = "scroll hidden"; const mainLayer = this.createLayer({ name: "main" }); this.createLayer({ name: "background", offscreen: true, zIndex: 0, isVisible: false }); @@ -655,21 +653,33 @@ export class Visualizer extends Events { const controlsLayer = this.createLayer({ name: "controls", offscreen: true, zIndex: 1000 }); this.playhead.setLayer(controlsLayer); + this.initScrollBar(); + mainLayer.appendTo(this.wrapper); + container.appendChild(this.wrapper); + } + + initScrollBar() { + this.wrapper.style.position = "relative"; + this.wrapper.style.overflowX = "scroll"; + this.wrapper.style.overflowY = "hidden"; + this.wrapper.style.scrollbarGutter = "stable"; + const mainLayer = this.getLayer("main") as Layer; + // The parent element scrolls natively, and the canvas is redrawn accordingly. + // To maintain its position during scrolling, the element must use "sticky" positioning. mainLayer.canvas.style.position = "sticky"; mainLayer.canvas.style.top = "0"; mainLayer.canvas.style.left = "0"; mainLayer.canvas.style.zIndex = "2"; - mainLayer?.appendTo(this.wrapper); - + // Adds a scroll filler element to adjust the size of the scrollable area this.scrollFiller = document.createElement("div"); this.scrollFiller.style.position = "absolute"; this.scrollFiller.style.width = "100%"; this.scrollFiller.style.height = `${BROWSER_SCROLLBAR_WIDTH}px`; this.scrollFiller.style.top = "100%"; + this.scrollFiller.style.minHeight = "1px"; + mainLayer.canvas.style.zIndex = "1"; this.wrapper.appendChild(this.scrollFiller); - - container.appendChild(this.wrapper); } updateScrollFiller() { From 366da97bc10442a450890953d234f203912a2dbf Mon Sep 17 00:00:00 2001 From: Gondragos Date: Wed, 5 Mar 2025 08:03:44 +0000 Subject: [PATCH 4/8] fix: adjust pixel color comparison for consistent tests Updated pixel color comparison coordinates in audio region tests to ensure accuracy and consistency. Added a getter to check the readiness of objects in the model and updated `isReady` logic in test helpers for smoother synchronization. --- .../src/tags/object/AudioUltra/model.js | 3 ++ .../integration/e2e/audio/audio_regions.cy.ts | 28 +++++++++---------- .../src/helpers/LSF/AudioView.ts | 2 ++ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/web/libs/editor/src/tags/object/AudioUltra/model.js b/web/libs/editor/src/tags/object/AudioUltra/model.js index a7295dba650e..b56aeaa16988 100644 --- a/web/libs/editor/src/tags/object/AudioUltra/model.js +++ b/web/libs/editor/src/tags/object/AudioUltra/model.js @@ -137,6 +137,9 @@ export const AudioModel = types.compose( _wfFrame: null, })) .views((self) => ({ + get isReady() { + return !!self._ws; + }, get hasStates() { const states = self.states(); diff --git a/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts b/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts index e62161ce48a5..857afc1f05df 100644 --- a/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts +++ b/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts @@ -12,17 +12,17 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); AudioView.clickAtRelative(0.38, 0.5); - const selectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const selectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); selectedRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); }); // unselecting cy.get("body").type("{esc}"); - const unselectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const unselectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); unselectedRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); }); @@ -38,11 +38,11 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); // moving the cursor AudioView.seekCurrentTimebox(38); - const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); activeRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -50,7 +50,7 @@ describe("Audio regions", () => { // deactivating AudioView.seekCurrentTimebox(0); - const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); inactiveRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); @@ -67,7 +67,7 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); // highlighting in relations mode Labels.select("Music"); @@ -77,7 +77,7 @@ describe("Audio regions", () => { AudioView.hoverAtRelative(0.4, 0.5); - const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); highlightedRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -86,7 +86,7 @@ describe("Audio regions", () => { // unhighlighting AudioView.container.trigger("mouseleave"); - const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); unhighlightedRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); @@ -103,7 +103,7 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); // highlighting in relations mode Labels.select("Music"); @@ -113,7 +113,7 @@ describe("Audio regions", () => { AudioView.hoverAtRelative(0.4, 0.5); - const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); highlightedRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -121,7 +121,7 @@ describe("Audio regions", () => { // moving the cursor AudioView.seekCurrentTimebox(38); - const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); activeRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -130,7 +130,7 @@ describe("Audio regions", () => { // deactivating AudioView.seekCurrentTimebox(0); - const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); // should still be highlighted inactiveRegionColor.then((color) => { @@ -141,7 +141,7 @@ describe("Audio regions", () => { // unhighlighting AudioView.container.trigger("mouseleave"); - const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); + const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); unhighlightedRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); diff --git a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts index 87263031dd25..3f38d561895a 100644 --- a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts +++ b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts @@ -1,6 +1,7 @@ import TriggerOptions = Cypress.TriggerOptions; import ObjectLike = Cypress.ObjectLike; import ClickOptions = Cypress.ClickOptions; +import { LabelStudio } from "@humansignal/frontend-test/helpers/LSF/LabelStudio"; type MouseInteractionOptions = Partial; @@ -37,6 +38,7 @@ export const AudioView = { }, isReady() { this.loadingBar.should("not.exist"); + LabelStudio.waitForObjectsReady(); }, get playButton() { return cy.get(`[data-testid="playback-button:play"]`); From b94c95ec21e9bf3ff44a61e1e2fedb01a7ca6531 Mon Sep 17 00:00:00 2001 From: Gondragos Date: Wed, 5 Mar 2025 08:13:41 +0000 Subject: [PATCH 5/8] fix: ensure audio view fully renders before proceeding Added a short wait after verifying the loading bar's absence to ensure the AudioView has completely rendered. This change addresses potential flakiness in tests caused by premature assertions. --- web/libs/frontend-test/src/helpers/LSF/AudioView.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts index 3f38d561895a..a76815f1bfc7 100644 --- a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts +++ b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts @@ -37,8 +37,9 @@ export const AudioView = { return this.root.get("loading-progress-bar", { timeout: 10000 }); }, isReady() { - this.loadingBar.should("not.exist"); LabelStudio.waitForObjectsReady(); + this.loadingBar.should("not.exist"); + cy.wait(32); // wait for render }, get playButton() { return cy.get(`[data-testid="playback-button:play"]`); From 59fdfe0b5fe52fc1344d6d01128ca9b9ec01de2f Mon Sep 17 00:00:00 2001 From: Gondragos Date: Wed, 5 Mar 2025 17:44:50 +0000 Subject: [PATCH 6/8] test: Revert changes and add comments --- .../integration/e2e/audio/audio_regions.cy.ts | 28 +++++++++---------- .../src/helpers/LSF/AudioView.ts | 9 +++++- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts b/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts index 857afc1f05df..e62161ce48a5 100644 --- a/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts +++ b/web/libs/editor/tests/integration/e2e/audio/audio_regions.cy.ts @@ -12,17 +12,17 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); AudioView.clickAtRelative(0.38, 0.5); - const selectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const selectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); selectedRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); }); // unselecting cy.get("body").type("{esc}"); - const unselectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const unselectedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); unselectedRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); }); @@ -38,11 +38,11 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); // moving the cursor AudioView.seekCurrentTimebox(38); - const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); activeRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -50,7 +50,7 @@ describe("Audio regions", () => { // deactivating AudioView.seekCurrentTimebox(0); - const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); inactiveRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); @@ -67,7 +67,7 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); // highlighting in relations mode Labels.select("Music"); @@ -77,7 +77,7 @@ describe("Audio regions", () => { AudioView.hoverAtRelative(0.4, 0.5); - const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); highlightedRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -86,7 +86,7 @@ describe("Audio regions", () => { // unhighlighting AudioView.container.trigger("mouseleave"); - const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); unhighlightedRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); @@ -103,7 +103,7 @@ describe("Audio regions", () => { LabelStudio.waitForObjectsReady(); AudioView.isReady(); - const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const baseRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); // highlighting in relations mode Labels.select("Music"); @@ -113,7 +113,7 @@ describe("Audio regions", () => { AudioView.hoverAtRelative(0.4, 0.5); - const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const highlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); highlightedRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -121,7 +121,7 @@ describe("Audio regions", () => { // moving the cursor AudioView.seekCurrentTimebox(38); - const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const activeRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); activeRegionColor.then((color) => { baseRegionColor.should("not.deep.equal", color); @@ -130,7 +130,7 @@ describe("Audio regions", () => { // deactivating AudioView.seekCurrentTimebox(0); - const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const inactiveRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); // should still be highlighted inactiveRegionColor.then((color) => { @@ -141,7 +141,7 @@ describe("Audio regions", () => { // unhighlighting AudioView.container.trigger("mouseleave"); - const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.85); + const unhighlightedRegionColor = AudioView.getPixelColorRelative(0.36, 0.9); unhighlightedRegionColor.then((color) => { baseRegionColor.should("deep.equal", color); diff --git a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts index a76815f1bfc7..43181ee75046 100644 --- a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts +++ b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts @@ -39,7 +39,14 @@ export const AudioView = { isReady() { LabelStudio.waitForObjectsReady(); this.loadingBar.should("not.exist"); - cy.wait(32); // wait for render + /** + * In `AudioUltra` version of the `Audio` tag isReady was missed. + * It's fixed right now, but it was done in the easiest way it could be done. + * In that case, there is still a time gap between setting `isReady` to `true` and getting the last initial draw at the canvas, + * which for now we are going to compensate by waiting approximately 2 frames of render (16 * 2 = 32 milliseconds) + * @todo: remove wait when `isReady` in audio become more precise + */ + cy.wait(32); }, get playButton() { return cy.get(`[data-testid="playback-button:play"]`); From 5d0c7ca3db73b1ce60fe0a84648321a18620bd02 Mon Sep 17 00:00:00 2001 From: Gondragos Date: Thu, 6 Mar 2025 19:46:23 +0000 Subject: [PATCH 7/8] fix: Revert isReady redefining It looks like it was not the problem for tests. --- web/libs/editor/src/tags/object/AudioUltra/model.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/web/libs/editor/src/tags/object/AudioUltra/model.js b/web/libs/editor/src/tags/object/AudioUltra/model.js index b56aeaa16988..a7295dba650e 100644 --- a/web/libs/editor/src/tags/object/AudioUltra/model.js +++ b/web/libs/editor/src/tags/object/AudioUltra/model.js @@ -137,9 +137,6 @@ export const AudioModel = types.compose( _wfFrame: null, })) .views((self) => ({ - get isReady() { - return !!self._ws; - }, get hasStates() { const states = self.states(); From 474ab71583d5ba87165d1a84babae303f7a09f50 Mon Sep 17 00:00:00 2001 From: Gondragos Date: Thu, 6 Mar 2025 19:48:54 +0000 Subject: [PATCH 8/8] fix: adjust comment regarding `isReady` timing in AudioView --- web/libs/frontend-test/src/helpers/LSF/AudioView.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts index 43181ee75046..a4331a890367 100644 --- a/web/libs/frontend-test/src/helpers/LSF/AudioView.ts +++ b/web/libs/frontend-test/src/helpers/LSF/AudioView.ts @@ -40,9 +40,7 @@ export const AudioView = { LabelStudio.waitForObjectsReady(); this.loadingBar.should("not.exist"); /** - * In `AudioUltra` version of the `Audio` tag isReady was missed. - * It's fixed right now, but it was done in the easiest way it could be done. - * In that case, there is still a time gap between setting `isReady` to `true` and getting the last initial draw at the canvas, + * There is a time gap between setting `isReady` to `true` and getting the last initial draw at the canvas, * which for now we are going to compensate by waiting approximately 2 frames of render (16 * 2 = 32 milliseconds) * @todo: remove wait when `isReady` in audio become more precise */