From 9079ab084883ae133af328538b1279ad933d2eb4 Mon Sep 17 00:00:00 2001 From: Ayo Date: Sun, 5 Jul 2026 06:43:12 +0200 Subject: [PATCH] refactor(leaderboard-element): wcb usage improvements --- packages/leaderboard/leaderboard-element.js | 8 +- packages/leaderboard/leaderboard-read.js | 3 +- .../test/leaderboard-element.test.js | 161 ++++++++++++++++++ .../test/read-write-separation.test.js | 12 +- 4 files changed, 180 insertions(+), 4 deletions(-) diff --git a/packages/leaderboard/leaderboard-element.js b/packages/leaderboard/leaderboard-element.js index 7364d5d..5793970 100644 --- a/packages/leaderboard/leaderboard-element.js +++ b/packages/leaderboard/leaderboard-element.js @@ -131,8 +131,10 @@ export class CozyLeaderboard extends WebComponent { * service so the new config actually takes effect. */ onChanges({ property, currentValue }) { - if (!this._connected || property === 'title') return + // Invalidate the cached service even while disconnected, so a score-order/ + // format change made on a detached element takes effect on re-connect. if (property === 'score-order' || property === 'format') this._svc = null + if (!this._connected || property === 'title') return this._mount(property === 'duration' ? (str(currentValue) || undefined) : undefined) } @@ -269,7 +271,9 @@ export class CozyLeaderboard extends WebComponent { * focus is handed to the replacement tab explicitly. */ _paint() { - const focused = document.activeElement + // getRootNode(), not document: inside a shadow root, document.activeElement + // is retargeted to the host and the focused tab would go undetected. + const focused = this.getRootNode().activeElement const focusedTab = focused && this.contains(focused) ? focused.dataset.duration : undefined this.render() if (focusedTab) this.querySelector(`button[data-duration="${focusedTab}"]`)?.focus() diff --git a/packages/leaderboard/leaderboard-read.js b/packages/leaderboard/leaderboard-read.js index 5d00fcf..8032ed2 100644 --- a/packages/leaderboard/leaderboard-read.js +++ b/packages/leaderboard/leaderboard-read.js @@ -249,7 +249,8 @@ export class LeaderBoardReader { const nameElement = document.createElement('div') const name = data.name || this.anonymousName - nameElement.innerHTML = name + // textContent, never innerHTML: names are player-controlled input. + nameElement.textContent = name nameElement.setAttribute('title', name) nameElement.style.textOverflow = 'ellipsis' nameElement.style.whiteSpace = 'nowrap' diff --git a/packages/leaderboard/test/leaderboard-element.test.js b/packages/leaderboard/test/leaderboard-element.test.js index 0e40c69..82038a0 100644 --- a/packages/leaderboard/test/leaderboard-element.test.js +++ b/packages/leaderboard/test/leaderboard-element.test.js @@ -428,4 +428,165 @@ describe('submit()', () => { await el.submit(entry()) expect(adapter.addScore).not.toHaveBeenCalled() }) + + it('is a safe no-op on an unconfigured element', async () => { + const el = mount({ category: 'beginner', title: 'T' }) + await vi.waitFor(() => expect(rowsText(el)).toContain('Leaderboard not configured.')) + expect(el.submit(entry())).toBeUndefined() + }) +}) + +describe('regressions pinned after review', () => { + it('renders entry names as text, never as HTML (element path)', async () => { + const adapter = makeAdapter([{ name: '', score: 1 }]) + const el = mount({ category: 'b', title: 'T' }, { adapter }) + await vi.waitFor(() => expect(rowsText(el)).toContain('')) + expect(el.querySelector('img')).toBeNull() + }) + + it('an invalid duration id shows the error text without querying the adapter', async () => { + const adapter = makeAdapter([{ name: 'Ada', score: 1 }]) + const el = mount({ category: 'b', title: 'T', duration: 'yesterday' }, { adapter }) + await vi.waitFor(() => + expect(rowsText(el)).toContain('Leaderboard unavailable right now.')) + expect(adapter.listScores).not.toHaveBeenCalled() + }) + + it('a title change does not re-query the backend', async () => { + const adapter = makeAdapter([]) + const el = mount({ category: 'b', title: 'T' }, { adapter }) + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(1)) + + el.setAttribute('title', 'New Title') + await vi.waitFor(() => expect(el.querySelector('h3').textContent).toBe('New Title')) + expect(adapter.listScores).toHaveBeenCalledTimes(1) + }) + + it('a score-order change after mount takes effect on the next query', async () => { + const adapter = makeAdapter([]) + const el = mount({ category: 'b', title: 'T', 'score-order': 'asc' }, { adapter }) + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(1)) + expect(adapter.listScores.mock.calls[0][0].order).toBe('asc') + + el.setAttribute('score-order', 'desc') + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(2)) + expect(adapter.listScores.mock.calls[1][0].order).toBe('desc') + }) + + it('a format change after mount takes effect on the next render', async () => { + const adapter = makeAdapter([{ name: 'Ada', score: 4200 }]) + const el = mount({ category: 'b', title: 'T' }, { adapter }) + await vi.waitFor(() => expect(rowsText(el)).toContain('4200')) + + el.setAttribute('format', 'time') + await vi.waitFor(() => expect(rowsText(el)).toContain('04.2')) + }) + + it('a score-order change while detached takes effect on re-connect', async () => { + const adapter = makeAdapter([]) + const el = mount({ category: 'b', title: 'T', 'score-order': 'asc' }, { adapter }) + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(1)) + + el.remove() + el.setAttribute('score-order', 'desc') + document.body.append(el) + await vi.waitFor(() => { + const calls = adapter.listScores.mock.calls + expect(calls[calls.length - 1][0].order).toBe('desc') + }) + }) + + it('configureLeaderboard() re-mounts every live element, keeping each selected tab', async () => { + const adapter = makeAdapter([]) + configureLeaderboard({ adapter }) + const one = mount({ category: 'a', title: 'One' }) + const two = mount({ category: 'b', title: 'Two' }) + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(2)) + + tabByLabel(two, 'Week').click() + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(3)) + + // Re-configuring re-mounts BOTH live elements (string overrides won't + // apply to their already-cached services — pinned separately below). + configureLeaderboard({}) + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(5)) + // element two kept its Week selection through the shared re-mount + await vi.waitFor(() => { + expect(tabByLabel(two, 'Week').style.fontWeight).toBe('bold') + expect(tabByLabel(one, 'Today').style.fontWeight).toBe('bold') + }) + }) + + it('a detached element is not re-mounted by configureLeaderboard, but remembers its tab on re-connect', async () => { + const adapter = makeAdapter([]) + const el = mount({ category: 'b', title: 'T' }, { adapter }) + await vi.waitFor(() => expect(tabButtons(el)).toHaveLength(4)) + tabByLabel(el, 'Month').click() + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(2)) + + el.remove() + const callsWhileDetached = adapter.listScores.mock.calls.length + configureLeaderboard({ loadingText: 'Wait…' }) + expect(adapter.listScores.mock.calls.length).toBe(callsWhileDetached) + + document.body.append(el) + await vi.waitFor(() => + expect(tabByLabel(el, 'Month').style.fontWeight).toBe('bold')) + }) + + it('keeps the cached service (old adapter) when configureLeaderboard swaps adapters later', async () => { + const first = makeAdapter([]) + configureLeaderboard({ adapter: first }) + mount({ category: 'b', title: 'T' }) + await vi.waitFor(() => expect(first.listScores).toHaveBeenCalledTimes(1)) + + const second = makeAdapter([]) + configureLeaderboard({ adapter: second }) + await vi.waitFor(() => expect(first.listScores).toHaveBeenCalledTimes(2)) + expect(second.listScores).not.toHaveBeenCalled() // documented caching semantics + }) + + it('keeps the cached service (old strings) when configureLeaderboard changes strings later', async () => { + const adapter = makeAdapter([]) + configureLeaderboard({ adapter }) + const el = mount({ category: 'b', title: 'T' }) + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(1)) + + configureLeaderboard({ emptyMessages: ['Nothing here yet.'] }) + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(2)) + // the already-built service keeps its original strings (documented caching) + expect(rowsText(el)).not.toContain('Nothing here yet.') + }) + + it('keeps focus on a clicked tab when composed inside a shadow root', async () => { + const adapter = makeAdapter([]) + const host = document.createElement('div') + const shadow = host.attachShadow({ mode: 'open' }) + const el = /** @type {any} */ (document.createElement('cozy-leaderboard')) + el.setAttribute('category', 'b') + el.setAttribute('title', 'T') + el.adapter = adapter + shadow.append(el) + document.body.append(host) + await vi.waitFor(() => expect(el.querySelectorAll('button')).toHaveLength(4)) + + const week = [...el.querySelectorAll('button')].find(b => b.textContent === 'Week') + week.focus() + week.click() + await vi.waitFor(() => expect(adapter.listScores).toHaveBeenCalledTimes(2)) + expect(shadow.activeElement?.textContent).toBe('Week') + }) + + it('pins prettyTime edge cases through the format attribute', async () => { + const adapter = makeAdapter([ + { name: 'Zero', score: 0 }, + { name: 'Minute', score: 60000 } + ]) + const el = mount({ category: 'b', title: 'T', format: 'time' }, { adapter }) + await vi.waitFor(() => { + expect(rowsText(el)).toContain('Zero') + expect(rowsText(el)).toContain('0') // falsy score -> '0' + expect(rowsText(el)).toContain('01:0') // exact minute -> '01:0' + }) + }) }) diff --git a/packages/leaderboard/test/read-write-separation.test.js b/packages/leaderboard/test/read-write-separation.test.js index bfacb9f..24d5ead 100644 --- a/packages/leaderboard/test/read-write-separation.test.js +++ b/packages/leaderboard/test/read-write-separation.test.js @@ -43,7 +43,7 @@ describe('read surface — usable standalone (no writes)', () => { expect(el.querySelectorAll('button')).toHaveLength(4) // four duration tabs expect(listScores).toHaveBeenCalledTimes(1) // 'today' window loaded once - // The list fills asynchronously — wait for the rows to appear (names use innerHTML). + // The list fills asynchronously — wait for the rows to appear (names render as text). await vi.waitFor(() => { expect(el.textContent).toContain('Ada') expect(el.textContent).toContain('Bo') @@ -52,6 +52,16 @@ describe('read surface — usable standalone (no writes)', () => { // Reader exposes no write API. expect(typeof (/** @type {any} */ (reader).submit)).toBe('undefined') }) + + it('renders entry names as text, never as HTML (names are player-controlled)', async () => { + const listScores = vi.fn(async () => [{ name: '', score: 1 }]) + const reader = new LeaderBoardReader({ adapter: { listScores } }) + + const el = await reader.render('beginner', 'Best Times') + await vi.waitFor(() => + expect(el.textContent).toContain('')) + expect(el.querySelector('img')).toBeNull() + }) }) describe('write surface — usable standalone with an injected instance', () => {