From 50f88a3e5ede006da05e66bf3953b716c091c9e7 Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Wed, 23 Sep 2026 08:57:36 -0700 Subject: [PATCH] feat(r2): C5 the lightbox, and the in-place client - On a gallery booth the marks panel moves into a sticky verdict aside beside the set. The aside comes first in the document, so a narrow screen stacks the question above the work; grid areas place it on the right when wide. Nothing in an ordered collection moves. Boards are unchanged. - The flag tray lists flagged items by tile number: the declared change from the panel list's (created, id). The standalone marks page keeps the list. - Inline group headers are divs, never figure.item. - Every mark-dependent element is a data-region: the verdict, each tile, the rail's filter counts. There is also a server-rendered status line. - The in-place script (base.html) POSTs with an explicit JSON Accept, then on 204 swaps every region from a fresh GET. Live media and per-viewer view state are carried across the swap, so there is no layout jolt and no stopped track. It never re-POSTs: on failure it says so and reloads. Tile controls re-bind after a swap, and the grid cursor survives it. - The `n` key opens the tile's closed note disclosure before focusing it. - test_embed_browser's keyboard-flag test expected a navigation, which is the defect R2 removes. It is updated as declared in the contract, and tightened: a window marker must survive, proving no reload. Browser tests: flag in place, with no reload and no scroll jump, and the tile, tray and rail count all updated; and a failed save that reloads without re-POSTing. Two mutations turn them red (no carry, no rail region). 700 passed. --- booth/app.py | 6 ++ booth/templates/_marks.html | 58 +++++++---- booth/templates/base.html | 130 +++++++++++++++++++++++- booth/templates/booth.html | 158 +++++++++++++---------------- docs/contracts/r2_flow.contract.md | 2 + tests/test_embed_browser.py | 17 +++- tests/test_flow.py | 69 +++++++++++++ tests/test_flow_browser.py | 114 +++++++++++++++++++++ 8 files changed, 445 insertions(+), 109 deletions(-) create mode 100644 tests/test_flow_browser.py diff --git a/booth/app.py b/booth/app.py index b48db1f..1852388 100644 --- a/booth/app.py +++ b/booth/app.py @@ -1139,6 +1139,12 @@ def create_app( it["name"]: marks_for_target(marks, it["name"]) for it in gallery }, "booth_marks": marks_for_target(marks, None), + # R2 C5: the flag tray — the flagged items in SET order, i.e. + # by ordinal, over the full gallery (a filter hides tiles, not + # judgments). Order is total with no tie-break: rels are unique. + "tray": [it for it in gallery if it["name"] in { + m.target for m in marks if m.shape == "flag" and m.error is None}], + "ord_width": len(str(len(gallery))), "uploaded": (booth / UPLOAD_MARKER).exists(), # The same provenance line the index card carries. Deliberate: # a booth URL handed to the operator lands HERE, never on the diff --git a/booth/templates/_marks.html b/booth/templates/_marks.html index 34beeb3..de38719 100644 --- a/booth/templates/_marks.html +++ b/booth/templates/_marks.html @@ -32,7 +32,7 @@ ⚠ broken {{ a.id }} -
+ {% if marks_page %}{% endif %} @@ -75,7 +75,7 @@ {% endif %}
{% if a.answer %}change answer{% else %}answer{% endif %} - + {# The field is still `ask`: inline fragments in reports the operator has already published POST that name, and breaking every landed verbatim report to tidy a form field is not a trade worth making. #} @@ -117,25 +117,26 @@ {% endfor %} -{% for a in notes %} -
+{# FLAGS come right after the picks. On the lightbox (`tray` defined) they + render as the TRAY: the flagged items in SET order — by tile number, the + declared R2 change from the click order below — each the original shown + small, blurred if the item is. The standalone marks page has no item + records, so it keeps the list, in `(created, id)` order. #} +{% if tray is defined and tray %} + -{% endfor %} - -{% if flags %} +{% elif tray is not defined and flags %}
✔ flagged @@ -149,9 +150,28 @@
{% endif %} +{% for a in notes %} +
+
+ note + {% if a.target %}on {{ a.target }} + {% else %}on this booth{% endif %} + + {{ a.created }}{% if a.by %} · {{ a.by }}{% endif %} +
+ + {% if marks_page %}{% endif %} + +
+
+
{{ a.text }}
+
+{% endfor %} + + {# The operator volunteering a remark, which before marks had no mechanism at all — this is the direction that was running through chat. #} -
+ {% if marks_page %}{% endif %} diff --git a/booth/templates/base.html b/booth/templates/base.html index da1b13d..93e9fac 100644 --- a/booth/templates/base.html +++ b/booth/templates/base.html @@ -419,6 +419,52 @@ .mark-add textarea,.vaddnote textarea,.item-addnote textarea{flex:1;min-width:0} .mark-add{margin-top:2px} + /* ---- THE LIGHTBOX (R2 C5) ---------------------------------------------- + The set on the left, the verdict on the right — placed by grid AREAS, not + by `order`: the aside is FIRST in the document so a narrow screen stacks + the question above the work without anything being reordered. The aside + is sticky and scrolls on its own, so the question stays beside tile 40. */ + main:has(.lightbox){max-width:1560px} + .lightbox{display:grid;grid-template-columns:minmax(0,1fr) 360px;grid-template-areas:"set verdict"; + gap:var(--space-6);align-items:start} + .lb-set{grid-area:set;min-width:0} + .verdict{grid-area:verdict;position:sticky;top:12px;max-height:calc(100vh - 24px);overflow:auto; + border:1px solid var(--border-default);border-radius:var(--radius-xl);background:var(--surface-base); + padding:12px;scrollbar-width:thin} + .verdict .marks{margin:0} + .verdict .mark-prompt{font-size:var(--size-body)} + .verdict .mark-add{flex-direction:column;align-items:stretch} + .verdict .mark-add button{align-self:flex-end} + @media (max-width:1000px){ + .lightbox{grid-template-columns:1fr;grid-template-areas:"verdict" "set"} + .verdict{position:static;max-height:none} + } + /* the flag tray: the flagged items in set order, the originals shown small */ + .tray{display:grid;grid-template-columns:repeat(auto-fill,minmax(64px,1fr));gap:6px;padding:12px 14px} + .tray-item{position:relative;display:block;aspect-ratio:1;border-radius:var(--radius-md);overflow:hidden; + border:1px solid var(--success);background:var(--surface-sunken)} + .tray-item img{width:100%;height:100%;object-fit:cover;display:block} + .tray-item.is-blurred img{filter:blur(8px)} + .tray-kind{display:flex;align-items:center;justify-content:center;height:100%;font-family:var(--font-mono); + font-size:var(--size-micro);text-transform:uppercase;color:var(--text-muted)} + .tray-ord{position:absolute;left:3px;bottom:3px;padding:1px 4px;border-radius:var(--radius-sm); + font:600 10px/1.2 var(--font-mono);background:oklch(0.17 0.01 250 / .85);color:oklch(0.91 0.008 216)} + .tray-item:hover{text-decoration:none;box-shadow:0 0 0 1px var(--success)} + /* ordinals: the item's number in the whole set (C1) */ + .item > .ord{position:absolute;top:10px;left:10px;z-index:2;padding:3px 6px;border-radius:var(--radius-sm); + font:600 11px/1.2 var(--font-mono);background:oklch(0.17 0.01 250 / .82);color:oklch(0.91 0.008 216); + pointer-events:none} + .item.blurred > .ord{top:44px} + .doc-bar .ord{font:600 11px/1 var(--font-mono);color:var(--text-muted)} + /* the inline group header: spans the grid, is never a tile */ + .grp-head{grid-column:1 / -1;display:flex;align-items:center;gap:8px;margin:8px 0 -4px; + font-family:var(--font-mono);font-size:var(--size-micro);letter-spacing:var(--tracking-caps);text-transform:uppercase} + .grp-head::after{content:"";flex:1;height:1px;background:var(--border-subtle)} + .grp-key{color:var(--text-heading);font-weight:600} + .grp-n{color:var(--text-muted)} + .status{margin:0 0 12px;padding:8px 12px;border-radius:var(--radius-md);background:var(--warning-soft); + color:var(--warning-text);font-size:var(--size-sm)} + /* ---- the gallery ------------------------------------------------------ */ /* THE RAIL. Sticky, opaque, and counted in mono because the counts are records. The active filter is a place you are, not an armed thing, so it @@ -726,7 +772,89 @@ The Booth ephemeral media · auto-wipes {{ ttl_hours }}h · kept boards don't -
{% block content %}{% endblock %}
+
+{# R2 C3: where the in-place script says it could not save in place. Server + rendered and empty, so the script only ever sets its text — it builds no + markup (INV-6). #} + +{% block content %}{% endblock %}
+ {% endif %} @@ -500,17 +535,30 @@ }); })(); - /* Inline-doc ✕ closes (hides) a rendered doc for the session. The button sits + /* TILE CONTROLS, bound per node and RE-BOUND after an in-place swap (R2 C3): + the swap puts fresh server-rendered tiles in the page, and a handler bound + to the node it replaced goes with that node. `__bound` keeps a node from + being bound twice. + + Inline-doc ✕ closes (hides) a rendered doc for the session. The button sits inside , so without this its click would just toggle the
open/closed — stopPropagation + preventDefault make ✕ mean "close", not "collapse". Collapse stays available via the rest of the summary bar. With - JS off the button is inert and collapse via
still works. */ - (function () { + JS off the button is inert and collapse via
still works. + + Blur reveal. WARNING: this handler previously sat after the content block's + closing tag, which in a child template Jinja DISCARDS — the button rendered + and did nothing, and two commits plus a README claimed click-to-reveal + worked. Anything that must reach the page belongs inside the content + block. Per-viewer and never persisted: a reload re-hides. */ + function bindTiles() { + function once(el) { if (el.__bound) return false; el.__bound = true; return true; } /* A form inside would otherwise collapse the doc on submit. */ - document.querySelectorAll('.doc-bar .blurtoggle').forEach(function (f) { - f.addEventListener('click', function (ev) { ev.stopPropagation(); }); + document.querySelectorAll('.doc-bar .blurtoggle, .doc-bar .flagtoggle').forEach(function (f) { + if (once(f)) f.addEventListener('click', function (ev) { ev.stopPropagation(); }); }); document.querySelectorAll('.doc-close').forEach(function (btn) { + if (!once(btn)) return; btn.addEventListener('click', function (ev) { ev.preventDefault(); ev.stopPropagation(); @@ -518,80 +566,20 @@ if (item) item.classList.add('is-closed'); }); }); - })(); - - /* Link-board multi-select. PROGRESSIVE ENHANCEMENT: the checkboxes, the per-row - × / ★, and the bulk 🗑 all submit as plain form POSTs with JS off — this only - adds select-all, a live count, and disabling 🗑 when nothing is ticked. The - per-row × confirm reads desc/url from data-* attributes rather than being - interpolated into an inline handler, so an arbitrary agent-posted description - (quotes, newlines) can never break out into the page's JS. */ - (function () { - var form = document.getElementById('boardform'); - if (!form) return; - var boxes = Array.prototype.slice.call(form.querySelectorAll('.board-check')); - var selall = document.getElementById('board-selall'); - var delBtn = document.getElementById('board-del-sel'); - var countEl = document.getElementById('board-selcount'); - - function selected() { return boxes.filter(function (b) { return b.checked; }); } - function refresh() { - var n = selected().length; - if (countEl) countEl.textContent = n; - if (delBtn) delBtn.disabled = n === 0; - if (selall) { - selall.checked = n > 0 && n === boxes.length; - selall.indeterminate = n > 0 && n < boxes.length; - } - } - if (selall) { - selall.addEventListener('change', function () { - boxes.forEach(function (b) { b.checked = selall.checked; }); - refresh(); - }); - } - boxes.forEach(function (b) { b.addEventListener('change', refresh); }); - - // Bulk delete: confirm with the count. Attached to the button (not the form's - // submit) so the per-row × / ★ submits — which share this form — are unaffected. - if (delBtn) { - delBtn.addEventListener('click', function (ev) { - var n = selected().length; - if (n === 0) { ev.preventDefault(); return; } - if (!confirm('Delete ' + n + ' selected link' + (n === 1 ? '' : 's') + '?\n\nThe rest of the board is untouched.')) { - ev.preventDefault(); - } - }); - } - - form.querySelectorAll('.board-rm-btn').forEach(function (btn) { - btn.addEventListener('click', function (ev) { - var d = btn.getAttribute('data-desc') || ''; - var u = btn.getAttribute('data-url') || ''; - if (!confirm('Remove this link?\n\n' + d + '\n' + u + '\n\nThe rest of the board is untouched.')) { - ev.preventDefault(); - } - }); - }); - - refresh(); - })(); - - /* Blur reveal. WARNING: this handler previously sat after the content - block's closing tag, which in a - child template Jinja DISCARDS — the button rendered and did nothing, and - two commits plus a README claimed click-to-reveal worked. Anything that - must reach the page belongs inside the content block. Verified now by - grepping the SERVED html for this function, not the template for the text. - Per-viewer and never persisted: a reload re-hides. */ - document.querySelectorAll('.item.blurred .reveal').forEach(function (btn) { - btn.addEventListener('click', function (ev) { - ev.preventDefault(); - ev.stopPropagation(); + document.querySelectorAll('.item.blurred .reveal').forEach(function (btn) { var fig = btn.closest('.item'); - var on = fig.classList.toggle('revealed'); - btn.textContent = on ? '🙈 hide' : '👁 reveal'; + /* a swap carries `revealed` across (base.html); the label follows it */ + btn.textContent = fig.classList.contains('revealed') ? '🙈 hide' : '👁 reveal'; + if (!once(btn)) return; + btn.addEventListener('click', function (ev) { + ev.preventDefault(); + ev.stopPropagation(); + var on = fig.classList.toggle('revealed'); + btn.textContent = on ? '🙈 hide' : '👁 reveal'; + }); }); - }); + } + bindTiles(); + document.addEventListener('booth:swapped', bindTiles); {% endblock %} diff --git a/docs/contracts/r2_flow.contract.md b/docs/contracts/r2_flow.contract.md index 7817463..4d9f9f5 100644 --- a/docs/contracts/r2_flow.contract.md +++ b/docs/contracts/r2_flow.contract.md @@ -33,6 +33,7 @@ touches: - "booth/static/embed.js (NOT TOUCHED — the verbatim path keeps its author's layout; requirement 6)" - "tests/test_booth.py (THREE assertions change, all in test_index_separates_kept_from_ephemeral: L785-786, the kept-lane presence pair, and L789, kept-before-ephemeral. L810-811, the absence pair, survive unchanged. See 'Assertions that change')" - "tests/test_flow.py (NEW)" + - "tests/test_embed_browser.py (ONE test changes: test_the_keyboard_flag_actually_submits expected a navigation, which is the defect R2 removes. See 'Assertions that change')" assumptions: - "ONE VIEWER. `.seen` records what has been seen at full size, not WHO saw it. ROADMAP parks 'per-viewer state (who has seen what)' on the one-viewer premise; this contract keeps that premise and does not reopen the parked item." - "EVERY JUDGMENT WORKS WITH JAVASCRIPT OFF. Each control stays a plain . The in-place behaviour is additive and falls back to today's 303." @@ -389,6 +390,7 @@ This applies to image, video and audio items. Docs keep `doc.html`. |---|---|---|---| | test_booth.py L785 | `class="grid kept-grid"` present when a booth is kept | absent; the kept booth appears in its Desk section with the `kept` lifetime line | requirement 8: the lanes sort nothing | | test_booth.py L786 | `class="card card-kept"` present | replaced by the row carrying `data-kept="1"` | same | +| test_embed_browser.py `test_the_keyboard_flag_actually_submits` | pressing `f` causes a NAVIGATION (`page.expect_navigation()`), and the reloaded page shows the flag | pressing `f` causes NO navigation; the flag comes back from the server into the swapped tile. A window marker set before the keypress must survive, proving no reload | requirement 3: the reload IS the defect. The test's real claim — the key reaches the server and the server's state comes back — is kept, and asserted more strictly | | test_booth.py L789 | the kept booth renders BEFORE the ephemeral one (`html.index("links") < html.index("scratch")`) | replaced by the Desk's stated order (needs → new → everything, each with its own key) | the kept-first order was the lane's; with no lane there is no kept-first rule, and a second hidden ordering would break INV-2 | | test_booth.py L810-811 | lane absent when nothing is kept | these two SURVIVE unchanged (they assert absence and stay true) | — | diff --git a/tests/test_embed_browser.py b/tests/test_embed_browser.py index 0c18364..a8affea 100644 --- a/tests/test_embed_browser.py +++ b/tests/test_embed_browser.py @@ -592,17 +592,26 @@ def test_the_keyboard_flag_actually_submits(browser, live): a hidden input does not submit its form. The shortcut never worked while still swallowing the keystroke. - Asserted end to end: press f, and the flag must come back from the server - on the reloaded page.""" + Asserted end to end: press f, and the flag must come back from the server. + + R2 C3 (docs/contracts/r2_flow.contract.md, "Assertions that change"): this + used to expect a NAVIGATION — the flag form POSTed and the page reloaded. + That reload is the defect R2 removes. The claim that matters is kept and + tightened: the flag must come back from the SERVER (the swapped tile is + server-rendered), and a marker set on the window before the keypress must + survive, which a reload would wipe.""" base, root = live _gallery(root) page = browser.new_page() page.goto(f"{base}/b/g/", wait_until="networkidle") + page.evaluate("window.__noReload = 1") page.keyboard.press("ArrowRight") # cursor onto the first tile - with page.expect_navigation(): # the flag form POSTs and redirects back - page.keyboard.press("f") + page.keyboard.press("f") + page.wait_for_selector("figure.item.is-flagged", timeout=10000) flagged = page.locator("figure.item.is-flagged").count() + survived = page.evaluate("window.__noReload === 1") page.close() assert flagged == 1, f"the f key flagged {flagged} items, expected 1" + assert survived, "the flag reloaded the page; in-place judgment must not" diff --git a/tests/test_flow.py b/tests/test_flow.py index fe66fb5..2c2606d 100644 --- a/tests/test_flow.py +++ b/tests/test_flow.py @@ -407,3 +407,72 @@ def test_a_booth_without_images_shows_its_kind_instead(tmp_path): _booth(tmp_path, "songs", {"a.mp3": b"ID3", "b.mp3": b"ID3"}) row = re.search(r'data-booth="songs".*?
', _client(tmp_path).get("/").text, re.S).group(0) assert "♪ audio" in row and " str: + m = re.search(r'<(\w+)[^>]*data-region="%s"[^>]*>' % re.escape(rid), body) + assert m, f"no region {rid}" + tag = m.group(1) + # regions in these templates do not nest a same-named tag inside themselves + end = body.index(f"", m.end()) + return body[m.start():end] + + +def test_the_verdict_sits_beside_the_set_on_a_gallery_booth(tmp_path): + """The tracer for C5: the open question, the flags and the notes live in + one aside next to the grid — not in a panel above it that scrolls away.""" + from booth.marks import declare_pick + b = _booth(tmp_path, "g", {"a.png": PNG, "b.png": PNG}) + declare_pick(b, "q", {"prompt": "Which one?", "options": ["a", "b"]}) + body = _client(tmp_path).get("/b/g/").text + aside = _region(body, "verdict") + assert aside.startswith('