[onboarding] Fix popover pushed off-screen by full-height targets (sidebar step)

Step 2 (sidebar) left the user "stuck looking at a glow with nothing to do": the
sidebar is a full-height column, so the below/above-only placement computed a
popover position past the bottom of the viewport — the card existed but was
invisible off-screen.

Placement is now a pure, exported decision (computeTourPlacement): try below →
above → beside-right → beside-left, whichever actually fits the popover, with a
centered-over-the-dim fallback when nothing does (viewport-filling targets) — and
coordinates are clamped so the card can NEVER leave the viewport. The sidebar
step also hints placement: "right". A preferred placement that doesn't fit falls
back to one that does.

Tests: +1 in tests/onboarding-tour.test.mjs — full-height column → beside-right
inside the viewport; button → bottom; near-bottom target → top; viewport-filling
target → centered; unfittable preference falls back and never goes below the
fold. Lint + build green.

Co-Authored-By: Claude Opus 4.8 <[email protected]>

js/ui/onboarding/bitvidTour.js

@@ -28,6 +28,7 @@ export function buildBitvidTourSteps({ openProfilePane } = {}) {
     {
       id: "feeds",
       target: "#sidebar",
+      placement: "right", // full-height column — popover beside it, not below
       title: "Your feeds",
       body:
         "Recent shows the newest videos across the network. For You learns from what you watch, and Trending ranks by views. Switch any time from this sidebar.",

js/ui/onboarding/tourEngine.js

@@ -72,6 +72,62 @@ const TOUR_CSS = `
 .bv-tour-buttons .bv-tour-skip { margin-right: auto; }
 `;
 
+// Pure popover-placement decision (exported for tests). Given the target rect
+// and viewport, pick below → above → beside → centered, whichever actually fits,
+// and NEVER return a position outside the viewport. The original below/above-only
+// logic pushed the popover off-screen for full-height targets like the sidebar
+// (the "stuck with a glow and nothing to do" bug).
+export function computeTourPlacement({
+  rect,
+  viewportWidth,
+  viewportHeight,
+  preferred = "",
+  popWidth = 336,
+  popHeight = 240,
+  pad = 12,
+} = {}) {
+  const clampX = (x) =>
+    Math.min(Math.max(x, popWidth / 2 + pad), viewportWidth - popWidth / 2 - pad);
+  const centerX = clampX(rect.left + rect.width / 2);
+
+  const fits = {
+    bottom: viewportHeight - rect.bottom >= popHeight + pad,
+    top: rect.top >= popHeight + pad,
+    right: viewportWidth - rect.right >= popWidth + pad * 2,
+    left: rect.left >= popWidth + pad * 2,
+  };
+
+  const order = preferred && fits[preferred]
+    ? [preferred]
+    : ["bottom", "top", "right", "left"].filter((side) => fits[side]);
+  const side = order[0] || "center";
+
+  const midY = Math.min(
+    Math.max(rect.top + rect.height / 2, popHeight / 2 + pad),
+    viewportHeight - popHeight / 2 - pad,
+  );
+
+  switch (side) {
+    case "bottom":
+      return { placement: "bottom", px: centerX, py: rect.bottom + pad, tx: "-50%", ty: "0%" };
+    case "top":
+      return { placement: "top", px: centerX, py: rect.top - pad, tx: "-50%", ty: "-100%" };
+    case "right":
+      return { placement: "right", px: rect.right + pad, py: midY, tx: "0%", ty: "-50%" };
+    case "left":
+      return { placement: "left", px: rect.left - pad, py: midY, tx: "-100%", ty: "-50%" };
+    default:
+      // Nothing fits (huge target): center over the dim — always visible.
+      return {
+        placement: "center",
+        px: viewportWidth / 2,
+        py: viewportHeight / 2,
+        tx: "-50%",
+        ty: "-50%",
+      };
+  }
+}
+
 function ensureStyles(doc) {
   if (doc.getElementById(STYLE_ID)) {
     return;
@@ -144,25 +200,16 @@ export function createTour({
     setVar(state.scrim, "--bv-tour-w", `${rect.width + pad * 2}px`);
     setVar(state.scrim, "--bv-tour-h", `${rect.height + pad * 2}px`);
 
-    // Popover: below the target unless there is more room above.
-    const viewportH = doc.defaultView?.innerHeight || 800;
-    const below = rect.bottom + pad * 2;
-    const placeAbove =
-      step.placement === "top" ||
-      (step.placement !== "bottom" && below > viewportH - 220 && rect.top > 240);
-    const centerX = Math.min(
-      Math.max(rect.left + rect.width / 2, 180),
-      (doc.defaultView?.innerWidth || 1200) - 180,
-    );
-    setVar(state.popover, "--bv-tour-px", `${centerX}px`);
-    setVar(state.popover, "--bv-tour-tx", "-50%");
-    if (placeAbove) {
-      setVar(state.popover, "--bv-tour-py", `${rect.top - pad * 2}px`);
-      setVar(state.popover, "--bv-tour-ty", "-100%");
-    } else {
-      setVar(state.popover, "--bv-tour-py", `${below + pad}px`);
-      setVar(state.popover, "--bv-tour-ty", "0%");
-    }
+    const placementResult = computeTourPlacement({
+      rect,
+      viewportWidth: doc.defaultView?.innerWidth || 1200,
+      viewportHeight: doc.defaultView?.innerHeight || 800,
+      preferred: step.placement || "",
+    });
+    setVar(state.popover, "--bv-tour-px", `${placementResult.px}px`);
+    setVar(state.popover, "--bv-tour-py", `${placementResult.py}px`);
+    setVar(state.popover, "--bv-tour-tx", placementResult.tx);
+    setVar(state.popover, "--bv-tour-ty", placementResult.ty);
   }
 
   function renderStep() {

tests/onboarding-tour.test.mjs

@@ -165,3 +165,68 @@ test("final-card action buttons deep-link and complete the tour", () => {
   assert.equal(doc.querySelector(".bv-tour-root"), null, "tour closed");
   assert.equal(shouldOfferOnboarding(PUB), false, "completion recorded");
 });
+
+// Regression (the "stuck at the sidebar" bug): the popover placement must NEVER
+// land outside the viewport. A full-height target (sidebar) used to push the
+// popover below the fold, leaving the user with a glowing sidebar and no card.
+//
+// test_integrity_note:
+//   change_type: ["new_tests"]
+//   scenarios:
+//     - id: SCN-tour-placement
+//       given: "target rects of various shapes vs a viewport"
+//       when: "computeTourPlacement decides where the popover goes"
+//       then: "it picks a side that fits and always stays inside the viewport"
+//   observable_outcomes:
+//     - "full-height left column -> 'right' placement, y within viewport"
+//     - "normal button with room below -> 'bottom'"
+//     - "target near the bottom edge with room above -> 'top'"
+//     - "viewport-filling target -> 'center' fallback (always visible)"
+//   determinism_controls:
+//     - "pure function, explicit rect/viewport inputs"
+//   anti_cheat_rationale:
+//     prevents: ["asserting the off-screen (broken) coordinates"]
+//   relaxation:
+//     did_relax_any_assertion: false
+
+test("placement: full-height sidebar goes BESIDE, never off-screen", async () => {
+  const { computeTourPlacement } = await import("../js/ui/onboarding/tourEngine.js");
+  const viewport = { viewportWidth: 1280, viewportHeight: 800 };
+
+  const sidebar = computeTourPlacement({
+    rect: { left: 0, top: 64, right: 240, bottom: 800, width: 240, height: 736 },
+    ...viewport,
+  });
+  assert.equal(sidebar.placement, "right");
+  assert.ok(sidebar.py > 0 && sidebar.py < 800, "y inside the viewport");
+  assert.ok(sidebar.px > 240 && sidebar.px < 1280, "x beside the sidebar");
+
+  const button = computeTourPlacement({
+    rect: { left: 600, top: 100, right: 700, bottom: 140, width: 100, height: 40 },
+    ...viewport,
+  });
+  assert.equal(button.placement, "bottom");
+
+  const nearBottom = computeTourPlacement({
+    rect: { left: 600, top: 700, right: 700, bottom: 780, width: 100, height: 80 },
+    ...viewport,
+  });
+  assert.equal(nearBottom.placement, "top");
+
+  const fullScreen = computeTourPlacement({
+    rect: { left: 0, top: 0, right: 1280, bottom: 800, width: 1280, height: 800 },
+    ...viewport,
+  });
+  assert.equal(fullScreen.placement, "center");
+  assert.equal(fullScreen.px, 640);
+  assert.equal(fullScreen.py, 400);
+
+  // A preferred placement that doesn't fit falls back to one that does.
+  const badPreference = computeTourPlacement({
+    rect: { left: 0, top: 64, right: 240, bottom: 800, width: 240, height: 736 },
+    preferred: "bottom",
+    ...viewport,
+  });
+  assert.notEqual(badPreference.placement, "bottom");
+  assert.ok(badPreference.py <= 800 - 12, "never below the fold");
+});