mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-01 20:42:50 +08:00
fix(web): keep the volume slider draggable under the per-frame progress loop (#111)
The 60fps requestAnimationFrame progress clock (#107) re-renders the player every ~16ms, and Vue re-applied `el.value = storeVolume` on a range input each time — snapping the thumb back to the stale store value mid-drag (un-draggable on desktop, janky on mobile). Extract the decoupling into a useDecoupledSlider composable used by both the desktop (Player.vue) and mobile (App.vue) sliders: a local display ref tracks the native drag via @input (so the bound value always matches the element), the store is committed only on @change (release), and an onRelease safety-net (pointerup/pointercancel/blur) clears the dragging guard even when the browser skips `change` (value released at its start point). External/store changes still flow into the display except while dragging. Adds a regression test for the no-snap-back invariant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
4cb1da29d4
commit
5ae5168b6b
4 files changed
+187
-14
No files matched your search
+19
-9
@@ -44,11 +44,15 @@
|
|||||||
type="range"
|
type="range"
|
||||||
min="0"
|
min="0"
|
||||||
max="100"
|
max="100"
|
||||||
:value="mobileVolume"
|
:value="mobileVolumeDisplay"
|
||||||
class="m-volume-slider"
|
class="m-volume-slider"
|
||||||
@input="onMobileVolumeChange"
|
@input="onMobileVolumeInput"
|
||||||
|
@change="onMobileVolumeCommit"
|
||||||
|
@pointerup="onMobileVolumeRelease"
|
||||||
|
@pointercancel="onMobileVolumeRelease"
|
||||||
|
@blur="onMobileVolumeRelease"
|
||||||
/>
|
/>
|
||||||
<span class="m-volume-value">{{ mobileVolume }}</span>
|
<span class="m-volume-value">{{ mobileVolumeDisplay }}</span>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
@@ -79,6 +83,7 @@ import { computed, onMounted, onUnmounted, ref } from 'vue';
|
|||||||
import { useRoute, useRouter } from 'vue-router';
|
import { useRoute, useRouter } from 'vue-router';
|
||||||
import { Icon } from '@iconify/vue';
|
import { Icon } from '@iconify/vue';
|
||||||
import { usePlayerStore } from './stores/player.js';
|
import { usePlayerStore } from './stores/player.js';
|
||||||
|
import { useDecoupledSlider } from './composables/useDecoupledSlider.js';
|
||||||
import { useWebSocket } from './composables/useWebSocket.js';
|
import { useWebSocket } from './composables/useWebSocket.js';
|
||||||
import { useSession } from './composables/useSession.js';
|
import { useSession } from './composables/useSession.js';
|
||||||
import Navbar from './components/Navbar.vue';
|
import Navbar from './components/Navbar.vue';
|
||||||
@@ -99,7 +104,17 @@ const route = useRoute();
|
|||||||
const router = useRouter();
|
const router = useRouter();
|
||||||
const { connect } = useWebSocket();
|
const { connect } = useWebSocket();
|
||||||
const currentSong = computed(() => playerStore.currentSong);
|
const currentSong = computed(() => playerStore.currentSong);
|
||||||
const mobileVolume = computed(() => playerStore.activeBot?.volume ?? 75);
|
// Volume slider decoupled from the 60fps updateMobileProgress() rAF re-render
|
||||||
|
// so it isn't reset mid-drag (#111 — same root cause as the desktop player).
|
||||||
|
const {
|
||||||
|
display: mobileVolumeDisplay,
|
||||||
|
onInput: onMobileVolumeInput,
|
||||||
|
onChange: onMobileVolumeCommit,
|
||||||
|
onRelease: onMobileVolumeRelease,
|
||||||
|
} = useDecoupledSlider(
|
||||||
|
() => playerStore.activeBot?.volume,
|
||||||
|
(v) => playerStore.setVolume(v)
|
||||||
|
);
|
||||||
const mobileMode = computed(() => playerStore.activeBot?.playMode ?? 'seq');
|
const mobileMode = computed(() => playerStore.activeBot?.playMode ?? 'seq');
|
||||||
const mobileModeOrder = ['seq', 'loop', 'random', 'rloop'];
|
const mobileModeOrder = ['seq', 'loop', 'random', 'rloop'];
|
||||||
const mobileModeIcons: Record<string, string> = {
|
const mobileModeIcons: Record<string, string> = {
|
||||||
@@ -126,11 +141,6 @@ function updateMobileProgress() {
|
|||||||
mobileRaf = requestAnimationFrame(updateMobileProgress);
|
mobileRaf = requestAnimationFrame(updateMobileProgress);
|
||||||
}
|
}
|
||||||
|
|
||||||
function onMobileVolumeChange(e: Event) {
|
|
||||||
const volume = Number((e.target as HTMLInputElement).value);
|
|
||||||
playerStore.setVolume(volume);
|
|
||||||
}
|
|
||||||
|
|
||||||
function toggleMobileVolume() {
|
function toggleMobileVolume() {
|
||||||
mobileVolumeOpen.value = !mobileVolumeOpen.value;
|
mobileVolumeOpen.value = !mobileVolumeOpen.value;
|
||||||
if (mobileVolumeOpen.value) mobileQueueOpen.value = false;
|
if (mobileVolumeOpen.value) mobileQueueOpen.value = false;
|
||||||
|
|||||||
@@ -65,8 +65,12 @@
|
|||||||
type="range"
|
type="range"
|
||||||
min="0"
|
min="0"
|
||||||
max="100"
|
max="100"
|
||||||
:value="activeBot?.volume ?? 75"
|
:value="volumeDisplay"
|
||||||
|
@input="onVolumeInput"
|
||||||
@change="onVolumeChange"
|
@change="onVolumeChange"
|
||||||
|
@pointerup="onVolumeRelease"
|
||||||
|
@pointercancel="onVolumeRelease"
|
||||||
|
@blur="onVolumeRelease"
|
||||||
class="volume-slider"
|
class="volume-slider"
|
||||||
/>
|
/>
|
||||||
</template>
|
</template>
|
||||||
@@ -87,6 +91,7 @@ import { Icon } from '@iconify/vue';
|
|||||||
import { useRoute, useRouter } from 'vue-router';
|
import { useRoute, useRouter } from 'vue-router';
|
||||||
import { usePlayerStore } from '../stores/player.js';
|
import { usePlayerStore } from '../stores/player.js';
|
||||||
import { useSession } from '../composables/useSession.js';
|
import { useSession } from '../composables/useSession.js';
|
||||||
|
import { useDecoupledSlider } from '../composables/useDecoupledSlider.js';
|
||||||
import CoverArt from './CoverArt.vue';
|
import CoverArt from './CoverArt.vue';
|
||||||
import Queue from './Queue.vue';
|
import Queue from './Queue.vue';
|
||||||
|
|
||||||
@@ -185,10 +190,17 @@ function togglePlay() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function onVolumeChange(e: Event) {
|
// Volume slider is decoupled from the per-frame rAF re-render so dragging the
|
||||||
const target = e.target as HTMLInputElement;
|
// thumb isn't reset every frame (#111). See useDecoupledSlider.
|
||||||
store.setVolume(parseInt(target.value));
|
const {
|
||||||
}
|
display: volumeDisplay,
|
||||||
|
onInput: onVolumeInput,
|
||||||
|
onChange: onVolumeChange,
|
||||||
|
onRelease: onVolumeRelease,
|
||||||
|
} = useDecoupledSlider(
|
||||||
|
() => activeBot.value?.volume,
|
||||||
|
(v) => store.setVolume(v)
|
||||||
|
);
|
||||||
|
|
||||||
const modeOrder = ['seq', 'loop', 'random', 'rloop'] as const;
|
const modeOrder = ['seq', 'loop', 'random', 'rloop'] as const;
|
||||||
const modeIcons: Record<string, string> = {
|
const modeIcons: Record<string, string> = {
|
||||||
|
|||||||
@@ -0,0 +1,84 @@
|
|||||||
|
import { describe, it, expect, vi } from "vitest";
|
||||||
|
import { ref, nextTick } from "vue";
|
||||||
|
import { useDecoupledSlider } from "./useDecoupledSlider.js";
|
||||||
|
|
||||||
|
/** Minimal stand-in for an <input type="range"> change/input Event. */
|
||||||
|
function ev(value: number): Event {
|
||||||
|
return { target: { value: String(value) } } as unknown as Event;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("useDecoupledSlider (#111)", () => {
|
||||||
|
it("initialises display from the source, falling back when undefined", () => {
|
||||||
|
const src = ref<number | undefined>(40);
|
||||||
|
const { display } = useDecoupledSlider(() => src.value, () => {});
|
||||||
|
expect(display.value).toBe(40);
|
||||||
|
|
||||||
|
const empty = useDecoupledSlider(() => undefined, () => {}, 75);
|
||||||
|
expect(empty.display.value).toBe(75);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reflects external source changes into the display when not dragging", async () => {
|
||||||
|
const src = ref<number | undefined>(50);
|
||||||
|
const { display } = useDecoupledSlider(() => src.value, () => {});
|
||||||
|
src.value = 80;
|
||||||
|
await nextTick();
|
||||||
|
expect(display.value).toBe(80);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("tracks @input locally without committing", () => {
|
||||||
|
const commit = vi.fn();
|
||||||
|
const { display, onInput } = useDecoupledSlider(() => 50, commit);
|
||||||
|
onInput(ev(63));
|
||||||
|
expect(display.value).toBe(63);
|
||||||
|
expect(commit).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
// THE REGRESSION: this is exactly what the 60fps rAF re-render did — push the
|
||||||
|
// (stale) source value back into the binding mid-drag. The guard must ignore
|
||||||
|
// it so the thumb stays where the user dragged it.
|
||||||
|
it("ignores external source changes WHILE dragging (no snap-back)", async () => {
|
||||||
|
const src = ref<number | undefined>(50);
|
||||||
|
const { display, onInput } = useDecoupledSlider(() => src.value, () => {});
|
||||||
|
|
||||||
|
onInput(ev(70)); // user starts dragging → display 70
|
||||||
|
expect(display.value).toBe(70);
|
||||||
|
|
||||||
|
// Simulate the per-frame re-render re-evaluating the (still-stale) source.
|
||||||
|
src.value = 50;
|
||||||
|
await nextTick();
|
||||||
|
expect(display.value).toBe(70); // stayed put — did NOT snap back to 50
|
||||||
|
});
|
||||||
|
|
||||||
|
// Corner case: a range input skips `change` when released back at its start
|
||||||
|
// value. onRelease (pointerup/pointercancel/blur) must still end the drag so
|
||||||
|
// the slider doesn't freeze against later external updates.
|
||||||
|
it("clears dragging on release even when @change never fires", async () => {
|
||||||
|
const src = ref<number | undefined>(50);
|
||||||
|
const { display, onInput, onRelease } = useDecoupledSlider(() => src.value, () => {});
|
||||||
|
|
||||||
|
onInput(ev(70)); // drag begins
|
||||||
|
onInput(ev(50)); // ...dragged back to the start value
|
||||||
|
onRelease(); // released — browser emits NO change event here
|
||||||
|
|
||||||
|
src.value = 30; // a later external update
|
||||||
|
await nextTick();
|
||||||
|
expect(display.value).toBe(30); // slider resumed following the source
|
||||||
|
});
|
||||||
|
|
||||||
|
it("commits on @change and resumes following the source afterwards", async () => {
|
||||||
|
const commit = vi.fn();
|
||||||
|
const src = ref<number | undefined>(50);
|
||||||
|
const { display, onInput, onChange } = useDecoupledSlider(() => src.value, commit);
|
||||||
|
|
||||||
|
onInput(ev(70));
|
||||||
|
onChange(ev(70)); // release
|
||||||
|
expect(commit).toHaveBeenCalledTimes(1);
|
||||||
|
expect(commit).toHaveBeenCalledWith(70);
|
||||||
|
expect(display.value).toBe(70);
|
||||||
|
|
||||||
|
// After release, external changes flow through again.
|
||||||
|
src.value = 35;
|
||||||
|
await nextTick();
|
||||||
|
expect(display.value).toBe(35);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,67 @@
|
|||||||
|
import { ref, watch, type Ref } from 'vue';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A slider whose displayed value is decoupled from its reactive source.
|
||||||
|
*
|
||||||
|
* Why this exists (#111): the player components run a 60fps requestAnimationFrame
|
||||||
|
* loop (progress clock, #107) that re-renders the whole component every ~16ms.
|
||||||
|
* Binding a range `<input :value>` straight to a reactive source (the store
|
||||||
|
* volume) let Vue re-apply `el.value = source` on every one of those re-renders.
|
||||||
|
* Mid-drag the source is still the *old* value, so the native drag position kept
|
||||||
|
* getting snapped back — the thumb was effectively un-draggable on desktop and
|
||||||
|
* janky on mobile.
|
||||||
|
*
|
||||||
|
* The fix is to bind `:value` to a LOCAL ref that:
|
||||||
|
* - tracks the native drag synchronously via `@input` (so the bound value always
|
||||||
|
* equals the element's value → Vue never resets it), and
|
||||||
|
* - is committed to the real source only on `@change` (release).
|
||||||
|
* External source changes (bot switch, another client) still flow into the
|
||||||
|
* display — except while the user is actively dragging, where they must be
|
||||||
|
* ignored or they'd fight the drag.
|
||||||
|
*
|
||||||
|
* @param source getter for the authoritative value (e.g. () => bot?.volume)
|
||||||
|
* @param commit called with the final value on release (e.g. store.setVolume)
|
||||||
|
* @param fallback value to show when the source is undefined (default 75)
|
||||||
|
*/
|
||||||
|
export function useDecoupledSlider(
|
||||||
|
source: () => number | undefined,
|
||||||
|
commit: (value: number) => void,
|
||||||
|
fallback = 75
|
||||||
|
): {
|
||||||
|
display: Ref<number>;
|
||||||
|
dragging: Ref<boolean>;
|
||||||
|
onInput: (e: Event) => void;
|
||||||
|
onChange: (e: Event) => void;
|
||||||
|
onRelease: () => void;
|
||||||
|
} {
|
||||||
|
const display = ref<number>(source() ?? fallback);
|
||||||
|
const dragging = ref(false);
|
||||||
|
|
||||||
|
watch(source, (v) => {
|
||||||
|
// Reflect external/store changes — but never while dragging, or the
|
||||||
|
// per-frame re-render would yank the thumb away from the user's finger.
|
||||||
|
if (!dragging.value && typeof v === 'number') display.value = v;
|
||||||
|
});
|
||||||
|
|
||||||
|
function onInput(e: Event): void {
|
||||||
|
dragging.value = true;
|
||||||
|
display.value = Number((e.target as HTMLInputElement).value);
|
||||||
|
}
|
||||||
|
|
||||||
|
function onChange(e: Event): void {
|
||||||
|
dragging.value = false;
|
||||||
|
const v = Number((e.target as HTMLInputElement).value);
|
||||||
|
display.value = v;
|
||||||
|
commit(v);
|
||||||
|
}
|
||||||
|
|
||||||
|
function onRelease(): void {
|
||||||
|
// Safety net for pointerup / pointercancel / blur: a range input does NOT
|
||||||
|
// emit `change` if the value is released back at its starting point, which
|
||||||
|
// would otherwise leave `dragging` stuck true and freeze the slider against
|
||||||
|
// later external updates. Clearing here is idempotent with onChange.
|
||||||
|
dragging.value = false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return { display, dragging, onInput, onChange, onRelease };
|
||||||
|
}
|
||||||
Reference in new issue
Block a user