Hold focus in thread seats and cards with holdFocus

TODO.md

@@ -181,12 +181,6 @@ and its chrome coordinate.
 
 ### Shared definitions
 
-- **Hold focus in the thread seats with `holdFocus`.** `focus.js` `holdFocus` is the
-  one hand-over of the user's place across a DOM move, but `thread/inline.js`
-  `ThreadSeat.present` (across one render) and `retain` (across a refused batch) still
-  pair `readCaret` with `focusDestination` by hand. Cutting `retain` over also lets a
-  user who moved on while the batch waited keep their new place. Left while the
-  `thread-focus-scroll` branch had unmerged edits to `present`.
 - **Say which panes the workspace holds without naming `lf-pane`.** `layouts.css`
   holds an authored pane only as the workspace body or a cell of it (a pane in a
   section flows), and a generated pane at any depth. It tells the two apart by the

skills/leaf/assets/runtime/focus.js

@@ -103,7 +103,10 @@ export const deepFocus = (at = document.activeElement) => {
 // stand-ins, most particular first: the replacement keyed on the identity the held node
 // had, then the widget's own fallback. The user lands on the first that is drawn and
 // takes focus, and the caret goes with them, since a stand-in is the same place under a
-// new node. Restoring answers whether the user now stands on one of them.
+// new node. A stand-in that is somewhere else, such as the reply of the thread a seat's
+// box gave way to, is the caller's own landing: a function that puts the user there its
+// own way, takes no caret, and answers whether it did. Restoring answers whether the user
+// now stands on one of them.
 //
 // Nothing is owed while the user still stands on the held node and it is drawn, and
 // nothing once focus has been placed anywhere since the hold began: a user who moved on
@@ -113,7 +116,19 @@ export const deepFocus = (at = document.activeElement) => {
 // `null` where the user does not stand inside `scope`, which may be a shadow root. The
 // platform's retargeting answers that: a user inside a widget's shadow tree stands
 // inside the scope holding the widget.
+//
+// A hold is a reading and nothing more, so one its caller never restores, such as a
+// batch's that commits, costs nothing: the one listener below counts placements for
+// every hold, and a hold compares the count it began at.
 let restoring = false;
+let placements = 0;
+document.addEventListener(
+  "focusin",
+  () => {
+    if (!restoring) placements += 1;
+  },
+  true,
+);
 // Drawn counts `visibility: hidden` as hidden: a node under it keeps focus for a frame
 // and then the browser blurs it to the body, as it does a node under `display: none`.
 const drawn = (node) =>
@@ -123,32 +138,33 @@ export function holdFocus(scope) {
   if (!standing || standing === document.body || !scope.contains(standing)) return null;
   const held = deepFocus(standing);
   const caret = readCaret(held);
-  let placed = false;
-  // Once, so a hold its caller never restores leaves nothing behind past the next move.
-  const listen = () =>
-    document.addEventListener("focusin", place, { capture: true, once: true });
-  const place = () => {
-    if (restoring) listen();
-    else placed = true;
-  };
-  listen();
+  const began = placements;
   return (...standIns) => {
-    document.removeEventListener("focusin", place, { capture: true });
-    if (placed) return false;
-    for (const node of [held, ...standIns]) {
-      if (!drawn(node)) continue;
-      if (deepFocus() === node) return true;
-      restoring = true;
-      try {
-        focusDestination(node, caret);
-      } finally {
-        restoring = false;
+    if (placements !== began) return false;
+    for (const standIn of [held, ...standIns]) {
+      if (typeof standIn === "function") {
+        if (land(standIn)) return true;
+        continue;
       }
-      if (node.matches(":focus")) return true;
+      if (!drawn(standIn)) continue;
+      if (deepFocus() === standIn) return true;
+      const landed = land(() => {
+        focusDestination(standIn, caret);
+        return standIn.matches(":focus");
+      });
+      if (landed) return true;
     }
     return false;
   };
 }
+const land = (landing) => {
+  restoring = true;
+  try {
+    return landing();
+  } finally {
+    restoring = false;
+  }
+};
 
 const TYPED_TYPES = new Set([
   "text",

skills/leaf/assets/runtime/thread/inline.js

@@ -5,7 +5,7 @@ import { seatRoot } from "./model.js";
 import { ThreadView, threadReading } from "./thread-card.js";
 import { elementById } from "../passages.js";
 import { focused } from "../keyboard/scopes.js";
-import { focusDestination, readCaret } from "../focus.js";
+import { holdFocus } from "../focus.js";
 import { registry } from "../registry.js";
 import { loadDraft } from "../drafts.js";
 import { holdBox } from "./reply-landing.js";
@@ -22,7 +22,6 @@ class ThreadSeat {
   #committed = EMPTY;
   #commands = null;
   #response = () => null;
-  #focus = null;
   #connected = false;
   #marginControls = null;
 
@@ -36,11 +35,12 @@ class ThreadSeat {
   }
 
   present(model, batch = activeBatch) {
-    batch?.seats.add(this);
+    // A refused batch hands the user back the place they stood in when the seat joined
+    // it (`retainThreadSeats`); this render hands them their place across itself.
+    if (batch && !batch.seats.has(this)) batch.seats.set(this, holdFocus(this.node));
+    const restoreFocus = holdFocus(this.node);
     const standing = focused();
-    const held = this.node.contains(standing);
-    this.#focus ??= held ? { element: standing, caret: readCaret(standing) } : null;
-    const restoreBox = held ? holdBox(standing) : () => {};
+    const restoreBox = this.node.contains(standing) ? holdBox(standing) : () => {};
     const added = model.threads.filter(
       (thread) => !this.#model.threads.some(({ key }) => key === thread.key),
     );
@@ -58,7 +58,6 @@ class ThreadSeat {
       view.present(descriptor);
       return { key: descriptor.key, node: view.node };
     });
-    const caret = held ? readCaret(standing) : null;
     render(
       [
         repeat(
@@ -70,20 +69,11 @@ class ThreadSeat {
       ],
       this.node,
     );
-    if (
-      held &&
-      standing.isConnected &&
-      !this.node.contains(focused()) &&
-      this.node.contains(standing)
-    )
-      focusDestination(standing, caret);
     // The seat's own box gives way to the thread its message started: the user goes on
     // in that thread's reply, as they would have gone on in the box they sent from.
-    else if (held && !standing.isConnected && added.length === 1) {
-      const view = this.#views.get(added[0].key);
-      const box = view?.node.querySelector(SAY_BOX);
-      if (box && !this.node.contains(focused())) this.#commands?.landInThread(box);
-    }
+    const started =
+      added.length === 1 && this.#views.get(added[0].key).node.querySelector(SAY_BOX);
+    restoreFocus?.(started && (() => this.#commands?.landInThread(started)));
     restoreBox();
     if (!batch) this.commit();
   }
@@ -98,14 +88,11 @@ class ThreadSeat {
         this.#views.delete(key);
       }
     }
-    this.#focus = null;
     this.#connected ||= this.node.isConnected;
   }
-  retain() {
+  retain(restoreFocus) {
     this.present(this.#committed);
-    if (this.#focus?.element.isConnected)
-      focusDestination(this.#focus.element, this.#focus.caret);
-    this.#focus = null;
+    restoreFocus?.();
   }
   prune() {
     if (!this.#connected || this.node.isConnected) return false;
@@ -184,12 +171,12 @@ export function renderMarginThread(host, thread, commands, marginControls = null
 }
 
 export function beginThreadSeats() {
-  activeBatch = { seats: new Set() };
+  activeBatch = { seats: new Map() };
   return activeBatch;
 }
 export function commitThreadSeats(batch) {
   if (activeBatch !== batch) return;
-  for (const seat of batch.seats) seat.commit();
+  for (const seat of batch.seats.keys()) seat.commit();
   activeBatch = null;
   for (const seat of activeSeats)
     if (seat.prune()) {
@@ -199,6 +186,6 @@ export function commitThreadSeats(batch) {
 }
 export function retainThreadSeats(batch) {
   if (activeBatch !== batch) return;
-  for (const seat of batch.seats) seat.retain();
+  for (const [seat, restoreFocus] of batch.seats) seat.retain(restoreFocus);
   activeBatch = null;
 }

skills/leaf/assets/runtime/thread/thread-card.js

@@ -5,7 +5,7 @@
    those values.
    The owner alone renders its native card root and all generated descendants; a
    failed candidate is restored by presenting its committed descriptor again. */
-import { TEXT_FIELD } from "../focus.js";
+import { TEXT_FIELD, holdFocus } from "../focus.js";
 import { html, render, repeat, nothing } from "../../vendor/browser-runtime.js";
 import { turns, threadKey, threadSummary } from "./model.js";
 import { anchorLabel, MessageView, messageReading } from "./messages.js";
@@ -238,9 +238,8 @@ export class ThreadView {
 
   present(model) {
     const prior = this.#model;
+    const restoreFocus = holdFocus(this.node);
     const standing = focused();
-    const heldFocus = this.node.contains(standing);
-    let summaryReplacedFocusedMessage = false;
     const priorSummaries = new Set(prior?.summaries.map(({ id }) => id) ?? []);
     // Only a summary that was not standing before can swallow what the user
     // holds or is reading, and reading geometry here forces layout.
@@ -255,7 +254,6 @@ export class ThreadView {
       );
       for (const summary of model.summaries) {
         if (priorSummaries.has(summary.id)) continue;
-        if (summary.covers.includes(heldMessage)) summaryReplacedFocusedMessage = true;
         if (
           summary.covers.includes(heldMessage) ||
           summary.covers.some((id) => beingRead.has(id))
@@ -440,15 +438,13 @@ export class ThreadView {
       this.node,
     );
     this.#wireKeys();
-    if (
-      summaryReplacedFocusedMessage &&
-      focused() !== standing &&
-      standing?.isConnected
-    ) {
-      standing.focus({ preventScroll: true });
-    } else if (heldFocus && !this.node.contains(standing) && !panel) {
-      this.#commands.landInThread(this.node.querySelector(SAY_BOX) ?? this.node);
-    }
+    // A summary gathering the message the user stands on moves it; a page thread whose
+    // render took their place puts them in its reply, or on the thread itself.
+    restoreFocus?.(
+      !panel &&
+        (() =>
+          this.#commands.landInThread(this.node.querySelector(SAY_BOX) ?? this.node)),
+    );
     return this.node;
   }
 

tests/test_render_application_boundary.py

@@ -1741,6 +1741,100 @@ def test_a_failed_list_candidate_restores_its_complete_committed_reading(
     )
 
 
+def test_a_refused_thread_reading_leaves_a_user_who_moved_on_where_they_went(
+    browser, serve
+):
+    """A seat the user left while a reading waited does not pull them back to it.
+
+    Rolling back a refused reading puts back only what the reading itself took: a user
+    who stood in one seat's reply when it began and moved to another seat's box before
+    it failed keeps the box they moved to, caret included."""
+    layer = {**PAGE_DECLARATION, "lf-verdict": SEATED_ASK_ENTRY}
+    widgets = {"lf-local.js": PAGE_WIDGET, "lf-verdict.js": SEATED_ASK_MODULE}
+    url = serve(
+        leaf_page(
+            "Thread rollback",
+            '<h1>Review</h1><lf-verdict id="proposal" asks>Ship it?</lf-verdict>'
+            '<lf-verdict id="other" asks>Hold it?</lf-verdict>',
+        ),
+        layer_registry=layer,
+        layer_widgets=widgets,
+    )
+    kept = events_model.append_event(
+        serve.page_dir,
+        {
+            "kind": "comment",
+            "id": "kept-thread",
+            "author": "user",
+            "revision": 1,
+            "anchor": {"section": "proposal"},
+            "text": "The user starts here.",
+        },
+    )
+    page = open_page(browser, live_url(url))
+    page.locator(".lf-threads-toggle").click()
+    panel_settled(page)
+    left = page.locator(
+        f'#proposal > .lf-thread-seat > [data-thread="{kept["id"]}"]'
+        " > .lf-say leaf-text"
+    )
+    went = page.locator("#other > .lf-thread-seat leaf-text")
+    write(left, "where the user was")
+    expect(left).to_be_focused()
+
+    # The reading that brings the arriving thread waits at a gate, and both its paints
+    # (the attempt and the list's own retry) fail, so the whole reading is refused.
+    page.evaluate(
+        """() => {
+          const list = document.querySelector('leaf-thread-list');
+          const present = list.present.bind(list);
+          let open;
+          window.gate = new Promise(done => { open = done; });
+          window.openGate = open;
+          list.present = async model => {
+            if (!model.rows.some(row => row.descriptor?.id === 'arriving')) {
+              return present(model);
+            }
+            window.waiting = true;
+            await window.gate;
+            throw new Error('injected refusal');
+          };
+        }"""
+    )
+    events_model.append_event(
+        serve.page_dir,
+        {
+            "kind": "comment",
+            "id": "arriving",
+            "author": "user",
+            "revision": 1,
+            "text": "A thread whose reading is refused once.",
+        },
+    )
+    page.wait_for_function("() => window.waiting === true", timeout=5000)
+
+    write(went, "where the user went")
+    went.evaluate("node => node.setSelectionRange(6, 10, 'backward')")
+    page.evaluate("() => window.openGate()")
+    reported_browser_errors(
+        page,
+        (
+            "leaf: Presentation failed: Thread list presentation retry failed: "
+            "injected refusal; injected refusal"
+        ),
+        "leaf: State presentation failed: Thread list presentation retry failed",
+        "leaf: read failed: Thread list presentation retry failed",
+    )
+    expect(page.locator('[data-id="arriving"]')).to_have_count(0)
+
+    expect(went).to_be_focused()
+    expect(went).to_have_js_property("value", "where the user went")
+    assert went.evaluate(
+        "node => [node.selectionStart, node.selectionEnd, node.selectionDirection]"
+    ) == [6, 10, "backward"]
+    expect(left).to_have_js_property("value", "where the user was")
+
+
 def test_thread_readiness_waits_for_the_keyed_thread_list(browser, serve):
     """The existing thread ticket includes Lit ordering without replacing a card."""
     url = serve(LIVE_V1)

tests/test_render_threads.py

@@ -571,6 +571,39 @@ def test_a_summary_folds_originals_and_a_direct_reply_link_reveals_them(browser,
     expect(expand).to_have_attribute("aria-expanded", "true")
 
 
+def test_a_summary_gathering_the_message_the_user_is_on_keeps_them_on_it(
+    browser, serve
+):
+    """A checkpoint arriving over the message the user stands on opens around it.
+
+    The message moves into the checkpoint's originals, and the user moves with it
+    rather than dropping to the page."""
+    url = serve(PANEL_PAGE)
+    root = panel_comment(serve.page_dir, "Start with the measured constraint.")
+    first = append_agent_reply(serve.page_dir, root, "The constraint still applies.")
+    held = append_user_reply(serve.page_dir, root, "It holds for the camera too.")
+    append_agent_reply(serve.page_dir, root, "The later result remains visible.")
+
+    page = open_page(browser, url)
+    page.locator(".lf-threads-toggle").click()
+    panel_settled(page)
+    card = page.locator(f'.lf-thread[data-id="{root}"]')
+    card.locator(":scope > .lf-thread-summary").click()
+    message = card.locator(f'.lf-msg[data-mid="{held["id"]}"]')
+    message.focus()
+    expect(message).to_be_focused()
+
+    summary = summarize_thread(
+        serve.page_dir, root, first["id"], held["id"], "The constraint was confirmed."
+    )
+    told(page)
+    checkpoint = card.locator(f'[data-summary-id="{summary["id"]}"]')
+    expect(
+        checkpoint.locator(f'.lf-summary-originals > .lf-msg[data-mid="{held["id"]}"]')
+    ).to_be_visible()
+    expect(message).to_be_focused()
+
+
 def test_a_root_summary_keeps_thread_actions_outside_its_fold(browser, serve):
     """A checkpoint may cover the root turn without hiding thread actions."""
     url = serve(PANEL_PAGE)