fix: LSDV-3012: Syncable with new simple approach (#1201)

New Syncable mixin to provide new and very simple and nice approach:

- don't send event back
- sync the whole state (time, speed, playing)
- group tags into syncing groups
- lock current sync window for any back-stream events
- sync is pluggable thing, tag should work without sync methods (Audio is still tied with Sync)

All tags are muted except Audio tags or the tag triggered even if no Audio is presented. That's because Audio is the only tag with controllable volume. May be changed in the future.

Things done:

- sync speed
- sync play when audio restarted after reaching the end
- sync time shortly after play? can be helpful for overloaded browser



* fix: LSDV-3012: Syncable with new simple approach

First try with some debugging and thoughts.
New Syncable mixin to provide new and very simple and nice approach:
- don't send event back
- sync the whole state (time, speed, playing)
- group tags in syncing groups
- lock current frame for any back-stream events

* Fix annoying react error about missing key

* Fix types in SyncableMixin

* adding some of the sync tests, fixing audio pause time sync

* make sync optional, otherwise it breaks everything else

* remove debug wait

* adding test for synced looping audio

* Sync Video; remove excess sync deps; use sync window

- Implement Syncable for Video as well
- Remove some dependencies on sync methods from usual flow
- Add sync window to suppress all excess events in it
- Sync states, not events

* Do nothing if tag is not synced

* Sync Paragraphs

- rename model and view to be more consistent
- use Syncable, implement just a few methods, required minimum
- remove bunch of old code with sync misuse
- Paragrpahs mostly sync other tags, and on external actions it only pauses

* Fix time displaying during synced seek

* Clean up; comments; data is main parameter

* update test usage of helper dragAudioElement

* fixing audio paragraphs test

* not sure why this file was suddenly uppercase, putting back to lower

* use a local file for test

* allow alternative chromium to be used, otherwise the default install from playwright contains no media codecs to support mp4 files

* put back the correct defaults for examples

* use video file for example

* use video for file ref in audio,video,paragraphs example config data

* Fix backward compatibility for video<->audio sync

Old way of referring each other names works!

* Sync speed as well

* fix video and audio references in sync tests, ensure video is not leaving leftover video elements

* use only audio v3 for all tests and development

* fixing audio config tests

* fix paragraph syncing of playback, and end time handling along with test fixes

* Mute other tags on play. And unmute the current one.

Paragraphs tag doesn't react on play, so should not be muted.
And when it's the origin of event it's not muted and others are.
So no handlers there.

* Fix muted for Video

* Fix Paragraphs sync handling

React on seek if the time within the current phrase.
Also helps with late back-sync from video tag.

* Fix renamed test helper dragAudioElement

* Fix Audio -> Video sync misteriously broken

* fix DomException when looping playback which also caused tests to fail

* ensure fields are cleared before input is sent for audio controls

* playback speed needs to visually update

* Fix tests for Audio v3

* Record video of weird readonly test failure

* Fix readonly audio regions test

Behaviour was changed, so let it check the new one.
Also fix for hotkeys enabled for tests.

* Raise tests timeout

* Ignore errors temporarily

* Return back usual codecept values

* Fix sync not triggerring when speed is 1

Because of early static handlers on waveform, rate was fixed at 1 in it.

* Fix speed input change in test

* Add threshold to time comparisons

* Ignore DOMException, it won't affect UX

This error should've been fixed, but stayed in some cases :(

* Move synced time assert to fragment; fix test

* Fix case for default overrides in flags.json

* Trigger play only after it actually started to play

This fixes problems with sync of region looped playing

* Actually, DEV is correct one for old Audio FF

* Fix sync for Audio v1/v2

* Clean up Paragraphs code, simplify and add comments

* Add tests for Paragraphs mst model methods

* Fix audio when synced tag jumped to the beginning

* Fix muted state

Only Audio has volume controls, so they should not be muted,
while other synced tags should be muted, otherwise volume can't
be controlled.
But if there are no Audio tags in group, the tag triggered sync
should be the main tag with volume active, and others should be muted.

* Fix Paragraphs UX: play/pause synced and natural

If you press pause and then play it will continue playing current region,
even if that was triggered from other tag.
Seek is synced inside current region.

* Fix Paragraphs with overlapping times

* Add feature flag

All methods are groupped even in old code, so these groups are extracted
to separate small models and composed conditionally by ff value.

* Add more comments about setMuted behavior

* Fix wrong flags combination

* Use new FF in e2e tests

* Move assertTimesInSync to Assertion helper

* Fix unit test with some weird FF mock

---------

Co-authored-by: Brandon Martel <brandonmartel@gmail.com>
Co-authored-by: Sergey <sergey.koshevarov@heartex.com>

Author: 3 people

e2e/codecept.conf.js

@@ -26,6 +26,9 @@ module.exports.config = {
       windowSize: '1200x900',
       waitForNavigation: 'networkidle',
       browser: 'chromium',
+      chromium: process.env.CHROMIUM_EXECUTABLE_PATH ? {
+        executablePath: process.env.CHROMIUM_EXECUTABLE_PATH,
+      } : {},
       trace: false,
       keepTraceForPassedTests: false,
     },
@@ -96,6 +99,12 @@ module.exports.config = {
       enabled: true,
       uncaughtErrorFilter: {
         interrupt: true,
+        ignore: [
+          /^ResizeObserver loop limit exceeded$/,
+          // @todo: solve the problems below
+          /^TypeError: Cannot read properties of null \(reading 'getBoundingClientRect'\)/,
+          /The play\(\) request was interrupted/,
+        ],
       },
       consoleErrorFilter: {
         // @todo switch it on to feel the pain

e2e/fragments/AtAudioView.js

@@ -35,20 +35,20 @@ module.exports = {
   async waitForAudio() {
     await I.executeScript(Helpers.waitForAudio);
     I.waitForInvisible(this._progressBarSelector);
+    I.waitForDetached('loading-progress-bar', 10);
   },
-
-  getCurrentAudioTime() {
-    return I.executeScript(Helpers.getCurrentAudioTime);
+  getCurrentAudio() {
+    return I.executeScript(Helpers.getCurrentMedia, 'audio');
   },
   /**
    * Mousedown - mousemove - mouseup drawing on the AudioView. Works in couple of lookForStage.
    * @example
    * await AtAudioView.lookForStage();
-   * AtAudioView.dragAudioRegion(50, 200);
+   * AtAudioView.dragAudioElement(50, 200);
    * @param x {number}
    * @param shiftX {number}
    */
-  dragAudioRegion(x, shiftX, shouldRelease = true) {
+  dragAudioElement(x, shiftX, shouldRelease = true) {
     I.scrollPageToTop();
     I.moveMouse(this._stageBbox.x + x, this._stageBbox.y + this._stageBbox.height / 2);
     I.pressMouseDown();
@@ -73,6 +73,17 @@ module.exports = {
     I.wait(1); // We gotta  wait here because clicks on the canvas are not processed immediately
   },
 
+  clickAtBeginning() {
+    this.clickAt(0);
+  },
+
+  clickAtEnd() {
+    // Clicking on the end of the canvas doesn't quite work, so we click a bit before the end
+    // to make sure we're it is not clicking outside the canvas, and move the cursor over.
+    this.clickAt(this._stageBbox.width - 1);
+    this.dragAudioElement(this._stageBbox.width - 1, 1);
+  },
+
   async createRegion(tagName, start, length) {
     const { x, y, height } = await this.getWrapperPosition(tagName);
 
@@ -87,7 +98,9 @@ module.exports = {
 
   async getWrapperPosition(tagName) {
     const wrapperPosition = await I.executeScript((tagName) => {
-      const wrapper = Htx.annotationStore.selected.names.get(tagName)._ws.container;
+      const _ws = Htx.annotationStore.selected.names.get(tagName)._ws;
+      // `visualizer.wrapper` is for Audio v3
+      const wrapper = _ws.visualizer?.wrapper ?? _ws.container;
       const bbox = wrapper.getBoundingClientRect();
 
       return {
@@ -119,6 +132,31 @@ module.exports = {
     });
   },
 
+  async moveRegionV3(regionId, offset = 30) {
+    const regionPosition = await I.executeScript(({ regionId, stageBbox }) => {
+      const region = Htx.annotationStore.selected.regions.find(r => r.cleanId === regionId);
+
+      const wsRegion = region._ws_region;
+
+      if (!wsRegion.inViewport) {
+        return null;
+      }
+      const { height } = wsRegion.visualizer;
+
+      return {
+        x: (wsRegion.xStart + wsRegion.xEnd) / 2 + stageBbox.x,
+        y: height / 2 + stageBbox.y,
+      };
+    }, { regionId, stageBbox: this._stageBbox });
+
+    if (!regionPosition) return;
+
+    return I.dragAndDropMouse(regionPosition, {
+      x: regionPosition.x + offset,
+      y: regionPosition.y,
+    });
+  },
+
   /**
    * Toggle the audio control menu. This houses the volume slider and mute button.
    */
@@ -193,6 +231,7 @@ module.exports = {
    */
   setVolumeInput(value) {
     this.toggleControlsMenu();
+    I.clearField(this._volumeInputSelector);
     I.fillField(this._volumeInputSelector, value);
     this.toggleControlsMenu();
   },
@@ -210,6 +249,7 @@ module.exports = {
 
     I.seeInField(this._playbackSpeedInputSelector, value);
     I.seeInField(this._playbackSpeedSliderSelector, value);
+
     const playbackSpeed = await I.grabAttributeFrom(this._audioElementSelector, 'playbackRate');
 
     assert.equal(playbackSpeed, value, 'Playback speed doesn\'t match in audio element');
@@ -226,6 +266,9 @@ module.exports = {
    */
   setPlaybackSpeedInput(value) {
     this.toggleSettingsMenu();
+    // it was not easy to set this field, so we have to carefully remove value and put it
+    I.doubleClick(locate(this._playbackSpeedInputSelector));
+    I.pressKey('Backspace');
     I.fillField(this._playbackSpeedInputSelector, value);
     this.toggleSettingsMenu();
   },
@@ -256,6 +299,7 @@ module.exports = {
    */
   setAmplitudeInput(value) {
     this.toggleSettingsMenu();
+    I.clearField(this._amplitudeInputSelector);
     I.fillField(this._amplitudeInputSelector, value);
     this.toggleSettingsMenu();
   },

e2e/fragments/AtVideoView.js

@@ -1,5 +1,7 @@
 const { I } = inject();
 
+const Helpers = require('../tests/helpers');
+
 /**
  * @typedef BoundingClientRect
  * @property {number} x
@@ -16,6 +18,11 @@ module.exports = {
   _positionSelector: '.lsf-seeker__position',
   _seekStepForwardSelector: '.lsf-timeline-controls__main-controls > div:nth-child(2) > button:nth-child(4)',
   _seekStepBackwardSelector: '.lsf-timeline-controls__main-controls > div:nth-child(2) > button:nth-child(2)',
+  _playButtonSelector: '.lsf-timeline-controls__main-controls > .lsf-timeline-controls__group:nth-child(2) > button:nth-child(2)',
+
+  locateRootSelector() {
+    return locate(this._rootSelector);
+  },
 
   locateVideoContainer() {
     return locate(this._videoRootSelector);
@@ -25,6 +32,22 @@ module.exports = {
     return locator ? locate(locator).inside(this.locateVideoContainer()) : this.locateVideoContainer();
   },
 
+  seekStepForwardSelector() {
+    return locate(this._seekStepForwardSelector).inside(this.locateRootSelector());
+  },
+
+  seekStepBackwardSelector() {
+    return locate(this._seekStepBackwardSelector).inside(this.locateRootSelector());
+  },
+
+  playButtonSelector() {
+    return locate(this._playButtonSelector).inside(this.locateRootSelector());
+  },
+
+  getCurrentVideo() {
+    return I.executeScript(Helpers.getCurrentMedia, 'video');
+  },
+
   /**
    * Grab the bounding rect of the video track
    * @returns {Promise<BoundingClientRect>}
@@ -70,7 +93,7 @@ module.exports = {
    */
   async clickSeekStepForward(steps = 1) {
     for (let i = 0; i < steps; i++) {
-      I.click(this._seekStepForwardSelector);
+      I.click(this.seekStepForwardSelector());
     }
   },
 
@@ -79,9 +102,25 @@ module.exports = {
    * @param {number} steps
    * @returns {Promise<void>}
    */
-  async clickSeekStepBackward(steps = 1) {
+  async clickSeekStepBackward(steps = 2) {
     for (let i = 0; i < steps; i++) {
-      I.click(this._seekStepBackwardSelector);
+      I.click(this.seekStepBackwardSelector());
     }
   },
+
+  /**
+   * Click the video controls play button.
+   * @returns {Promise<void>}
+   */
+  async clickPlayButton() {
+    I.click(this.playButtonSelector());
+  },
+
+  /**
+   * Click the video controls pause button.
+   * @returns {Promise<void>}
+   */
+  async clickPauseButton() {
+    I.click(this.playButtonSelector());
+  },
 };

e2e/helpers/Assertion.js

@@ -2,6 +2,9 @@ const Helpers = require('../tests/helpers');
 const assert = require('assert');
 const Helper = require('@codeceptjs/helper');
 
+// used in Audio/Video/Paragraphs sync
+const TIME_DIFF_THRESHOLD = 0.3;
+
 class AssertionHelper extends Helper {
   assertDeepEqualWithTolerance(actual, expected, fractionDigits = 2, message) {
     assert.deepStrictEqual(
@@ -17,6 +20,15 @@ class AssertionHelper extends Helper {
       message,
     );
   }
+  /**
+   * Asserts that two times are equal after sync (with some possible threshold)
+   * @param {number} time1
+   * @param {number} time2
+   * @param {string} [message] for assertion
+   */
+  assertTimesInSync(time1, time2, message = '') {
+    assert.equal(Math.abs(time1 - time2) < TIME_DIFF_THRESHOLD, true, message);
+  }
 }
 
 module.exports = AssertionHelper;

e2e/setup/feature-flags.js

@@ -1,7 +1,8 @@
-/*
-  List of feature flags that should be switched on during every testing scenario
- */
-
-module.exports = {
-  fflag_feat_front_lsdv_4659_skipduplicates_060323_short: true,
-};
+/*
+  List of feature flags that should be switched on during every testing scenario
+ */
+
+module.exports = {
+  fflag_feat_front_lsdv_4659_skipduplicates_060323_short: true,
+  fflag_fix_font_lsdv_1148_hotkeys_namespaces_01022023_short: true,
+};

e2e/tests/audio/audio-controls.test.js

@@ -67,9 +67,6 @@ Scenario('Check the audio controls work', async function({ I, LabelStudio, AtAud
   LabelStudio.init(params);
 
   await AtAudioView.waitForAudio();
-
-  I.waitForDetached('loading-progress-bar', 10);
-
   await AtAudioView.lookForStage();
 
   AtSidebar.seeRegions(1);

e2e/tests/audio/audio-regions.test.js

@@ -82,16 +82,13 @@ Scenario('Check if regions are selected', async function({ I, LabelStudio, AtAud
   LabelStudio.init(params);
 
   await AtAudioView.waitForAudio();
-
-  I.waitForDetached('loading-progress-bar', 10);
-
   await AtAudioView.lookForStage();
 
   AtSidebar.seeRegions(1);
 
   // creating a new region
   I.pressKey('1');
-  AtAudioView.dragAudioRegion(160,80);
+  AtAudioView.dragAudioElement(160,80);
   I.pressKey('u');
 
   AtSidebar.seeRegions(2);
@@ -100,7 +97,7 @@ Scenario('Check if regions are selected', async function({ I, LabelStudio, AtAud
   AtSidebar.seeSelectedRegion();
   AtAudioView.clickAt(170);
   AtSidebar.dontSeeSelectedRegion();
-  AtAudioView.dragAudioRegion(170,40);
+  AtAudioView.dragAudioElement(170,40);
   AtSidebar.seeSelectedRegion();
   AtAudioView.clickAt(220);
   AtSidebar.dontSeeSelectedRegion();
@@ -115,15 +112,12 @@ Scenario('Check if multiple regions are working changing labels', async function
   LabelStudio.init(paramsSpeech);
 
   await AtAudioView.waitForAudio();
-
-  I.waitForDetached('loading-progress-bar', 10);
-
   await AtAudioView.lookForStage();
 
   for (let i = 0; i < 20; i++) {
     // creating a new region
     I.pressKey('1');
-    AtAudioView.dragAudioRegion((40 * i) + 10,30);
+    AtAudioView.dragAudioElement((40 * i) + 10,30);
     AtAudioView.clickAt((40 * i) + 20);
     I.pressKey('2');
     I.pressKey('1');
@@ -155,21 +149,18 @@ Scenario('Can select a region below a hidden region', async function({ I, LabelS
   LabelStudio.init(paramsSpeech);
 
   await AtAudioView.waitForAudio();
-
-  I.waitForDetached('loading-progress-bar', 10);
-
   await AtAudioView.lookForStage();
 
   // create a new region
   I.pressKey('1');
-  AtAudioView.dragAudioRegion(50, 80);
+  AtAudioView.dragAudioElement(50, 80);
   I.pressKey('u');
 
   AtSidebar.seeRegions(1);
 
   // create a new region above the first one
   I.pressKey('2');
-  AtAudioView.dragAudioRegion(49, 81);
+  AtAudioView.dragAudioElement(49, 81);
   I.pressKey('u');
 
   AtSidebar.seeRegions(2);
@@ -195,15 +186,12 @@ Scenario('Delete region by pressing delete hotkey', async function({ I, LabelStu
   LabelStudio.init(params);
 
   await AtAudioView.waitForAudio();
-
-  I.waitForDetached('loading-progress-bar', 10);
-
   await AtAudioView.lookForStage();
 
   AtSidebar.seeRegions(1);
 
   // creating a new region
-  AtAudioView.dragAudioRegion(160,80);
+  AtAudioView.dragAudioElement(160,80);
 
   I.pressKey('Delete');
 
@@ -221,20 +209,17 @@ Scenario('Check if there are ghost regions', async function({ I, LabelStudio, At
   LabelStudio.init(paramsSpeech);
 
   await AtAudioView.waitForAudio();
-
-  I.waitForDetached('loading-progress-bar', 10);
-
   await AtAudioView.lookForStage();
 
   // creating a new region
   I.pressKey('1');
-  AtAudioView.dragAudioRegion(300,80);
+  AtAudioView.dragAudioElement(300,80);
   I.pressKey('u');
 
 
   // creating a ghost region
   I.pressKey('1');
-  AtAudioView.dragAudioRegion(160,80, false);
+  AtAudioView.dragAudioElement(160,80, false);
   I.pressKey('1');
   I.wait(1);
   I.pressMouseUp();