Merge pull request #582 from pbakaus/fix/build-path-flip-inspiration-stack

Flipping to comp demotes the inspiration instead of stacking a second slot

skill/scripts/serve-question.mjs

@@ -881,6 +881,7 @@ function page() {
 <div id="ambient" aria-hidden="true"></div>
 <div id="scrim" aria-hidden="true"></div>
 <div id="lightbox" hidden><img alt=""></div>
+<template id="tpl-expand-chip">${expandChip}</template>
 ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria-labelledby="bp-confirm-title" hidden>
   <div class="bp-confirm-panel">
     <h2 id="bp-confirm-title">Flip to comp-first?</h2>
@@ -1019,6 +1020,12 @@ ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria
   const landTracker = { last: Date.now() };
   const pollComp = (m) => {
     const url = m.dataset.comp;
+    // A converted slot is the SAME node across flip cycles, so a probe from an
+    // earlier cycle can still be in flight when the next one starts. Without a
+    // generation stamp its late onload settles the new slot: shimmer stripped,
+    // pending state cleared, comp still hidden, live poll stopped.
+    const generation = String(Number(m.dataset.pollGen || 0));
+    const current = () => String(Number(m.dataset.pollGen || 0)) === generation;
     const img = m.querySelector('img.comp');
     const note = m.querySelector('.comp-note');
     const started = Date.now();
@@ -1063,11 +1070,25 @@ ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria
       return true;
     };
     const tryLoad = () => {
-      // A slot the user flipped back out of leaves the DOM; let its loop die.
-      if (!m.isConnected) { clearInterval(tick); return; }
+      // A slot the user flipped back out of either leaves the DOM or, when it
+      // was an inspiration face converted in place, stays and loses its
+      // pending state. Either way its loop is done.
+      if (!m.isConnected || !m.classList.contains('comp-pending') || !current()) { clearInterval(tick); return; }
       const probe = new Image();
-      probe.onload = () => { landTracker.last = Date.now(); img.src = probe.src; img.hidden = false; settle(); };
+      probe.onload = () => {
+        // A stale generation also ends this run's clock. tryLoad clears it on
+        // re-entry, and an in-flight probe that finishes stale schedules no
+        // re-entry, so returning without clearing ran the interval for the rest
+        // of the page's life.
+        if (!current()) { clearInterval(tick); return; }
+        landTracker.last = Date.now();
+        const target = m.querySelector('img.comp') || img;
+        target.src = probe.src;
+        target.hidden = false;
+        settle();
+      };
       probe.onerror = () => {
+        if (!current()) { clearInterval(tick); return; }
         const quiet = Date.now() - landTracker.last > 240000;
         if (Date.now() - started > 240000 && quiet && fallback()) return;
         setTimeout(tryLoad, m.classList.contains('stand-in') ? 5000 : 2500);
@@ -1101,17 +1122,58 @@ ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria
       if (noteEl) noteEl.textContent = notes[value];
     };
     set(current);
+    const INSPO_TITLE = 'Inspiration: the world this direction draws from. Your page will not look like this image.';
+    // Every media slot carries the expand affordance. A slot built here after
+    // the deal used to ship without one, so a comp the user waited minutes for
+    // could not be opened.
+    const ensureChips = (m) => {
+      if (m.querySelector('.chips')) return;
+      const tpl = document.getElementById('tpl-expand-chip');
+      if (!tpl) return;
+      const chips = document.createElement('div');
+      chips.className = 'chips';
+      chips.appendChild(tpl.content.cloneNode(true));
+      m.appendChild(chips);
+    };
+    const shimmerHtml = '<div class="shimmer"><span class="comp-note">rendering&hellip;</span></div><img class="comp" alt="" hidden>';
     const enterComp = () => {
       document.querySelectorAll('.card[data-comp-slot]').forEach(card => {
         const front = card.querySelector('.face.front');
         if (!front || front.querySelector('.media.comp-pending') || front.querySelector('.media img.comp:not([hidden])')) return;
-        const m = document.createElement('div');
-        m.className = 'media comp-pending';
-        m.dataset.comp = card.dataset.compSlot;
-        m.innerHTML = '<div class="shimmer"><span class="comp-note">rendering&hellip;</span></div><img class="comp" alt="" hidden>';
-        const wireEl = front.querySelector('.media.wire');
-        if (wireEl) { wireEl.hidden = true; front.insertBefore(m, wireEl); }
-        else { front.classList.remove('text-only'); front.insertBefore(m, front.querySelector('.body')); }
+        // On a code-led card the inspiration IS the face. Comp-first demotes it
+        // to the corner, so convert that slot in place rather than inserting a
+        // second one: two stacked images say the catalog art and the comp are
+        // peers, and the whole point of the corner is that they are not.
+        const inspo = front.querySelector('.media:not(.wire):not(.comp-pending)');
+        let m;
+        if (inspo) {
+          m = inspo;
+          m.dataset.compRestore = 'inspiration';
+          m.dataset.comp = card.dataset.compSlot;
+          m.classList.add('comp-pending');
+          m.removeAttribute('title');
+          m.querySelector('.media-label')?.remove();
+          const art = m.querySelector(':scope > img');
+          if (art) {
+            const pip = document.createElement('figure');
+            pip.className = 'pip';
+            pip.title = INSPO_TITLE;
+            const cap = document.createElement('figcaption');
+            cap.textContent = 'inspiration';
+            pip.append(art, cap);
+            m.appendChild(pip);
+          }
+          m.insertAdjacentHTML('afterbegin', shimmerHtml);
+        } else {
+          m = document.createElement('div');
+          m.className = 'media comp-pending';
+          m.dataset.comp = card.dataset.compSlot;
+          m.innerHTML = shimmerHtml;
+          const wireEl = front.querySelector('.media.wire');
+          if (wireEl) { wireEl.hidden = true; front.insertBefore(m, wireEl); }
+          else { front.classList.remove('text-only'); front.insertBefore(m, front.querySelector('.body')); }
+        }
+        ensureChips(m);
         pollComp(m);
       });
     };
@@ -1120,6 +1182,34 @@ ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria
         const front = card.querySelector('.face.front');
         const pending = front?.querySelector('.media.comp-pending');
         if (!pending) return; // landed comps stay; they exist either way
+        // A converted slot is restored, not removed: the inspiration goes back
+        // to being the face, so flipping back leaves the card as it was dealt.
+        if (pending.dataset.compRestore === 'inspiration') {
+          // Everything the pending state added has to leave, presentation
+          // included: a slot that reached stand-in kept its "inspiration comp
+          // pending" label beside a fresh one, and a slot whose art had failed
+          // came back still marked unavailable.
+          pending.classList.remove('comp-pending', 'stand-in');
+          delete pending.dataset.compRestore;
+          delete pending.dataset.comp;
+          pending.dataset.pollGen = String(Number(pending.dataset.pollGen || 0) + 1);
+          pending.querySelector('.shimmer')?.remove();
+          pending.querySelector('img.comp')?.remove();
+          pending.querySelector('.stand-in-label')?.remove();
+          pending.querySelectorAll('.media-label').forEach((el) => el.remove());
+          const pip = pending.querySelector('.pip');
+          const art = pip?.querySelector('img');
+          if (art) pending.insertBefore(art, pending.firstChild);
+          pip?.remove();
+          // No art means the image failed to load before the flip, so hand the
+          // slot back to the same honest treatment rather than calling it art.
+          const label = document.createElement('p');
+          label.className = 'media-label';
+          label.textContent = art ? 'inspiration' : 'artwork unavailable';
+          pending.insertBefore(label, pending.querySelector('.chips'));
+          if (art) pending.title = INSPO_TITLE; else pending.classList.add('unavailable');
+          return;
+        }
         pending.remove();
         const wireEl = front.querySelector('.media.wire');
         if (wireEl) wireEl.hidden = false;
@@ -1181,15 +1271,42 @@ ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria
     else img.addEventListener('error', gone, { once: true });
   });
 
-  // Inspiration PIP or body thumb opens the full catalog card in the lightbox.
-  document.querySelectorAll('.pip, .inspo').forEach(p => p.addEventListener('click', (e) => {
-    e.stopPropagation();
-    const img = p.querySelector('img');
-    if (!img) return;
-    lightboxImg.src = img.getAttribute('src');
-    lightbox.hidden = false;
-    requestAnimationFrame(() => lightbox.classList.add('open'));
-  }));
+  // Every zoom target resolves in ONE delegated listener, in priority order.
+  // Delegation is what lets a slot built by a build-path flip work at all, and
+  // a single listener is what keeps the targets from fighting: stopPropagation
+  // ends bubbling, not other listeners on the same target, so split across two
+  // handlers a click on the corner inspiration opened the inspiration and then
+  // the comp overwrote it in the lightbox. Per-element handlers elsewhere (the
+  // flip chip, the raise cycler) still stop bubbling before the event lands
+  // here, so they keep their own behavior.
+  document.addEventListener('click', (e) => {
+    const target = e.target;
+    if (!target || !target.closest) return;
+    // The corner inspiration wins over the slot it sits inside.
+    const pip = target.closest('.pip, .inspo');
+    if (pip) {
+      const art = pip.querySelector('img');
+      if (art) openLightbox(art);
+      return;
+    }
+    const chip = target.closest('.chip');
+    if (chip) {
+      // Only expand zooms. Any other chip owns its click and must not fall
+      // through to the media underneath it.
+      if (!chip.classList.contains('expand')) return;
+      const card = chip.closest('.card');
+      const face = card && card.classList.contains('flipped') ? '.face.back' : '.face.front';
+      const shown = card && card.querySelector(face + ' .media img:not([hidden])');
+      if (shown && shown.getAttribute('src')) openLightbox(shown);
+      return;
+    }
+    // The whole image is the zoom target, not just the chip; the chip stays as
+    // the visible affordance.
+    const media = target.closest('.media');
+    if (!media) return;
+    const art = media.querySelector(':scope > img:not([hidden])');
+    if (art && art.getAttribute('src')) openLightbox(art);
+  });
 
   // Deck paging: arrows appear only when the deck overflows its axis, page
   // one card at a time, and follow the aspect-ratio flip between row and column.
@@ -1239,16 +1356,13 @@ ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria
   // Expand: lightbox for whichever face is showing.
   const lightbox = document.getElementById('lightbox');
   const lightboxImg = lightbox.querySelector('img');
-  document.querySelectorAll('.expand').forEach(b => b.addEventListener('click', (e) => {
-    e.stopPropagation();
-    const card = b.closest('.card');
-    const face = card.classList.contains('flipped') ? '.face.back' : '.face.front';
-    const img = card.querySelector(face + ' .media img:not([hidden])');
-    if (!img || !img.getAttribute('src')) return;
+  // Declared, not assigned to a const, so the delegated handlers above can call
+  // it wherever they sit in this file.
+  function openLightbox(img) {
     lightboxImg.src = img.getAttribute('src');
     lightbox.hidden = false;
     requestAnimationFrame(() => lightbox.classList.add('open'));
-  }));
+  }
   // Portrait art (native / mobile-first surfaces): the slot takes the
   // image's own ratio so nothing crops, and the whole deck narrows so
   // portrait cards sit side by side. Load events don't bubble; capture.
@@ -1262,17 +1376,6 @@ ${buildPath?.toggle ? `<div id="bp-confirm" role="dialog" aria-modal="true" aria
       document.querySelector('.grid')?.classList.add('portrait-media');
     }
   }, true);
-
-  // The whole image is the zoom target, not just the expand chip; the chip
-  // stays as the visible affordance. Chip and PIP handlers stop propagation,
-  // so this fires only for clicks on the art itself.
-  document.querySelectorAll('.media').forEach(m => m.addEventListener('click', () => {
-    const img = m.querySelector(':scope > img:not([hidden])');
-    if (!img || !img.getAttribute('src')) return;
-    lightboxImg.src = img.getAttribute('src');
-    lightbox.hidden = false;
-    requestAnimationFrame(() => lightbox.classList.add('open'));
-  }));
   const closeLightbox = () => { lightbox.classList.remove('open'); setTimeout(() => { lightbox.hidden = true; }, 250); };
   lightbox.addEventListener('click', closeLightbox);
   document.addEventListener('keydown', (e) => { if (e.key === 'Escape' && !lightbox.hidden) closeLightbox(); });

tests/new-work-e2e.test.mjs

@@ -609,6 +609,92 @@ describe('new-work-e2e: serve-question decision page', () => {
     }
   });
 
+  // The flip used to be tested only on a wireframe card, where the schematic
+  // is hidden and a fresh slot inserted. A card carrying catalog art instead
+  // took the other branch: the inspiration stayed the face and the comp slot
+  // was inserted below it, so the card showed two stacked images. Comp-first
+  // demotes the inspiration to the corner, and a flip has to reach the same
+  // shape a comp-first render would have served.
+  it('(f2) flipping to comp demotes an inspiration face to the corner instead of stacking a second slot', async () => {
+    const cwd = makeWorkspace();
+    const key = 'flipinspo';
+    const hero = makeFakeImage(cwd, 'catalog inspiration', 'inspo.png');
+    const payload = {
+      title: 'Choose the visual world',
+      buildPath: { value: 'code', toggle: true },
+      options: [
+        {
+          id: 'assigned', label: 'The Ledger Spine', kicker: 'THE ROLL', hero,
+          comp: '.impeccable/mocks/decision/assigned.webp',
+          viewport: 'A ruled masthead over a dense grid.', risk: 'Reads editorial.',
+        },
+      ],
+    };
+    const { url } = await startDaemon(cwd, payload, key);
+    try {
+      const context = await browser.newContext();
+      const page = await context.newPage();
+      await page.goto(url, { waitUntil: 'load' });
+
+      const front = '.card[data-id="assigned"] .face.front';
+      // Code-led: the catalog art is the face, labeled so it never reads as a promise.
+      assert.equal(await page.$$eval(`${front} .media`, (els) => els.length), 1, 'one media slot before the flip');
+      assert.ok(await page.$(`${front} .media .media-label`), 'the inspiration face is labeled');
+      assert.equal(await page.$(`${front} .pip`), null, 'no corner inspiration while code-led');
+
+      await page.click('.bp-opt[data-bp="comp"]');
+      await page.waitForSelector('#bp-confirm:not([hidden])');
+      await page.click('#bp-confirm [data-confirm]');
+      await page.waitForSelector(`${front} .media.comp-pending`);
+
+      // The whole point: one slot, not two.
+      assert.equal(await page.$$eval(`${front} .media`, (els) => els.length), 1,
+        'the flip converts the inspiration slot rather than stacking a second one');
+      assert.ok(await page.$(`${front} .media.comp-pending .pip img`), 'the inspiration moved into the corner');
+      assert.equal(await page.$(`${front} .media > .media-label`), null, 'it is no longer presented as the face');
+      assert.ok(await page.$(`${front} .media .chip.expand`), 'the converted slot keeps its expand affordance');
+
+      const flipped = await waitLoop(cwd, key, { poll: 10 });
+      assert.match(flipped.out, /BUILD PATH FLIPPED: comp/);
+
+      // A comp that streams in must be openable. The zoom handlers used to be
+      // bound once at deal time, so anything built by the flip was inert.
+      mkdirSync(path.join(cwd, '.impeccable', 'mocks', 'decision'), { recursive: true });
+      makeFakeImage(path.join(cwd, '.impeccable', 'mocks', 'decision'), 'ledger spine comp', 'assigned.webp');
+      await page.waitForSelector(`${front} .media img.comp:not([hidden])`, { timeout: 15000 });
+      await page.click(`${front} .media .chip.expand`);
+      await page.waitForSelector('#lightbox:not([hidden])');
+      const compSrc = await page.$eval('#lightbox img', (img) => img.getAttribute('src'));
+      assert.ok(compSrc, 'the streamed-in comp opens full screen');
+      await page.click('#lightbox');
+      await page.waitForSelector('#lightbox', { state: 'hidden' });
+
+      // The corner inspiration opens the catalog art, not the comp behind it.
+      // Both zoom targets are delegated to document, where stopPropagation
+      // cannot stop a sibling listener, so split handlers let the media one
+      // overwrite the lightbox the pip had just filled.
+      const cornerSrc = await page.$eval(`${front} .media .pip img`, (img) => img.getAttribute('src'));
+      await page.click(`${front} .media .pip`);
+      await page.waitForSelector('#lightbox:not([hidden])');
+      const pipSrc = await page.$eval('#lightbox img', (img) => img.getAttribute('src'));
+      assert.notEqual(pipSrc, compSrc, 'the corner opens the inspiration, not the comp');
+      assert.equal(pipSrc, cornerSrc, 'and it is exactly the art in the corner');
+      await page.click('#lightbox');
+      await page.waitForSelector('#lightbox', { state: 'hidden' });
+
+      // Flipping back keeps a comp that already landed, by design, and must
+      // still leave exactly one slot. exitComp is synchronous, so there is
+      // nothing to wait for.
+      await page.click('.bp-opt[data-bp="code"]');
+      assert.equal(await page.$$eval(`${front} .media`, (els) => els.length), 1, 'still one slot after flipping back');
+      assert.ok(await page.$(`${front} .media img.comp:not([hidden])`), 'the landed comp survives the flip back');
+      await context.close();
+    } finally {
+      await stopDaemon(cwd, key);
+      rmSync(cwd, { recursive: true, force: true });
+    }
+  });
+
   it('(g) a wireframe card draws its schematic in the media slot and keeps its full read on the front', async () => {
     const cwd = makeWorkspace();
     const key = 'wire';