fix: update slider and progress modifiers to better handle changes wh…

…en the url changes
jkeen · Apr 17, 2023 · 235d563 · 235d563
1 parent 4f4d695
commit 235d563
Show file tree

Hide file tree

Showing 15 changed files with 235 additions and 50 deletions.
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -7,7 +7,7 @@ on:
 jobs:
   release:
     name: Release
-    runs-on: ubuntu-18.04
+    runs-on: ubuntu-latest
     steps:
       - name: Checkout
         uses: actions/checkout@v2

diff --git a/addon/modifiers/sound-position-progress.js b/addon/modifiers/sound-position-progress.js
@@ -12,7 +12,6 @@
 import { action } from '@ember/object';
 import { inject as service } from '@ember/service';
 import Modifier from 'ember-modifier';
-import { once } from '@ember/runloop';
 import { registerDestructor } from '@ember/destroyable';
 
 export default class SoundPositionProgressModifier extends Modifier {
@@ -28,11 +27,15 @@ export default class SoundPositionProgressModifier extends Modifier {
     return this.stereo.findLoadedSound(this.url);
   }
 
-  @action onPositionChange() {
-    once(this, this.modifyPosition, ...arguments, 500);
+  @action
+  onPositionChange({ sound }) {
+    if (sound.hasUrl(this.url)) {
+      this.modifyPosition(...arguments);
+    }
   }
 
   modifyPosition({ newPosition }) {
+    newPosition = newPosition || this.loadedSound.position;
     this.element.style.width = `${
       ((newPosition || this.loadedSound.position) / this.loadedSound.duration) *
       100
@@ -41,37 +44,34 @@ export default class SoundPositionProgressModifier extends Modifier {
   }
 
   modify(element, [url], options) {
+    this.url = url;
+    this.options = options;
+
     if (!this.element) {
       this.element = element;
-      this.url = url;
-      this.options = options;
-
       this.element.setAttribute('data-sound-position-progress', true);
-    }
 
-    if (this.url) {
-      this.stereo.soundProxy(this.url).afterLoad((sound) => {
-        sound.on(
-          'audio-position-will-change',
-          this.onPositionChange.bind(this)
-        );
-        sound.on('audio-position-changed', this.onPositionChange.bind(this));
-      });
+      this.stereo.on(
+        'audio-position-will-change',
+        this.onPositionChange.bind(this)
+      );
+      this.stereo.on(
+        'audio-position-changed',
+        this.onPositionChange.bind(this)
+      );
     }
   }
 
   unregisterListeners() {
     try {
-      if (this.loadedSound) {
-        this.loadedSound.off(
-          'audio-position-changed',
-          this.onPositionChange.bind(this)
-        );
-        this.loadedSound.off(
-          'audio-position-will-change',
-          this.onPositionChange.bind(this)
-        );
-      }
+      this.stereo.off(
+        'audio-position-changed',
+        this.onPositionChange.bind(this)
+      );
+      this.stereo.off(
+        'audio-position-will-change',
+        this.onPositionChange.bind(this)
+      );
     } catch (e) {
       /* geez, relax */
     }

diff --git a/addon/modifiers/sound-position-slider.js b/addon/modifiers/sound-position-slider.js
@@ -26,7 +26,7 @@ export default class SoundPositionSliderModifier extends DidPanModifier {
   }
 
   get loadedSound() {
-    return this.stereo.findSound(this.url);
+    return this.stereo.findLoadedSound(this.url);
   }
 
   get isRangeControl() {
@@ -57,7 +57,6 @@ export default class SoundPositionSliderModifier extends DidPanModifier {
   @action
   handleTap(e) {
     e.preventDefault();
-
     var rect = this.element.getBoundingClientRect();
     var x = e.clientX - rect.left; //x position within the element.
 
@@ -81,29 +80,14 @@ export default class SoundPositionSliderModifier extends DidPanModifier {
 
   modify(element, [url], options) {
     if (!this.element) {
-      this.url = url;
-      this.options = options;
       this.element = element;
+      this.options = options;
 
       if (this.isRangeControl) {
         this.element.setAttribute('max', 100);
         this.element.setAttribute('min', 0);
         this.element.setAttribute('value', 0);
         this.element.setAttribute('disabled', 'disabled');
-
-        this.afterLoadTask
-          .perform((sound) => {
-            sound.on(
-              'audio-position-changed',
-              this.onPositionChange.bind(this)
-            );
-
-            this.element.addEventListener('change', this.onChange, true);
-            if (sound.isSeekable) {
-              this.element.removeAttribute('disabled');
-            }
-          })
-          .catch(() => {});
       } else {
         this.element.addEventListener('click', this.handleTap);
         this.element.addEventListener('mousedown', this.handleTap);
@@ -112,6 +96,29 @@ export default class SoundPositionSliderModifier extends DidPanModifier {
       this.element.setAttribute('data-sound-position-slider', true);
     }
 
+    if (this.isRangeControl && this.url !== url) {
+      this.url = url;
+
+      if (this.loadedSound) {
+        this.loadedSound.off(
+          'audio-position-changed',
+          this.onPositionChange.bind(this)
+        );
+      }
+      this.afterLoadTask
+        .perform((sound) => {
+          sound.on('audio-position-changed', this.onPositionChange.bind(this));
+
+          this.element.addEventListener('change', this.onChange, true);
+          if (sound.isSeekable) {
+            this.element.removeAttribute('disabled');
+          }
+        })
+        .catch(() => {});
+    } else if (this.url !== url) {
+      this.url = url;
+    }
+
     if (!this.isRangeControl) {
       super.removeEventListeners();
 

diff --git a/addon/stereo-connections/native-audio.js b/addon/stereo-connections/native-audio.js
@@ -18,6 +18,8 @@ const AUDIO_EVENTS = [
   'playing',
   'pause',
   'ended',
+  'seeking',
+  'seeked',
   'emptied',
   'timeupdate',
 ];
@@ -122,9 +124,8 @@ export default class NativeAudio extends BaseSound {
       case 'ended':
         this._onAudioEnded();
         break;
+      case 'seeking':
       case 'seeked':
-        this._onPositionChange();
-        break;
       case 'timeupdate':
         this._onPositionChange();
         break;

diff --git a/tests/dummy/app/components/docs/stereo-player-audio-debug.hbs b/tests/dummy/app/components/docs/stereo-player-audio-debug.hbs
@@ -0,0 +1,6 @@
+{{yield}}
+
+<div
+  class='stereo-player-debug-element'
+  {{did-insert this.registerDebugArea this}}
+></div>
diff --git a/tests/dummy/app/components/docs/stereo-player-audio-debug.js b/tests/dummy/app/components/docs/stereo-player-audio-debug.js
@@ -0,0 +1,25 @@
+import Component from '@glimmer/component';
+import { action } from '@ember/object';
+import { inject as service } from '@ember/service';
+export default class DocsStereoPlayerAudioDebugComponent extends Component {
+  @service stereo;
+
+  element = null;
+  constructor() {
+    super(...arguments);
+    this.stereo.on('audio-loaded', this.onAudioLoaded.bind(this));
+  }
+
+  @action
+  onAudioLoaded({ sound }) {
+    if (sound.hasUrl(this.args.identifier) && this.element) {
+      // sound.audioElement.setAttribute('controls', true);
+      // this.element.appendChild(sound.audioElement);
+    }
+  }
+
+  @action
+  registerDebugArea(element) {
+    this.element = element;
+  }
+}
diff --git a/tests/dummy/app/components/docs/stereo-player-changer.hbs b/tests/dummy/app/components/docs/stereo-player-changer.hbs
@@ -0,0 +1,3 @@
+<Docs::StereoPlayerModifiers @identifier={{this.identifier}} />
+
+<button type='button' {{on 'click' this.pickNextIdentifier}}>Next</button>
diff --git a/tests/dummy/app/components/docs/stereo-player-changer.js b/tests/dummy/app/components/docs/stereo-player-changer.js
@@ -0,0 +1,17 @@
+import Component from '@glimmer/component';
+import { inject as service } from '@ember/service';
+import { action } from '@ember/object';
+import { tracked } from '@glimmer/tracking';
+
+export default class StereoPlayerChanger extends Component {
+  @service stereo;
+  @tracked sound;
+  @tracked identifier = this.args.identifiers[0];
+
+  @action
+  async pickNextIdentifier() {
+    const index = this.args.identifiers.indexOf(this.identifier);
+    const nextIndex = (index + 1) % this.args.identifiers.length;
+    this.identifier = this.args.identifiers[nextIndex];
+  }
+}
diff --git a/tests/dummy/app/components/docs/stereo-player-modifiers.hbs b/tests/dummy/app/components/docs/stereo-player-modifiers.hbs
@@ -0,0 +1,107 @@
+{{! BEGIN-SNIPPET stereo-player.hbs }}
+<div class='stereo-player'>
+  <div class='stereo-player-main'>
+    <div class='stereo-player-controls'>
+      <button
+        {{on 'click' (rewind-sound @identifier increment=5000)}}
+        disabled={{not (sound-is-rewindable @identifier)}}
+        type='button'
+        class='stereo-button'
+      >
+        {{svg-jar
+          'skip-backward-outline'
+          height=20
+          width=30
+          title='Skip Backward'
+        }}
+      </button>
+
+      <button
+        {{on 'click' (toggle-play-sound @identifier)}}
+        type='button'
+        class='stereo-button'
+      >
+        {{#if (sound-is-playing @identifier)}}
+          {{svg-jar 'pause-outline' height=20 width=30 title='Pause'}}
+        {{else if (sound-is-loading @identifier)}}
+          {{svg-jar
+            'loading-outline'
+            height=20
+            width=30
+            title='Loading'
+            class='animate-spin'
+          }}
+        {{else if (sound-is-loaded @identifier)}}
+          {{svg-jar 'resume-outline' height=20 width=30 title='Pause'}}
+        {{else}}
+          {{svg-jar 'play-outline' height=20 width=30 title='Play'}}
+        {{/if}}
+      </button>
+
+      <button
+        {{on 'click' (fastforward-sound @identifier increment=5000)}}
+        disabled={{not (sound-is-fastforwardable @identifier)}}
+        type='button'
+        class='stereo-button'
+      >
+        {{svg-jar
+          'skip-forward-outline'
+          height=20
+          width=30
+          title='Skip Forward'
+        }}
+      </button>
+    </div>
+
+    <div
+      class='stereo-player-position h-2 rounded-md bg-on-surface-3 inset-1'
+      {{sound-position-slider @identifier}}
+    >
+      {{#if (sound-is-seekable @identifier)}}
+        <div
+          class='h-2 rounded-md bg-accent-1 inset-1'
+          data-test-element='progress'
+          {{sound-position-progress @identifier}}
+        ></div>
+      {{else}}
+        <div
+          class='{{if
+              (sound-is-playing @identifier)
+              "from-accent-0 to-accent-2 bg-gradient-to-r animate-gradient-x animated"
+              "bg-on-surface-3"
+            }}
+            w-full h-2 rounded-md inset-1 shadow-inset'
+        >
+        </div>
+      {{/if}}
+    </div>
+
+    {{! BEGIN-SNIPPET stereo-volume-control.hbs }}
+    <div class='stereo-player-volume'>
+      <Input
+        {{stereo-volume}}
+        id='stereo-volume'
+        @type='range'
+        class='flex-auto flex-shrink w-16 lg:w-1/6'
+      />
+      <label for='stereo-volume' class='stereo-volume'>
+        <button type='button' {{on 'click' (toggle-stereo-mute)}}>
+          {{#if (gt (stereo-volume) 0)}}
+            {{svg-jar 'volume-low-outline' height=20 width=30}}
+          {{else}}
+            {{svg-jar 'volume-mute-outline' height=20 width=30}}
+          {{/if}}
+        </button>
+      </label>
+    </div>
+    {{! END-SNIPPET }}
+  </div>
+
+  {{#if (sound-is-errored @identifier)}}
+    <div class='stereo-player-errors'>
+      {{sound-error-details @identifier}}
+    </div>
+  {{/if}}
+</div>
+
+{{! END-SNIPPET }}
diff --git a/tests/dummy/app/components/docs/stereo-player.hbs b/tests/dummy/app/components/docs/stereo-player.hbs
@@ -105,4 +105,5 @@
     </div>
   {{/if}}
 </div>
+
 {{! END-SNIPPET }}
diff --git a/tests/dummy/app/router.js b/tests/dummy/app/router.js
@@ -29,6 +29,7 @@ Router.map(function () {
   this.route('diagnostic', function () {
     this.route('index');
     this.route('sync');
+    this.route('players');
   });
 
   this.route('autoplay-test');

diff --git a/tests/dummy/app/routes/diagnostic/players.js b/tests/dummy/app/routes/diagnostic/players.js
@@ -0,0 +1,3 @@
+import Route from '@ember/routing/route';
+
+export default class Players extends Route {}
diff --git a/tests/dummy/app/templates/diagnostic.hbs b/tests/dummy/app/templates/diagnostic.hbs
@@ -5,9 +5,11 @@
 </DocsHeader>
 <DocsViewer as |viewer|>
   <viewer.nav @media={{this.media}} as |nav|>
-    <nav.section @label='Diagnostic'/>
-    <nav.item @label='Main' @route='diagnostic.index'/>
-    <nav.item @label="Sync" @route='diagnostic.sync'/>
+    <nav.section @label='Diagnostic' />
+    <nav.item @label='Main' @route='diagnostic.index' />
+    <nav.item @label='Sync' @route='diagnostic.sync' />
+    <nav.item @label='Players' @route='diagnostic.players' />
+
   </viewer.nav>
 
   <viewer.main @media={{this.media}}>