Skip to content

RFC: Extract the mini-player seek-bar/storyboard engine into a deep attachMiniSeekBar module #1389

Description

@VampireChicken12

Problem

src/features/miniPlayer/controller.ts (883 lines) fuses three unrelated concerns in one file:

  1. The MiniPlayerController class (lines 52–435): overlay creation, drag/resize, detaching the YouTube player into the overlay and restoring it, rect clamp/snap math, rect persistence.
  2. A complete custom seek bar with storyboard hover previews (lines 472–883, ~410 lines), implemented as a module-level singleton (miniPlayerBarState) driven by two global functions, enableMiniPlayerCustomProgress() / disableMiniPlayerCustomProgress().
  3. Assorted pure algorithms (storyboard spec parsing, sprite geometry, URL signing, seek-window math) fused inline with DOM style writes.

The shallow seams create concrete integration risk:

  • Import-time side effect: controller.ts:11 grabs stateAPI from the registry singleton at module evaluation, so nothing in the file can be instantiated in a test without the whole registry.
  • Entangled cleanup: the bar's teardown is dumped into the shared cleanupRegistry bucket keyed "miniPlayer", mixed with the controller's own cleanups — disableMiniPlayerCustomProgress() runs the entire bucket, not just the bar's teardown.
  • Cross-feature DOM poke: src/features/timestampPeek/utils.ts:313,337 reaches into this feature's DOM with document.querySelector("#yte-mini-player-overlay")!.style.display — a non-null assertion that crashes if the overlay is absent, and a coupling with no contract.
  • Singleton globals: only one bar instance can ever exist, and its lifecycle is decoupled from the controller that logically owns it.
  • Zero testability: the genuinely algorithmic code (the W#H#count#cols#rows#…#sig spec grammar, $L/$N/sigh URL construction, frame→sheet→row/col sprite mapping, aspect-fit scaling, VOD-vs-live seek windows) cannot be tested because it is interleaved with style mutations and page-global reads.

Proposed Interface

Extract lines 472–883 into src/features/miniPlayer/seekBar/ as one deep module with a single entry point. (This design was selected from four competing candidates — minimal-surface, composable, caller-optimized, ports & adapters — and hybridizes the winners.)

// seekBar/index.ts — the ONLY public surface
export type StoryboardRenderer = {
  fineScrubbingRecommendedLevel?: number;
  highResolutionRecommendedLevel?: number;
  recommendedLevel?: number;
  spec?: string;
};

export type MiniSeekBarOptions = {
  /** The player element already inside the overlay; video, controls host, and native bar are found within it. */
  playerElement: HTMLElement;
  /** Element the bar mounts into (the mini-player overlay). */
  host: HTMLElement;
  /** Port: async storyboard spec discovery. Resolves null → bar degrades to timestamp-only preview.
   *  Default: the YouTube page chain (rAF-polled getPlayerResponse() → ytInitialPlayerResponse → ytplayer.config). */
  storyboards?: () => Promise<StoryboardRenderer | null>;
  /** Port: subscribe to "the page may have swapped the <video>". Returns unsubscribe.
   *  Default: document yt-navigate-finish + yt-player-updated. */
  onMediaChanged?: (cb: () => void) => () => void;
};

export type MiniSeekBar = {
  /** Idempotent. Removes bar DOM, unbinds every listener/observer/timer, restores the native progress bar. */
  destroy(): void;
};

/** Never throws. If no <video> exists yet, arms rebinding and activates when one appears. */
export function attachMiniSeekBar(options: MiniSeekBarOptions): MiniSeekBar;
// seekBar/core.ts — pure, zero DOM imports; deep-imported by unit tests
export function parseStoryboardSheet(renderer: StoryboardRenderer): StoryboardSheet | null;
export function buildStoryboardTileUrl(sheet: StoryboardSheet, imageIndex: number): string;
export function storyboardTileAt(sheet: StoryboardSheet, timeRatio: number, videoAspect: number,
                                 max?: { height: number; width: number }): StoryboardTile; // aspect-fit ≤160×90
export function computeSeekWindow(v: { duration: number; seekable: TimeRangesLike | null }): SeekWindow | null;
export function ratioToTime(w: SeekWindow, ratio: number): number;
export function timeToRatio(w: SeekWindow, t: number): number; // clamped 0..1
export function formatTime(seconds: number): string;
/** Visibility reducer: autohide × force-show × scrubbing precedence as a pure predicate. */
export function isBarHidden(s: { controlsVisible: boolean; forced: boolean; scrubbing: boolean }): boolean;
// seekBar/youtubePage.ts — the ONLY file allowed to touch YouTube selectors/globals
// (default implementations of the two ports + video/controls/native-bar lookups)

The cross-feature contract lives on the feature barrel, not the engine:

// miniPlayer/index.ts
/** Hides the overlay if active; returns a restore fn. No-op → no-op restore when inactive.
 *  Safe to call unconditionally. */
export function suspendMiniPlayerOverlay(): () => void;

Usage

// MiniPlayerController
private seekBar: Nullable<MiniSeekBar> = null;

// movePlayerIntoOverlay() — replaces the enableMiniPlayerCustomProgress try/catch:
this.seekBar = attachMiniSeekBar({ host: this.overlayElement, playerElement: player });

// restorePlayer() — replaces disableMiniPlayerCustomProgress():
this.seekBar?.destroy();
this.seekBar = null;
// timestampPeek/utils.ts — replaces both querySelector(...)! pokes:
state.restoreMiniPlayer = suspendMiniPlayerOverlay();  // show branch
state.restoreMiniPlayer?.(); state.restoreMiniPlayer = null;  // hide branch

What it hides

  • Bar DOM construction (9 elements) and mount/unmount.
  • Video discovery and swap survival: on media-change or loadedmetadata, re-resolve the element, migrate timeupdate/progress/durationchange listeners leak-free, re-fetch the storyboard. Stale async resolutions after destroy() are dropped (generation counter).
  • Scrubbing state machine: pointer capture, hover range, preview clamping to bar edges, seek via ratio math.
  • Visibility choreography: MutationObserver on ytp-autohide, pointer force-show, 1200 ms decay — reduced to the pure isBarHidden predicate plus one timer.
  • Storyboard pipeline: late/absent spec (timestamp-only until arrival, upgrade in place), level preference, URL signing, sprite geometry.
  • Native .ytp-progress-bar-container hide/restore tied to the handle lifecycle.
  • All cleanup: a private disposer list drained by destroy() — no shared cleanupRegistry bucket, no eventManager, no registry import anywhere in seekBar/.

Dependency Strategy

Dependency Category Handling Test stand-in
Spec parsing, URL grammar, sprite geometry, seek/ratio math, visibility reducer In-process merged into pure core.ts none needed — fixtures in, numbers out
Bar DOM, <video> events, MutationObserver, timers Local-substitutable engine touches only elements it created or was handed happy-dom + fake timers; stub getBoundingClientRect
Storyboard spec (page globals, #movie_player.getPlayerResponse(), rAF poll) True external injected port storyboards, default in youtubePage.ts async () => fixture / async () => null
Video-swap signal (yt-navigate-finish, yt-player-updated) True external injected port onMediaChanged, default in youtubePage.ts test hands back a manually-fired callback
Registry / stateAPI / cleanupRegistry / eventManager never imported by seekBar/* n/a

Testing Strategy

New boundary tests to write:

  • core.ts (pure, no DOM): spec-grammar fixtures (valid, malformed, missing signature), URL building ($L/$N substitution, protocol-relative prefix, sigh appending), tile geometry (frame→sheet→row/col; aspect-fit for wide and tall videos), seek windows (VOD, live seekable range with nonzero start, none-yet), ratio clamping, formatTime, isBarHidden truth table.
  • attachMiniSeekBar (happy-dom): played/buffered bars render on timeupdate; pointerdown at X seeks to the right absolute time in a live window; preview is timestamp-only when storyboards resolves null and upgrades in place on late resolve; a resolve arriving after destroy() is dropped; firing onMediaChanged with a swapped <video> migrates listeners without leaks; destroy() restores the native bar, removes all DOM, and is idempotent.
  • suspendMiniPlayerOverlay: returns a no-op restore when the mini player is inactive; hides and restores the overlay when active.

Old tests to delete: none — the repo currently has no tests (the only spec file is empty). This module is intended as the first real unit-test surface.

Test environment needs: vitest + happy-dom + fake timers. No network, no browser, no extension APIs.

Implementation Recommendations

  • The module owns: its bar DOM subtree, the scrub interaction, visibility state, the storyboard pipeline, native-bar suppression, and the entirety of its own cleanup.
  • The module hides: the storyboard spec grammar and discovery fallback chain, the video-rebind dance, sprite geometry, and timer choreography. These must never leak into the controller again.
  • The module exposes: one attach function returning one destroy handle, plus the pure core for direct unit testing. Resist adding entry points; new needs should become options or internal behavior.
  • Hard rule: nothing under seekBar/ imports the registry, eventManager, or cleanupRegistry. All page-specific selectors and globals stay in the single YouTube adapter file, so a YouTube markup change is a one-file fix.
  • Migration: the controller replaces its two global-function call sites with a held handle (attach on player-detach, destroy on restore); timestampPeek swaps its two DOM pokes for suspendMiniPlayerOverlay(); lines 472–883 are then deleted from controller.ts. The controller's own registry/stateAPI coupling is out of scope here (it belongs to the registry-deepening RFC).
  • Deferred, not rejected: splitting out a reusable StoryboardProvider/MediaTransport layering (the "composable" candidate design) is worthwhile only when a second storyboard consumer materializes — e.g. timestampPeek previewing via storyboard tiles instead of pausing real playback. The pure core.ts already carries the reusable 80% (parsing + geometry), so that evolution requires no interface break.

Findings context: docs/prds/architecture-module-deepening.md (candidate 6).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions