fix(web): the nav may not say where you are until the rows agree (#754)

keel/web/static/css/keel.css

@@ -451,6 +451,75 @@ header a {
 header a:hover { color: var(--fg); }
 header a[aria-current="page"] { color: var(--fg); border-bottom-color: var(--accent); }
 
+/* ── the navigation transition (#754) ─────────────────────────────────────────────────────────
+ *
+ * `show()` used to set `aria-current` and the tab title before awaiting the read, so for the
+ * length of the fetch the nav said "Balances" over Positions' rows -- a stale figure under a
+ * fresh label. The claim now waits for the DOM (`commitNavigation`), which leaves two states to
+ * draw here, and they must not look alike:
+ *
+ *   [data-pending]           your click was heard; the rows are still the old route's
+ *   [aria-current="page"]    you are here, and what is on screen is this route
+ *
+ * Pending is deliberately WEAKER than current -- dotted rather than solid, and it does not take
+ * the foreground colour. A pending link that looked identical to the current one would restore
+ * exactly the confusion the deferral removes. */
+header a[data-pending] {
+  border-bottom-style: dotted;
+  border-bottom-color: var(--muted);
+}
+
+/* The busy state, which `main.js` raises ONLY on navigation -- never on the 15-second poll or an
+ * SSE repaint, both of which would otherwise flash the page four times a minute on a route
+ * nobody navigated. `index.html` ships `aria-busy="true"` so the first paint is covered too.
+ *
+ * Dimmed rather than emptied. Clearing would be the stronger claim and it costs a layout jump on
+ * every navigation; dimming says "this is not current" while leaving the reader's place. The
+ * figures beneath are still the previous route's and are still legible, which is why the nav no
+ * longer says otherwise -- the two halves of this fix only work together. */
+#content[aria-busy="true"] {
+  opacity: 0.55;
+  /* The rows underneath are stale by definition here. Nothing beneath should be clickable while
+   * it is: acting on a figure that is about to be replaced is the failure mode of a dimmed view
+   * that stays live. */
+  pointer-events: none;
+}
+
+/* An indeterminate bar, because the read has no measurable progress to report -- the server
+ * sends one response, not a stream of them, and a fake percentage would be a claim about
+ * something nothing is counting.
+ *
+ * On `#view` rather than `#content` so it sits above the dimmed rows without inheriting the dim
+ * -- `opacity` creates a stacking context and would take the bar down with the content.
+ *
+ * `:has(> ...)` and NOT a descendant combinator, because the nesting runs the OTHER WAY:
+ * `main#view > div#content`. The first cut of this rule was `#content[aria-busy="true"]
+ * #view::before`, which asks for a `#view` INSIDE `#content` -- true of no document this shell
+ * ever produces, so it matched nothing and the bar never drew. CSS fails silently, and the test
+ * asserting the selector's text passed the whole time;
+ * `test_the_busy_bar_selector_matches_the_shell_it_targets` is what makes that failure loud.
+ *
+ * Reduced motion is handled by the blanket rule further down, which flattens the animation to a
+ * static bar rather than removing the signal. */
+#view:has(> #content[aria-busy="true"])::before {
+  content: "";
+  position: fixed;
+  top: 0;
+  left: 0;
+  right: 0;
+  height: 2px;
+  background: var(--accent);
+  transform-origin: 0 50%;
+  animation: keel-busy 1.1s ease-in-out infinite;
+  z-index: 1;
+}
+
+@keyframes keel-busy {
+  0%   { transform: scaleX(0); }
+  50%  { transform: scaleX(0.7); }
+  100% { transform: scaleX(1); }
+}
+
 /* ── layout ───────────────────────────────────────────────────────────────────────────────────
  *
  * `max-width: 62rem` is render.py's measure, kept: it is the line length the existing page was
@@ -997,10 +1066,15 @@ footer {
   .grid { grid-template-columns: repeat(auto-fit, minmax(0, 1fr)); }
 }
 
-/* Honour a reader who has asked the operating system for less motion. There is no animation in
- * this stylesheet today; the rule is here so that the first one added inherits the courtesy
- * rather than depending on its author remembering -- the same reasoning `server.needs_database`
- * gives for guarding a whole route set instead of one page. */
+/* Honour a reader who has asked the operating system for less motion. Written before there was
+ * any animation to honour, so that the first one added inherited the courtesy rather than
+ * depending on its author remembering -- the same reasoning `server.needs_database` gives for
+ * guarding a whole route set instead of one page. #754's `keel-busy` is that first animation,
+ * and it arrived already covered, which is what the rule was for.
+ *
+ * Note what this does to it: `animation-duration: 0.01ms` plus `iteration-count: 1` leaves the
+ * busy bar drawn at its final frame -- a static full-width rule -- rather than removing it. The
+ * signal survives; only the movement goes. */
 @media (prefers-reduced-motion: reduce) {
   *, *::before, *::after {
     animation-duration: 0.01ms !important;

keel/web/static/js/main.js

@@ -506,7 +506,22 @@ async function paint(route, rebuild, force) {
 
   if (!rebuild) return;
 
-  if (primary.data === null) {
+  // `try`/`finally` around the swap, and the flag comes down in the `finally` (#757 review).
+  // `read` resolves on a transport failure rather than rejecting, but `mount` is a renderer and a
+  // renderer can throw -- and `paint` is called as `void paint(...)`, so nothing catches it. Left
+  // raised, `aria-busy` is no longer the harmless assistive-only signal it was before #754: it now
+  // carries `opacity` and `pointer-events: none`, so a throw here would leave the view dimmed and
+  // every control inside it dead. The stale-but-usable view we had before is strictly better than
+  // that, and this is what keeps it.
+  //
+  // `commitNavigation` stays INSIDE the try, after the swap: a render that threw is not an
+  // arrival, and claiming one would put the nav label back out of step with the rows -- the exact
+  // bug #754 exists to remove.
+  //
+  // Nothing awaits between the `route !== current` guard above and here, so `current` cannot move
+  // under this block and the flag being lowered is always this route's own.
+  try {
+    if (primary.data === null) {
     // `data: null` is the ONLY route into these two views, and `payload.envelope` guarantees the
     // key is `null` rather than `{}` for exactly this reason -- see `render.stoppedView`.
     //
@@ -515,15 +530,23 @@ async function paint(route, rebuild, force) {
     // refused this browser. The two 403s (`_admitted`'s host check and its session check) share
     // this branch on purpose. They have the same remedy from the operator's side: the address
     // keel printed is what admits this browser, and pasting it is the action either one needs.
-    if (primary.error && primary.error.status === REFUSED_STATUS) {
-      rebuildInto(refusedView(primary, reconnect), Boolean(force));
+      if (primary.error && primary.error.status === REFUSED_STATUS) {
+        rebuildInto(refusedView(primary, reconnect), Boolean(force));
+      } else {
+        rebuildInto(stoppedView(primary, pathFor(SETUP_ROUTE)), Boolean(force));
+      }
     } else {
-      rebuildInto(stoppedView(primary, pathFor(SETUP_ROUTE)), Boolean(force));
+      rebuildInto(mount(route, readings), Boolean(force));
     }
-  } else {
-    rebuildInto(mount(route, readings), Boolean(force));
+    // AFTER the swap, never before: this is the point at which the nav label, the tab title and
+    // the rows on screen describe the same route (#754). Reached by all three branches above -- a
+    // stopped view and a refused view are outcomes of this navigation too, and leaving the nav
+    // pointing at the previous route while the content reports on this one is the same mismatch.
+    commitNavigation(route);
+  } finally {
+    clearPending();
+    contentNode.setAttribute("aria-busy", "false");
   }
-  contentNode.setAttribute("aria-busy", "false");
 }
 
 /**
@@ -606,18 +629,71 @@ const SETUP_ROUTE = ROUTES.find((route) => route.name === "setup") ?? DEFAULT_RO
  * @param {Route} route
  * @param {boolean} focus
  */
-function show(route, focus) {
-  current = route;
-  document.title = "keel — ".concat(route.label);
+/**
+ * The attribute marking the link a reader clicked, while its route is still loading.
+ *
+ * Deliberately NOT `aria-current`. That one means "this IS the current page" to a screen reader
+ * and to the stylesheet, and saying it before the view exists is the bug #754 is about. This says
+ * only "your click was heard", which is a different claim and needs a different attribute --
+ * without it, deferring `aria-current` would leave a slow route looking like a dead link and
+ * invite a second click.
+ *
+ * `data-` rather than a class so it cannot collide with the palette's own state classes, and so
+ * the stylesheet hook and the meaning stay in one name.
+ * @type {string}
+ */
+const PENDING_ATTR = "data-pending";
+
+/**
+ * Acknowledge a click on `route` without claiming arrival.
+ * @param {Route} route
+ */
+function markPending(route) {
+  for (const link of document.querySelectorAll("header nav a")) {
+    if (link.getAttribute("href") === pathFor(route)) link.setAttribute(PENDING_ATTR, "");
+    else link.removeAttribute(PENDING_ATTR);
+  }
+}
 
+/**
+ * Say where the reader now is -- called by `paint` AFTER the DOM backing it has been swapped in.
+ *
+ * Every claim about arrival lives here, in one function, so there is one place to get the
+ * ordering wrong instead of three. `aria-current="page"` remains both the assistive signal and
+ * the CSS hook (`header a[aria-current="page"]`), so the underline a sighted user sees and the
+ * word a reader hears still come from one attribute and cannot drift apart.
+ *
+ * Clearing `PENDING_ATTR` belongs here for the same reason: the acknowledgement ends exactly when
+ * the arrival begins, so a marker cannot outlive the read that set it.
+ * @param {Route} route
+ */
+function commitNavigation(route) {
+  document.title = "keel — ".concat(route.label);
   for (const link of document.querySelectorAll("header nav a")) {
     const isCurrent = link.getAttribute("href") === pathFor(route);
-    // `aria-current="page"` is both the assistive signal and the CSS hook (`header
-    // a[aria-current="page"]`), so the underline a sighted user sees and the word a reader hears
-    // come from one attribute and cannot drift apart.
     if (isCurrent) link.setAttribute("aria-current", "page");
     else link.removeAttribute("aria-current");
   }
+}
+
+/**
+ * Drop the click acknowledgement, however the read ended.
+ *
+ * Separate from `commitNavigation` because the two answer different questions. Arrival is a claim
+ * about where you ARE and must not be made when the render failed; the acknowledgement is about a
+ * read being over, which is true whether it succeeded, failed, or was superseded. Folding it into
+ * the commit left a link marked pending forever on any path that never arrived.
+ */
+function clearPending() {
+  for (const link of document.querySelectorAll("header nav a")) link.removeAttribute(PENDING_ATTR);
+}
+
+function show(route, focus) {
+  current = route;
+  // NOT `aria-current` and NOT `document.title` -- both are claims about where you ARE, and the
+  // DOM that would back them is still the previous route's until `paint` swaps it (#754). What
+  // this may say is that a read is happening, which is `data-pending` and `aria-busy`.
+  markPending(route);
 
   contentNode.setAttribute("aria-busy", "true");
   if (focus) viewNode.focus();