fix(career): passport review polish — a11y semantics + seen-state guard (#937)

CodeRabbit follow-up on #936 (the one Major — overlay outside the click
root — was verified false: the host mounts every screen.html root inside
#plugin-career, ✕-close confirmed working live):

- Tabs: aria-selected/aria-controls + role=tabpanel/aria-labelledby.
- Book overlay: role=dialog + aria-modal + aria-label; focus moves to
  the close button on open and returns to the opener on close.
- seenBadges(): guard non-object JSON so a corrupt stored value cannot
  throw on every passport refresh (covered by a new corruption test).
- Fresh-session suppression test (badge seen → no re-notification).
- Stylelint declaration-empty-line-before nit in .pp-stamp.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Byron Gamatos
2026-07-13 12:14:24 +02:00
committed by GitHub
co-authored by Claude Fable 5
parent 7ffa6e2c51
commit 99b974a5a1
5 changed files with 49 additions and 9 deletions
+5
View File
@@ -56,6 +56,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry). engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry).
### Fixed ### Fixed
- **Career passports review polish** — the passport tabs and book overlay carry
proper ARIA semantics (`aria-selected`/`aria-controls`/`tabpanel`;
`role="dialog"` + `aria-modal` with focus moved to the close button on open
and restored on close), and a corrupt stored seen-badges value (e.g. a stray
`"null"`) can no longer throw on every passport refresh.
- **The packaged desktop app could not start (`ModuleNotFoundError: No module named - **The packaged desktop app could not start (`ModuleNotFoundError: No module named
'appstate'`).** feedback-desktop's `scripts/bundle-slopsmith.sh` copies a *hardcoded 'appstate'`).** feedback-desktop's `scripts/bundle-slopsmith.sh` copies a *hardcoded
list* of core files into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`, list* of core files into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`,
+1
View File
@@ -257,6 +257,7 @@
/* The rubber stamp */ /* The rubber stamp */
.pp-stamp { .pp-stamp {
--pp-rot: 0deg; --pp-rot: 0deg;
position: relative; position: relative;
display: flex; display: flex;
flex-direction: column; flex-direction: column;
+4 -4
View File
@@ -4,11 +4,11 @@
<div id="career-stars-summary" class="text-sm text-gray-400"></div> <div id="career-stars-summary" class="text-sm text-gray-400"></div>
</div> </div>
<div class="career-tabs" role="tablist"> <div class="career-tabs" role="tablist">
<button class="career-tab" data-career-tab="venues" role="tab">Venues</button> <button class="career-tab" data-career-tab="venues" role="tab" id="career-tab-btn-venues" aria-controls="career-tab-venues">Venues</button>
<button class="career-tab" data-career-tab="passports" role="tab">Passports</button> <button class="career-tab" data-career-tab="passports" role="tab" id="career-tab-btn-passports" aria-controls="career-tab-passports">Passports</button>
</div> </div>
<div id="career-tab-venues"> <div id="career-tab-venues" role="tabpanel" aria-labelledby="career-tab-btn-venues">
<p class="text-sm text-gray-400 mb-4">Earn stars by playing songs well — 60% accuracy is a star, 75% two, 85% three. Stars unlock bigger stages, and the crowd plays along with you.</p> <p class="text-sm text-gray-400 mb-4">Earn stars by playing songs well — 60% accuracy is a star, 75% two, 85% three. Stars unlock bigger stages, and the crowd plays along with you.</p>
<div id="career-progress-wrap" class="mb-6"> <div id="career-progress-wrap" class="mb-6">
<div class="career-bar-track"> <div class="career-bar-track">
@@ -26,7 +26,7 @@
</div> </div>
</div> </div>
<div id="career-tab-passports" class="hidden"> <div id="career-tab-passports" class="hidden" role="tabpanel" aria-labelledby="career-tab-btn-passports">
<p class="text-sm text-gray-400 mb-4">Commit to an instrument, pick a genre, and stamp your way to its badge — five ★★ songs mint a Bronze. Your passport wall is who you are as a musician.</p> <p class="text-sm text-gray-400 mb-4">Commit to an instrument, pick a genre, and stamp your way to its badge — five ★★ songs mint a Bronze. Your passport wall is who you are as a musician.</p>
<div id="pp-instruments" class="pp-instruments"></div> <div id="pp-instruments" class="pp-instruments"></div>
<div id="pp-shelf-wrap" class="mt-5"> <div id="pp-shelf-wrap" class="mt-5">
+19 -3
View File
@@ -33,6 +33,7 @@
let _pp = null; // last /passports view let _pp = null; // last /passports view
let _ppRelayTimer = 0; let _ppRelayTimer = 0;
let _ppBook = null; // {inst, gkey} of the open spread let _ppBook = null; // {inst, gkey} of the open spread
let _ppReturnFocus = null; // element to refocus when the book closes
let _ppBootstrapped = false; let _ppBootstrapped = false;
let _ppNotified = {}; // badges chimed this session (slam still pending) let _ppNotified = {}; // badges chimed this session (slam still pending)
@@ -276,7 +277,9 @@
venues.classList.toggle('hidden', tab !== 'venues'); venues.classList.toggle('hidden', tab !== 'venues');
pp.classList.toggle('hidden', tab !== 'passports'); pp.classList.toggle('hidden', tab !== 'passports');
document.querySelectorAll('#plugin-career .career-tab').forEach((b) => { document.querySelectorAll('#plugin-career .career-tab').forEach((b) => {
b.classList.toggle('active', b.dataset.careerTab === tab); const active = b.dataset.careerTab === tab;
b.classList.toggle('active', active);
b.setAttribute('aria-selected', active ? 'true' : 'false');
}); });
} }
@@ -289,7 +292,12 @@
} }
function seenBadges() { function seenBadges() {
try { return JSON.parse(lsGet(PP_SEEN_KEY) || '{}'); } catch (_) { return {}; } try {
const seen = JSON.parse(lsGet(PP_SEEN_KEY) || '{}');
// Guard non-object JSON (a stray "null" or array) — a broken
// stored value must not throw on every passport refresh.
return seen && typeof seen === 'object' && !Array.isArray(seen) ? seen : {};
} catch (_) { return {}; }
} }
function badgeId(inst, gkey) { return inst + '/' + gkey; } function badgeId(inst, gkey) { return inst + '/' + gkey; }
@@ -475,7 +483,7 @@
: `Play ${esc(p.genre)} songs at ${starGl} to collect ticket stubs.`; : `Play ${esc(p.genre)} songs at ${starGl} to collect ticket stubs.`;
const stubsHTML = stubs.length ? stubs.map(ppStubHTML).join('') const stubsHTML = stubs.length ? stubs.map(ppStubHTML).join('')
: `<div class="pp-stub-empty">${emptyLine}</div>`; : `<div class="pp-stub-empty">${emptyLine}</div>`;
return `<div class="pp-book-wrap" data-pp-close-bg="1"> return `<div class="pp-book-wrap" data-pp-close-bg="1" role="dialog" aria-modal="true" aria-label="${esc(p.genre)} ${esc(ppLabel(inst))} passport">
<div class="pp-book"> <div class="pp-book">
<div class="pp-page pp-page-left"> <div class="pp-page pp-page-left">
<div class="pp-page-head">${esc(p.genre)}${esc(ppLabel(inst))}</div> <div class="pp-page-head">${esc(p.genre)}${esc(ppLabel(inst))}</div>
@@ -501,9 +509,12 @@
const overlay = $('pp-overlay'); const overlay = $('pp-overlay');
if (!p || !overlay) return; if (!p || !overlay) return;
_ppBook = { inst, gkey }; _ppBook = { inst, gkey };
_ppReturnFocus = document.activeElement;
const pending = p.badge === 'earned' && !seenBadges()[badgeId(inst, gkey)]; const pending = p.badge === 'earned' && !seenBadges()[badgeId(inst, gkey)];
overlay.innerHTML = ppBookHTML(inst, p, pending); overlay.innerHTML = ppBookHTML(inst, p, pending);
overlay.classList.remove('hidden'); overlay.classList.remove('hidden');
const close = overlay.querySelector('.pp-book-close');
if (close) close.focus();
sfx('page'); sfx('page');
// Double rAF so the cover's closed state paints before the transition. // Double rAF so the cover's closed state paints before the transition.
requestAnimationFrame(() => requestAnimationFrame(() => { requestAnimationFrame(() => requestAnimationFrame(() => {
@@ -530,6 +541,11 @@
_ppBook = null; _ppBook = null;
const overlay = $('pp-overlay'); const overlay = $('pp-overlay');
if (overlay) { overlay.classList.add('hidden'); overlay.innerHTML = ''; } if (overlay) { overlay.classList.add('hidden'); overlay.innerHTML = ''; }
if (_ppReturnFocus && typeof _ppReturnFocus.focus === 'function' &&
document.contains(_ppReturnFocus)) {
_ppReturnFocus.focus();
}
_ppReturnFocus = null;
} }
function commitInstrument(inst, after) { function commitInstrument(inst, after) {
+20 -2
View File
@@ -6,8 +6,8 @@ const fs = require('node:fs');
const path = require('node:path'); const path = require('node:path');
const vm = require('node:vm'); const vm = require('node:vm');
function load() { function load(seed) {
const store = {}; const store = Object.assign({}, seed);
const window = { const window = {
console, console,
localStorage: { localStorage: {
@@ -80,4 +80,22 @@ test('detectNewBadges notifies once per badge, never after it is seen', () => {
t.markBadgeSeen('guitar', 'blues'); t.markBadgeSeen('guitar', 'blues');
// JSON-compare: vm objects carry a foreign Object prototype. // JSON-compare: vm objects carry a foreign Object prototype.
assert.equal(JSON.stringify(t.seenBadges()), '{"guitar/blues":1}'); assert.equal(JSON.stringify(t.seenBadges()), '{"guitar/blues":1}');
// Fresh session (new vm, empty notify cache) with the badge already seen:
// detection must stay silent.
const w2 = load({ 'feedBack-career-badges-seen': '{"guitar/blues":1}' });
w2.__careerPassportTest.detectNewBadges(view);
assert.equal(w2.notifications.length, 0);
});
test('seenBadges tolerates corrupt stored values', () => {
for (const bad of ['null', '[1,2]', '"x"', '{{{']) {
const w = load({ 'feedBack-career-badges-seen': bad });
const t = w.__careerPassportTest;
assert.equal(JSON.stringify(t.seenBadges()), '{}', `stored ${bad}`);
// And detection still works on top of the recovered empty state.
t.detectNewBadges({ instruments: { guitar: { passports: [
{ genre_key: 'blues', genre: 'Blues', badge: 'earned' }] } } });
assert.equal(w.notifications.length, 1, `stored ${bad}`);
}
}); });