diff --git a/booth/templates/base.html b/booth/templates/base.html index 2846331..986768a 100644 --- a/booth/templates/base.html +++ b/booth/templates/base.html @@ -860,10 +860,13 @@ /* The sides share ONE set of rows (subgrid): a caption under A takes its height from both stages alike, never from A's alone — two stages of different sizes would draw the same picture at two scales in Fit. */ - .cmp-body{display:grid;grid-template-columns:minmax(0,1fr) minmax(0,1fr); - grid-template-rows:auto minmax(0,1fr) auto;min-height:0} - .cmp-side{display:grid;grid-row:1 / -1;grid-template-rows:subgrid;min-width:0;min-height:0} - .cmp-side+.cmp-side{border-left:1px solid var(--border-subtle)} + /* The separator is a 1px column GAP showing the body's background, never a + border on B: a border comes out of one side's width alone, and the two + stages must be the same size to the pixel (heid code-review, hulda). */ + .cmp-body{display:grid;grid-template-columns:minmax(0,1fr) minmax(0,1fr);column-gap:1px; + background:var(--border-subtle);grid-template-rows:auto minmax(0,1fr) auto;min-height:0} + .cmp-side{display:grid;grid-row:1 / -1;grid-template-rows:subgrid;min-width:0;min-height:0; + background:var(--surface-sunken)} .cmp-head{display:flex;align-items:center;gap:8px;min-width:0;padding:8px 12px; border-bottom:1px solid var(--border-subtle);background:var(--surface-base)} .cmp-label{flex:1;min-width:0;font-family:var(--font-mono);font-size:var(--size-caption);color:var(--text-body); @@ -909,7 +912,7 @@ .viewer.review.compare{display:block} .cmp-body{grid-template-columns:minmax(0,1fr);grid-template-rows:none} .cmp-side{grid-row:auto;grid-template-rows:auto auto auto} - .cmp-side+.cmp-side{border-left:0;border-top:1px solid var(--border-subtle)} + .cmp-side+.cmp-side{border-top:1px solid var(--border-subtle)} .review.compare .vstage{height:45vh} .cmp-keys{display:none} } diff --git a/docs/contracts/r3_compare.contract.md b/docs/contracts/r3_compare.contract.md index f900ddd..63ea0bf 100644 --- a/docs/contracts/r3_compare.contract.md +++ b/docs/contracts/r3_compare.contract.md @@ -47,7 +47,8 @@ assumptions: - **A missing or empty param is a 404, not FastAPI's 422.** The review declares `f: str` and so answers 422 when `f` is absent. Compare declares `a: str = ""` and `b: str = ""`, 404s an empty one, and checks each with the - same `isinstance(str)` the view branch gives `f`. + same `isinstance(str)` that `_mark_redirect`'s `back=view` branch gives `f` + (the view route itself declares `f: str` and checks nothing more). - **Route order:** the compare route is registered BEFORE the catch-all `/b/{name}/{filepath:path}`, as view is. Accepted, and written down: a booth FILE literally named `compare` is unreachable at `/b//compare`. This @@ -71,8 +72,9 @@ assumptions: - Each frame is marked `A`, `B`, or nothing (both marks when `a == b`). - With JS, a click on a frame sets the ACTIVE side to that item and stays on the compare page. The active side defaults to B. - - The active side wears the SVOS reticle (the one selection device). A click - on either stage, or the key `X`, makes that side (or the other) active. + - The active side wears the SVOS reticle (the one selection device). A press + on either stage (pointerdown, so starting a 1:1 pan there also makes it + active), or the key `X`, makes that side (or the other) active. - **The active side lives on a NON-region element**, the side's wrapper around its stage, and in the URL's `side` (C2), rewritten in place when it changes. An in-place save swaps regions (strip, labels, flags), and @@ -131,8 +133,9 @@ assumptions: review's. - **The two stages are always the same size.** The sides share one set of rows (subgrid), so a caption under one side takes its height from both stages, - never from that side's alone. Two stages of different sizes would draw the - same picture at two scales in Fit. + never from that side's alone, and the separator between them is a column + gap, never a border that comes out of one side's width. Two stages of + different sizes would draw the same picture at two scales in Fit. - **At phone width (600px and below) a top bar that cannot hold its controls WRAPS** instead of scrolling the page sideways or crushing a control. This applies to the review's bar too, which gains the Compare control (only its @@ -245,8 +248,9 @@ assumptions: - compare.html carries `{% block html_attrs %} data-booth="{{ name }}"`, because without it Reveal all's script and the head script's reveal restore both bail (S6, r2b's mutation row for the review). -- **Each side's caption** shows under its stage, clamped to 20vh with its own - scroll, the same element style as the review's `.vcap`. +- **Each side's caption** shows under its stage in `.cmp-cap`: the review's + `.vcap` type (size, leading, colour, pre-wrap), clamped to 20vh rather than + the review's 30vh, with its own scroll. Two sides share the height. ## Invariants @@ -261,7 +265,8 @@ assumptions: would apply a stored 1:1, which is itself a script): two Fit stages. Both sides' flag forms are present, so choosing which side to flag needs no picker; per-side and linked step - links; filmstrip links that set B; flag forms that land back on the same pair. + links; filmstrip links that replace the URL's active side (B by default, + C2); flag forms that land back on the same pair. Nothing judgment-bearing hides behind a script. - **INV-5 — one record of the stage mode**, shared with the review: `stage-one` on ``. Storage never raises. diff --git a/tests/mutations/r3.toml b/tests/mutations/r3.toml index 9c290ae..3dd35b1 100644 --- a/tests/mutations/r3.toml +++ b/tests/mutations/r3.toml @@ -182,9 +182,9 @@ label = "C4 a caption takes its height from one stage only (Fit draws the two at file = "booth/templates/base.html" test = "tests/test_compare_browser.py::test_two_stages_side_by_side_wide_and_stacked_narrow" old = ''' - .cmp-side{display:grid;grid-row:1 / -1;grid-template-rows:subgrid;min-width:0;min-height:0}''' + .cmp-side{display:grid;grid-row:1 / -1;grid-template-rows:subgrid;''' new = ''' - .cmp-side{display:grid;grid-row:1 / -1;grid-template-rows:auto minmax(0,1fr) auto;min-width:0;min-height:0}''' + .cmp-side{display:grid;grid-row:1 / -1;grid-template-rows:auto minmax(0,1fr) auto;''' [[mutation]] label = "C4 stacked, a stage keeps the review's 60vh" @@ -418,6 +418,99 @@ old = ''' new = ''' ''' +# ---- the heid code-review fold (01M3AJXX8RH4QZPY5D1DQWQQGQ) + +[[mutation]] +label = "C4 the separator is a border on B (B's stage 1px narrower than A's)" +file = "booth/templates/base.html" +test = "tests/test_compare_browser.py::test_two_stages_side_by_side_wide_and_stacked_narrow" +old = ''' + background:var(--surface-sunken)}''' +new = ''' + background:var(--surface-sunken)} + .cmp-side+.cmp-side{border-left:1px solid var(--border-subtle)}''' + +[[mutation]] +label = "C4 the stack break drifts to 1000px" +file = "booth/templates/base.html" +test = "tests/test_compare_browser.py::test_two_stages_side_by_side_wide_and_stacked_narrow" +old = ''' + @media (max-width:900px){ + .viewer.review.compare{display:block}''' +new = ''' + @media (max-width:1000px){ + .viewer.review.compare{display:block}''' + +[[mutation]] +label = "C4 a side with nothing to scroll on an axis does not ignore it (0/0 resets the other)" +file = "booth/templates/compare.html" +test = "tests/test_compare_browser.py::test_an_axis_with_nothing_to_scroll_is_ignored" +old = ''' + if (fy > 0 && ty > 0) t = from.scrollTop / fy * ty;''' +new = ''' + if (ty > 0) t = from.scrollTop / fy * ty;''' + +[[mutation]] +label = "C4 two videos get a Fit | 1:1 toggle" +file = "booth/app.py" +test = "tests/test_compare.py::test_a_video_or_track_plays_in_its_own_stage_and_two_get_no_toggle" +old = ''' + "any_image": any(by_rel[r].kind == "image" for r in (a, b)),''' +new = ''' + "any_image": True,''' + +[[mutation]] +label = "C3 Space on a focused player steps the pair" +file = "booth/templates/compare.html" +test = "tests/test_compare_browser.py::test_the_keys_keep_the_reviews_guards_and_c_toggles_the_view" +old = '''e.target.closest('button, a, summary, video, audio')''' +new = '''e.target.closest('button, a, summary')''' + +[[mutation]] +label = "C3 there is no back arrow" +file = "booth/templates/compare.html" +test = "tests/test_compare_browser.py::test_unlinked_moves_only_the_active_side_and_the_strip_picks_it" +old = ''' + else if (k === 'ArrowLeft') step(-1);''' +new = ''' +''' + +[[mutation]] +label = "C1 containment is a bare prefix (a sibling booth sharing the name opens)" +file = "booth/app.py" +test = "tests/test_compare.py::test_an_outside_symlink_in_the_ring_is_404" +old = ''' + if not str(target).startswith(str(booth) + os.sep) or not target.is_file(): + raise HTTPException(status_code=404, detail="no such item")''' +new = ''' + if not str(target).startswith(str(booth)) or not target.is_file(): + raise HTTPException(status_code=404, detail="no such item")''' + +[[mutation]] +label = "C1 the strip is not in ring order" +file = "booth/app.py" +test = "tests/test_compare.py::test_compare_renders_the_pair" +old = ''' + "pick_a": url(r, b), "pick_b": url(a, r)} for r in ring]''' +new = ''' + "pick_a": url(r, b), "pick_b": url(a, r)} for r in ring[::-1]]''' + +[[mutation]] +label = "C6 each reveal lifts B's blur whichever side it sits on" +file = "booth/templates/compare.html" +test = "tests/test_compare_browser.py::test_blur_is_honest_on_both_sides" +old = ''' + var stage = sides[btn.getAttribute('data-side')].querySelector('.vstage');''' +new = ''' + var stage = sides.b.querySelector('.vstage');''' + +[[mutation]] +label = "C5 the strip does not show a flag made in place" +file = "booth/templates/compare.html" +test = "tests/test_compare_browser.py::test_a_flags_A_in_place_and_the_stages_survive" +old = '''' % key, body, re.S).group(0) + assert f'name="target" value="{rel}"' in form, (key, form) def _four(root: pathlib.Path) -> pathlib.Path: @@ -103,9 +108,16 @@ def test_an_outside_symlink_in_the_ring_is_404(tmp_path): outside.write_bytes(PNG) (b / "zz-link.png").symlink_to(outside) assert "zz-link.png" in review_chain(booth_items(b)), "the fixture must put it in the ring" + # a SIBLING whose name shares the booth's prefix is outside too (the + # containment check compares with the separator, never a bare prefix) + sib = tmp_path / "g-extra" + sib.mkdir() + (sib / "x.png").write_bytes(PNG) + (b / "zz-sib.png").symlink_to(sib / "x.png") c = _client(tmp_path) - assert c.get("/b/g/compare?a=zz-link.png&b=p.png").status_code == 404 - assert c.get("/b/g/compare?a=p.png&b=zz-link.png").status_code == 404 + for rel in ("zz-link.png", "zz-sib.png"): + assert c.get(f"/b/g/compare?a={rel}&b=p.png").status_code == 404, rel + assert c.get(f"/b/g/compare?a=p.png&b={rel}").status_code == 404, rel def test_a_look_records_both_seen(tmp_path): @@ -144,6 +156,18 @@ def test_no_data_region_repeats(tmp_path): assert _frames(body)["q.png"] == "AB", "a == b marks the one frame both ways" +def test_a_video_or_track_plays_in_its_own_stage_and_two_get_no_toggle(tmp_path): + """C4: video and audio play in their own stage; the Fit | 1:1 toggle is + bound only when a side is a picture, so two videos get none.""" + _booth(tmp_path, "g", {"a.webm": b"\x1aE\xdf\xa3", "b.mp3": b"ID3", "c.png": PNG}) + c = _client(tmp_path) + body = c.get("/b/g/compare?a=a.webm&b=b.mp3").text + assert re.search(r' [...document.querySelectorAll('.cmp-side .vstage')].map(s => { def test_two_stages_side_by_side_wide_and_stacked_narrow(browser, live): - """The tracer. At 1440 A and B share one row, A on the left, each about - half the body. At 390 they stack, A above B, each at most 45vh tall.""" + """The tracer. Above 900px A and B share one row, A on the left, each + exactly half the body — the SAME width, so equal pictures have equal + ranges. At 900px and below (the review's break) they stack, A above B, + each at most 45vh tall.""" base, root = live from booth.app import set_blurred b = _pics(root, {"a.png": (800, 600), "b.png": (800, 600)}) (b / "a.png.txt").write_text("a caption on A only, " * 8) set_blurred(b, "b.png", True) # the bar carries Reveal all too: its fullest got = {} - for w, h in ((1440, 900), (390, 844)): + for w, h in ((1440, 900), (901, 800), (900, 800), (390, 844)): page = browser.new_page(viewport={"width": w, "height": h}) _open(page, f"{base}/b/g/compare?a=a.png&b=b.png") got[w] = (page.evaluate(_BOXES), h, @@ -67,7 +69,13 @@ def test_two_stages_side_by_side_wide_and_stacked_narrow(browser, live): # the SAME stage for both, or Fit draws one smaller: A's caption must not # take its height from A's stage alone assert abs(a["h"] - b["h"]) <= 1 and abs(a["b"] - b["b"]) <= 1, (a, b) + # ...and the same width: a separator must not come out of one side alone + assert abs((a["r"] - a["l"]) - (b["r"] - b["l"])) <= 0.5, (a, b) assert over <= 0 and tallest[0] <= 40 and tallest[1] <= 1, (over, tallest) + (a, b), _, _, _ = got[901] + assert abs(a["t"] - b["t"]) <= 1 and a["r"] <= b["l"], ("side by side at 901", a, b) + (a, b), _, _, _ = got[900] + assert a["b"] <= b["t"], ("stacked at 900", a, b) (a, b), vh, over, tallest = got[390] assert a["b"] <= b["t"], ("A above B", a, b) assert a["h"] <= 0.45 * vh + 1 and b["h"] <= 0.45 * vh + 1, (a, b) @@ -101,7 +109,10 @@ def test_one_mode_for_both_and_for_the_review(browser, live): stored = page.evaluate("localStorage.getItem('booth.fit')") pressed = page.locator("#btn-one").get_attribute("aria-pressed") page.goto(f"{base}/b/g/view?f=a.png", wait_until="networkidle") - review_one = page.evaluate("document.documentElement.classList.contains('stage-one')") + page.wait_for_function("document.getElementById('vimg').complete && document.getElementById('vimg').naturalWidth > 0") + review_one = page.evaluate("""() => { const i = document.getElementById('vimg'), b = i.getBoundingClientRect(); + return document.documentElement.classList.contains('stage-one') && + Math.round(b.width) === i.naturalWidth && Math.round(b.height) === i.naturalHeight; }""") page.goto(f"{base}/b/g/compare?a=a.png&b=b.png", wait_until="networkidle") page.keyboard.press("Z") back = page.evaluate("[document.documentElement.classList.contains('stage-one'), localStorage.getItem('booth.fit')]") @@ -202,6 +213,28 @@ def test_synced_pan_by_fraction_for_different_sizes(browser, live): assert quarter[1] == [x, y] and idle == quarter, (x, y, quarter, idle) +def test_an_axis_with_nothing_to_scroll_is_ignored(browser, live): + """A side with nothing to scroll on an axis ignores that axis, each axis + on its own. A is wide and short (it scrolls across only), B is large. B + scrolled down stays down when A pans across: B's x follows, B's y is B's.""" + base, root = live + _pics(root, {"a.png": (3000, 200), "b.png": (3000, 3000)}) + page = browser.new_page(viewport={"width": 1440, "height": 900}) + _one_to_one(page, f"{base}/b/g/compare?a=a.png&b=b.png") + (rax, ray), (rbx, rby) = page.evaluate(_RANGES) + page.evaluate("document.querySelector('.cmp-side[data-side=\"b\"] .vstage').scrollTo(0, 500)") + page.wait_for_timeout(250) + down = page.evaluate(_SCROLLS) + page.evaluate("document.querySelector('.cmp-side[data-side=\"a\"] .vstage').scrollTo(800, 0)") + page.wait_for_timeout(250) + across = page.evaluate(_SCROLLS) + page.close() + assert ray == 0 and rax > 0 and rby > 0, (rax, ray, rbx, rby) + assert down == [[0, 0], [0, 500]], down + assert across[0] == [800, 0], across + assert abs(across[1][0] - 800 / rax * rbx) <= 1 and across[1][1] == 500, across + + def test_a_flags_A_in_place_and_the_stages_survive(browser, live): """`A` flags A in place: no navigation, A's control and label show the flag, B's do not, and both stages are the SAME nodes — a save swaps the @@ -221,6 +254,7 @@ def test_a_flags_A_in_place_and_the_stages_survive(browser, live): a: document.querySelector('[data-region="label-a"]').textContent, b: document.querySelector('[data-region="label-b"]').textContent, bflag: document.getElementById('cmp-flag-b').classList.contains('is-flagged'), + strip: [...document.querySelectorAll('.film-f.is-flagged')].map(f => f.dataset.rel), })""") flagged = [m.target for m in marks_for(b) if m.shape == "flag"] # the save REPLACED the buttons: each key must find the fresh one @@ -233,6 +267,7 @@ def test_a_flags_A_in_place_and_the_stages_survive(browser, live): page.close() assert not got["reload"] and got["same"], got assert "flagged" in got["a"] and "flagged" not in got["b"] and not got["bflag"], got + assert got["strip"] == ["a.png"], got assert flagged == ["a.png"] and after == ["b.png"] and not reloaded, (flagged, after, reloaded) @@ -267,9 +302,9 @@ def test_linked_arrow_walks_a_bakeoff(browser, live): _press_and_wait(page, "ArrowRight") pairs.append(_pair(page)) page.close() - assert [(a[2:], b[2:]) for a, b in pairs] == [(a[2:], a[2:]) for a, _ in pairs], pairs - assert [a[0] + b[0] for a, b in pairs] == ["mr"] * 4, pairs - assert pairs[-1] == ("m-4-tower-s42.png", "r-4-tower-s42.png"), pairs + # every pair, in order: no pair skipped and none repeated + scenes = ("dock-s11", "forge-s23", "marsh-s37", "tower-s42") + assert pairs == [(f"m-{i}-{sc}.png", f"r-{i}-{sc}.png") for i, sc in enumerate(scenes, 1)], pairs def test_unlinked_moves_only_the_active_side_and_the_strip_picks_it(browser, live): @@ -286,6 +321,9 @@ def test_unlinked_moves_only_the_active_side_and_the_strip_picks_it(browser, liv one = _pair(page) _press_and_wait(page, "ArrowRight") two = _pair(page) + _press_and_wait(page, "ArrowLeft") + left = _pair(page) + _press_and_wait(page, "ArrowRight") with page.expect_navigation(wait_until="networkidle"): page.locator('.film-f[data-rel="m-3-marsh-s37.png"]').click() picked = _pair(page) @@ -301,6 +339,7 @@ def test_unlinked_moves_only_the_active_side_and_the_strip_picks_it(browser, liv assert linked == "false", linked assert one == ("m-1-dock-s11.png", "r-2-forge-s23.png"), one assert two == ("m-1-dock-s11.png", "r-3-marsh-s37.png"), two + assert left == one, ("← moves only the active side back", left) assert picked == ("m-1-dock-s11.png", "m-3-marsh-s37.png"), picked assert active == ["a", ["m-1-dock-s11.png"]], active assert stepped == ("m-2-forge-s23.png", "m-3-marsh-s37.png"), stepped @@ -328,6 +367,10 @@ def test_blur_is_honest_on_both_sides(browser, live): page.wait_for_timeout(400) # the filter transition own = page.evaluate(_FILTERS) page.locator('.cmp-reveal[data-side="b"]').click() + page.locator('.cmp-reveal[data-side="a"]').click() + page.wait_for_timeout(400) + own_a = page.evaluate(_FILTERS) + page.locator('.cmp-reveal[data-side="a"]').click() page.locator("[data-reveal-all]").click() page.wait_for_timeout(400) everything = page.evaluate(_FILTERS) @@ -336,6 +379,7 @@ def test_blur_is_honest_on_both_sides(browser, live): assert all("blur(" in f for f in before), before assert shown == [True, True], shown assert "blur(" in own[0] and "blur(" not in own[1], own + assert "blur(" not in own_a[0] and "blur(" in own_a[1], own_a assert all("blur(" not in f for f in everything), everything assert stood_down == [False, False], stood_down @@ -385,7 +429,9 @@ def test_without_js_every_judgment_and_step_still_works(browser, live): ctx = browser.new_context(java_script_enabled=False, viewport={"width": 1440, "height": 900}) page = ctx.new_page() page.goto(f"{base}/b/g/compare?a=m-1-dock-s11.png&b=r-1-dock-s11.png", wait_until="networkidle") - fit = page.evaluate("document.documentElement.classList.contains('stage-one')") + fit = page.evaluate("""() => document.documentElement.classList.contains('stage-one') || + [...document.querySelectorAll('.cmp-side .vstage img')].some(i => + Math.round(i.getBoundingClientRect().width) === i.naturalWidth)""") hidden = [page.locator(s).is_visible() for s in ("#cmp-link", "#vtoggle")] forms = page.locator(".cmp-flag form").count() page.locator('a[data-step="both-next"]').click() @@ -417,6 +463,8 @@ def test_the_keys_keep_the_reviews_guards_and_c_toggles_the_view(browser, live): item; `Esc` (or `C`) in compare returns to the review of A.""" base, root = live _bakeoff(root) + v = _pics(root, {"a.png": (400, 300)}, name="v") + (v / "b.webm").write_bytes(b"\x1aE\xdf\xa3") page = browser.new_page(viewport={"width": 1440, "height": 900}) _open(page, f"{base}/b/g/compare?a=m-2-forge-s23.png&b=r-2-forge-s23.png") start = page.url @@ -432,6 +480,13 @@ def test_the_keys_keep_the_reviews_guards_and_c_toggles_the_view(browser, live): pressed = (page.url.split("?")[1], page.locator("#cmp-link").get_attribute("aria-pressed")) page.locator("#cmp-link").press(" ") # linked again page.evaluate("document.activeElement.blur()") + # ...and never from a player on either stage: Space is the player's + page.goto(f"{base}/b/v/compare?a=a.png&b=b.webm", wait_until="networkidle") + page.locator(".cmp-media").focus() + page.keyboard.press(" ") + page.wait_for_timeout(300) + player = page.url.endswith("/b/v/compare?a=a.png&b=b.webm") + page.goto(start, wait_until="networkidle") _press_and_wait(page, " ") fwd = _pair(page) _press_and_wait(page, "Shift+ ") @@ -444,6 +499,7 @@ def test_the_keys_keep_the_reviews_guards_and_c_toggles_the_view(browser, live): c_back = page.url page.close() assert inert == (True, "b", "true"), inert + assert player, "Space on a focused player stepped the pair" assert pressed == ("a=m-2-forge-s23.png&b=r-2-forge-s23.png&link=0", "false"), pressed assert fwd == ("m-3-marsh-s37.png", "r-3-marsh-s37.png") and back == ("m-2-forge-s23.png", "r-2-forge-s23.png"), (fwd, back) assert esc.endswith("/b/g/view?f=m-2-forge-s23.png"), esc