From 36e91574e8f7c4a0d4a30b704addaf86946b6c67 Mon Sep 17 00:00:00 2001
From: byrongamatos <xasiklas@gmail.com>
Date: Mon, 13 Jul 2026 12:01:17 +0200
Subject: [PATCH] =?UTF-8?q?fix(career):=20passport=20review=20polish=20?=
 =?UTF-8?q?=E2=80=94=20a11y=20semantics=20+=20seen-state=20guard?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 CHANGELOG.md                           |  5 +++++
 plugins/career/assets/career.css       |  1 +
 plugins/career/screen.html             |  8 ++++----
 plugins/career/screen.js               | 22 +++++++++++++++++++---
 plugins/career/tests/passports.test.js | 22 ++++++++++++++++++++--
 5 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 494a943..13e556e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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).
 
 ### 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
   'appstate'`).** feedback-desktop's `scripts/bundle-slopsmith.sh` copies a *hardcoded
   list* of core files into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`,
diff --git a/plugins/career/assets/career.css b/plugins/career/assets/career.css
index b21d9f2..b6c09b9 100644
--- a/plugins/career/assets/career.css
+++ b/plugins/career/assets/career.css
@@ -257,6 +257,7 @@
 /* The rubber stamp */
 .pp-stamp {
     --pp-rot: 0deg;
+
     position: relative;
     display: flex;
     flex-direction: column;
diff --git a/plugins/career/screen.html b/plugins/career/screen.html
index 756f369..3fa5047 100644
--- a/plugins/career/screen.html
+++ b/plugins/career/screen.html
@@ -4,11 +4,11 @@
         <div id="career-stars-summary" class="text-sm text-gray-400"></div>
     </div>
     <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="passports" role="tab">Passports</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" id="career-tab-btn-passports" aria-controls="career-tab-passports">Passports</button>
     </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>
         <div id="career-progress-wrap" class="mb-6">
             <div class="career-bar-track">
@@ -26,7 +26,7 @@
         </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>
         <div id="pp-instruments" class="pp-instruments"></div>
         <div id="pp-shelf-wrap" class="mt-5">
diff --git a/plugins/career/screen.js b/plugins/career/screen.js
index ff6641f..a6ffe88 100644
--- a/plugins/career/screen.js
+++ b/plugins/career/screen.js
@@ -33,6 +33,7 @@
     let _pp = null;              // last /passports view
     let _ppRelayTimer = 0;
     let _ppBook = null;          // {inst, gkey} of the open spread
+    let _ppReturnFocus = null;   // element to refocus when the book closes
     let _ppBootstrapped = false;
     let _ppNotified = {};        // badges chimed this session (slam still pending)
 
@@ -276,7 +277,9 @@
         venues.classList.toggle('hidden', tab !== 'venues');
         pp.classList.toggle('hidden', tab !== 'passports');
         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() {
-        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; }
@@ -475,7 +483,7 @@
             : `Play ${esc(p.genre)} songs at ${starGl} to collect ticket stubs.`;
         const stubsHTML = stubs.length ? stubs.map(ppStubHTML).join('')
             : `<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-page pp-page-left">
                     <div class="pp-page-head">${esc(p.genre)} — ${esc(ppLabel(inst))}</div>
@@ -501,9 +509,12 @@
         const overlay = $('pp-overlay');
         if (!p || !overlay) return;
         _ppBook = { inst, gkey };
+        _ppReturnFocus = document.activeElement;
         const pending = p.badge === 'earned' && !seenBadges()[badgeId(inst, gkey)];
         overlay.innerHTML = ppBookHTML(inst, p, pending);
         overlay.classList.remove('hidden');
+        const close = overlay.querySelector('.pp-book-close');
+        if (close) close.focus();
         sfx('page');
         // Double rAF so the cover's closed state paints before the transition.
         requestAnimationFrame(() => requestAnimationFrame(() => {
@@ -530,6 +541,11 @@
         _ppBook = null;
         const overlay = $('pp-overlay');
         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) {
diff --git a/plugins/career/tests/passports.test.js b/plugins/career/tests/passports.test.js
index 7ed4cfe..dc39963 100644
--- a/plugins/career/tests/passports.test.js
+++ b/plugins/career/tests/passports.test.js
@@ -6,8 +6,8 @@ const fs = require('node:fs');
 const path = require('node:path');
 const vm = require('node:vm');
 
-function load() {
-    const store = {};
+function load(seed) {
+    const store = Object.assign({}, seed);
     const window = {
         console,
         localStorage: {
@@ -80,4 +80,22 @@ test('detectNewBadges notifies once per badge, never after it is seen', () => {
     t.markBadgeSeen('guitar', 'blues');
     // JSON-compare: vm objects carry a foreign Object prototype.
     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}`);
+    }
 });