spotify: a progress bar that works, and a tile that fits its size

server/src/providers/spotify.ts

@@ -11,21 +11,40 @@ import { tr } from '../i18n.js'
  */
 const SEP = '\x1f'
 const IS_RUNNING = 'tell application "System Events" to (name of processes) contains "Spotify"'
-const STATE = `tell application "Spotify"
+/**
+ * The position is made a whole number of milliseconds by AppleScript rather than read as the
+ * float it is.
+ *
+ * `player position` is the one fractional value here, and AppleScript writes a number with the
+ * decimal separator of the Mac's number format: where that is a comma it answers `69,724998`,
+ * which `Number()` reads as NaN — and a NaN position is a progress bar that never moves. An
+ * integer has no separator to get wrong. `duration` is already in milliseconds, and `sound
+ * volume` is already whole.
+ *
+ * With `div`, an operator, and not `round`, which is a Standard Additions command: inside a
+ * `tell` block a command goes to the target first, so `round` works only where Spotify agrees to
+ * load the addition, and one that refuses fails the whole script — a bar stuck at zero would
+ * become "Spotify error". An operator never leaves AppleScript. It truncates where `round`
+ * rounded, which under a millisecond nothing on the bar can show.
+ */
+export const STATE = `tell application "Spotify"
   if player state is stopped then return "stopped"
   set sep to "${SEP}"
-  return (player state as string) & sep & (name of current track) & sep & (artist of current track) & sep & (album of current track) & sep & (artwork url of current track) & sep & (duration of current track) & sep & (player position) & sep & (sound volume)
+  return (player state as string) & sep & (name of current track) & sep & (artist of current track) & sep & (album of current track) & sep & (artwork url of current track) & sep & (duration of current track) & sep & (((player position) * 1000) div 1) & sep & (sound volume)
 end tell`
 
 export function parseSpotify(raw: string) {
   const [state, title, artist, album, artwork, duration, position, volume] = raw.split(SEP)
+  // The script already sends integers; this is the second line of defence, so that a real which
+  // gets through anyway, decimal comma and all, still lands as a whole number rather than NaN.
+  const num = (v: string) => Math.round(Number(v.replace(',', '.')))
   return {
     available: true as const,
     state: state as 'playing' | 'paused' | 'stopped',
     title, artist, album, artwork,
-    durationMs: Number(duration),
-    positionMs: Math.round(Number(position) * 1000),
-    volume: Number(volume),
+    durationMs: num(duration),
+    positionMs: num(position),
+    volume: num(volume),
   }
 }
 
@@ -57,6 +76,8 @@ export function createSpotifyProvider(run: Runner = osascript, open: Opener = op
   const tell = (cmd: string) => run(`tell application "Spotify" to ${cmd}`).then(() => ({ ok: true }))
   return {
     channel: 'spotify',
+    // The widget's fill glides across exactly this second: `POLL_MS` and the `1s` transition in
+    // widgets/spotify/index.html say it again. Change the three together.
     intervalMs: 1000,
     async poll() {
       if ((await run(IS_RUNNING)) !== 'true') return { available: false }

server/test/providers.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect, vi } from 'vitest'
 import { parseVolume, createVolumeProvider } from '../src/providers/volume.js'
-import { parseSpotify, createSpotifyProvider, OPEN_PROGRAM, SPOTIFY_BUNDLE_ID } from '../src/providers/spotify.js'
+import { parseSpotify, createSpotifyProvider, OPEN_PROGRAM, SPOTIFY_BUNDLE_ID, STATE } from '../src/providers/spotify.js'
 import { createMutedeckProvider } from '../src/providers/mutedeck.js'
 
 describe('volume', () => {
@@ -21,9 +21,30 @@ describe('volume', () => {
 describe('spotify', () => {
   it('parses the delimited state', () => {
     // The separator is an ASCII unit separator: a track name can contain any printable text.
-    const s = parseSpotify(['playing', 'Song', 'Artist', 'Album', 'https://i.scdn.co/x', '215000', '12.5', '80'].join('\x1f'))
+    const s = parseSpotify(['playing', 'Song', 'Artist', 'Album', 'https://i.scdn.co/x', '215000', '12500', '80'].join('\x1f'))
     expect(s).toEqual({ available: true, state: 'playing', title: 'Song', artist: 'Artist', album: 'Album', artwork: 'https://i.scdn.co/x', durationMs: 215000, positionMs: 12500, volume: 80 })
   })
+
+  /**
+   * The bug this guards: AppleScript writes a real with the decimal separator of the Mac's number
+   * format, so where that is a comma `player position` answered `69,724998`, `Number()` made NaN
+   * of it, and the progress bar never moved. Both ends hold now: the script asks for a whole
+   * number of milliseconds, and a real that gets through anyway, comma and all, is read as the
+   * number it is and rounded to the millisecond.
+   */
+  it('reads a real written with a decimal comma as whole milliseconds, not NaN', () => {
+    const french = (position: string) =>
+      parseSpotify(['playing', 'Song', 'Artist', 'Album', '', '215000', position, '80'].join('\x1f')).positionMs
+    expect(french('69724,998')).toBe(69725)
+    // What AppleScript actually writes for a real from 10 000 up: the comma, and an exponent.
+    expect(french('6,9724998E+4')).toBe(69725)
+  })
+  it('asks Spotify for a whole number of milliseconds, with operators alone', () => {
+    // `div` and not `round`, a Standard Additions command that Spotify would have to load: see `STATE`.
+    expect(STATE).toContain('((player position) * 1000) div 1')
+    expect(STATE).not.toContain('round (')
+    expect(STATE).not.toContain('sep & (player position)')
+  })
   it('reports unavailable when Spotify is not running', async () => {
     const run = vi.fn(async (script: string) => (script.includes('System Events') ? 'false' : ''))
     expect(await createSpotifyProvider(run).poll!()).toEqual({ available: false })

server/test/spotify-widget.test.ts

@@ -9,29 +9,63 @@ import { fileURLToPath } from 'node:url'
  * that record the listeners it registers — then the recorded `contextmenu` handler is called,
  * which is exactly what the Edge's touch driver produces for a press held past its threshold.
  */
+interface StubNode {
+  textContent: string; className: string; title: string; onclick: (() => void) | null
+  style: Record<string, string> & { transform: string; setProperty(name: string, value: string): void }
+  childNodes: StubNode[]
+  /** Every transform the node was given, with the class it had at that moment, in order. */
+  writes: [string, string][]
+}
+
 interface Loaded {
   listeners: Map<string, ((e: unknown) => void)[]>
   commands: [string, string, unknown?][]
   settings: Record<string, unknown>
+  /** The elements the widget asked for by id, so a test can read what it put in them. */
+  nodes: Map<string, StubNode>
+  /** Hands the widget one answer from the spotify channel, as the host delivers it. */
+  publish(data: Record<string, unknown>): void
 }
 
-async function loadWidget(settings: Record<string, unknown>): Promise<Loaded> {
+async function loadWidget(settings: Record<string, unknown>, viewport: [number, number] = [632, 282]): Promise<Loaded> {
   const html = await readFile(fileURLToPath(new URL('../../widgets/spotify/index.html', import.meta.url)), 'utf8')
   const script = [...html.matchAll(/<script[^>]*>([\s\S]*?)<\/script>/g)].map((m) => m[1]).join('\n')
 
   const listeners = new Map<string, ((e: unknown) => void)[]>()
   const commands: [string, string, unknown?][] = []
-  const node = () => ({
-    textContent: '', className: '', style: {}, src: '',
-    classList: { add: () => {}, remove: () => {}, toggle: () => {} },
-    setAttribute: () => {}, removeAttribute: () => {}, appendChild: () => {},
-    addEventListener: () => {}, onclick: null,
-  })
+  const subscribers: ((data: unknown) => void)[] = []
+  const node = () => {
+    // A setter rather than a field: a snap and a glide end on the same transform, and only the
+    // writes on the way there tell them apart.
+    let transform = ''
+    const self = {
+      textContent: '', className: '', title: '', src: '',
+      writes: [] as [string, string][],
+      style: {
+        setProperty(name: string, value: string) { (this as Record<string, unknown>)[name] = value },
+        get transform() { return transform },
+        set transform(value: string) { transform = value; self.writes.push([self.className, value]) },
+      } as StubNode['style'],
+      clientWidth: viewport[0] - 36,
+      childNodes: [] as StubNode[],
+      classList: { add: () => {}, remove: () => {}, toggle: () => {} },
+      setAttribute: () => {}, removeAttribute: () => {},
+      appendChild: (child: StubNode) => { self.childNodes.push(child) },
+      addEventListener: () => {}, onclick: null,
+    }
+    return self
+  }
+  // By id, and the same object every time: `textContent = ''` on a fresh stub would erase nothing.
+  const nodes = new Map<string, ReturnType<typeof node>>()
+  const byId = (id: string) => {
+    if (!nodes.has(id)) nodes.set(id, node())
+    return nodes.get(id)!
+  }
   const document = {
     documentElement: { lang: '', classList: { add: () => {}, remove: () => {} }, style: { setProperty: () => {} } },
     head: { firstChild: null, insertBefore: () => {}, appendChild: () => {} },
     createElement: node,
-    getElementById: node,
+    getElementById: byId,
     querySelector: node,
     addEventListener: (type: string, fn: (e: unknown) => void) => {
       listeners.set(type, [...(listeners.get(type) ?? []), fn])
@@ -43,21 +77,34 @@ async function loadWidget(settings: Record<string, unknown>): Promise<Loaded> {
     compact: false,
     instanceId: 'spotify-1',
     whenReady: (cb: () => void) => cb(),
-    subscribe: () => () => {},
+    subscribe: (_channel: string, cb: (data: unknown) => void) => { subscribers.push(cb); return () => {} },
     onLocale: () => {},
     onResize: () => {},
     onSettings: () => {},
     sendCommand: (channel: string, name: string, payload?: unknown) => {
       commands.push([channel, name, payload])
       return Promise.resolve({ ok: true })
     },
+    el: (_tag: string, className: string, text?: unknown) => {
+      const n = node()
+      n.className = className ?? ''
+      n.textContent = String(text ?? '')
+      return n
+    },
     t: (dict: unknown) => (typeof dict === 'string' ? dict : ((dict as Record<string, string>)?.fr ?? '')),
     esc: (v: unknown) => String(v ?? ''),
     color: (v: unknown, fallback: string) => fallback,
   }
-  const window = { setTimeout: (fn: () => void, ms: number) => setTimeout(fn, ms), clearTimeout }
+  const window = {
+    setTimeout: (fn: () => void, ms: number) => setTimeout(fn, ms), clearTimeout,
+    innerWidth: viewport[0], innerHeight: viewport[1],
+  }
   new Function('document', 'Fremkit', 'window', 'console', script)(document, Fremkit, window, console)
-  return { listeners, commands, settings }
+  return {
+    listeners, commands, settings,
+    nodes: nodes as unknown as Map<string, StubNode>,
+    publish: (data: Record<string, unknown>) => subscribers.forEach((cb) => cb(data)),
+  }
 }
 
 const fire = (loaded: Loaded, type: string, event: unknown = {}) =>
@@ -117,3 +164,87 @@ describe('the spotify widget long press', () => {
     }
   })
 })
+
+/**
+ * The progress bar. The position arrives once a second, so a bar that only moved when an answer
+ * landed stepped once a second — visibly. It is aimed a second ahead and given a second to get
+ * there instead, and each answer corrects the aim. What must not glide is a seek or a new track:
+ * running the fill backwards across a second reads as a bug, not as an animation.
+ */
+describe('the spotify widget progress bar', () => {
+  const playing = (positionMs: number, extra: Record<string, unknown> = {}) => ({
+    available: true, state: 'playing', title: 'Song', artist: 'Artist', album: 'Album',
+    durationMs: 200_000, positionMs, ...extra,
+  })
+  const scale = (loaded: Loaded) =>
+    Number(/scaleX\(([\d.]+)\)/.exec(loaded.nodes.get('bar')!.style.transform ?? '')?.[1])
+  /** What one answer does to the fill: each transform it is given, and the class it has then. */
+  const writesFor = (loaded: Loaded, data: Record<string, unknown>) => {
+    const bar = loaded.nodes.get('bar')!
+    bar.writes.length = 0
+    loaded.publish(data)
+    return bar.writes
+  }
+
+  it('aims one poll ahead while the track plays', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(100_000))
+    // 100 s of 200 s, plus the second it will have played by the next answer.
+    expect(scale(loaded)).toBeCloseTo(0.505, 5)
+  })
+
+  it('stops at where it is when the track is paused', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(100_000, { state: 'paused' }))
+    expect(scale(loaded)).toBeCloseTo(0.5, 5)
+  })
+
+  it('never aims past the end of the track', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(199_800))
+    expect(scale(loaded)).toBe(1)
+  })
+
+  it('glides on an ordinary answer: one write, with the transition left on', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(100_000))
+    expect(writesFor(loaded, playing(101_000))).toEqual([['', 'scaleX(0.51)']])
+  })
+
+  it('snaps rather than glides when the track changes', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(180_000))
+    // Put down on the new track's position with the transition off, then sent on from there.
+    expect(writesFor(loaded, playing(1_000, { title: 'Another' })))
+      .toEqual([['snap', 'scaleX(0.005)'], ['', 'scaleX(0.01)']])
+  })
+
+  it('snaps on a seek backwards, and glides on one forwards', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(100_000))
+    expect(writesFor(loaded, playing(20_000))).toEqual([['snap', 'scaleX(0.1)'], ['', 'scaleX(0.105)']])
+    expect(writesFor(loaded, playing(150_000))).toEqual([['', 'scaleX(0.755)']])
+  })
+
+  it('snaps on a pause, which lands behind a fill aimed a second ahead', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(100_000))
+    // Aimed at 101 s; the next answer is `paused` at 100.2 s. Gliding there runs the fill back.
+    expect(writesFor(loaded, playing(100_200, { state: 'paused' })))
+      .toEqual([['snap', 'scaleX(0.501)'], ['', 'scaleX(0.501)']])
+  })
+
+  it('empties at once when Spotify closes, and lands rather than glides when it plays again', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(100_000))
+    expect(writesFor(loaded, { available: false })).toEqual([['snap', 'scaleX(0)']])
+    // The same track at the same place: from an empty bar, that is a jump and not a glide.
+    expect(writesFor(loaded, playing(100_000))).toEqual([['snap', 'scaleX(0.5)'], ['', 'scaleX(0.505)']])
+  })
+
+  it('draws an empty bar for a track with no duration at all', async () => {
+    const loaded = await loadWidget({})
+    loaded.publish(playing(0, { durationMs: 0 }))
+    expect(scale(loaded)).toBe(0)
+  })
+})

server/test/widget-catalog.test.ts

@@ -147,6 +147,8 @@ describe('WidgetCatalog', () => {
     const battery = cat.get('battery')!
     expect(battery.subscriptions).toEqual(['battery'])
     expect([battery.minSize, battery.defaultSize]).toEqual([[8, 4], [8, 6]])
+    const spotify = cat.get('spotify')!
+    expect([spotify.minSize, spotify.defaultSize]).toEqual([[8, 4], [16, 5]])
     const status = cat.get('service-status')!
     expect(status.subscriptions).toEqual([])
     expect(status.commands).toEqual(['service-status'])