From 4591d4a26051764fe4f2744532d4ce81bbf17071 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Thu, 2 Jul 2026 22:50:46 -0400 Subject: [PATCH] [onboarding] Fix popover pushed off-screen by full-height targets (sidebar step) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- js/ui/onboarding/bitvidTour.js | 1 + js/ui/onboarding/tourEngine.js | 85 ++++++++++++++++++++++++++-------- tests/onboarding-tour.test.mjs | 65 ++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 19 deletions(-) diff --git a/js/ui/onboarding/bitvidTour.js b/js/ui/onboarding/bitvidTour.js index 321e9b6b..395a8a1a 100644 --- a/js/ui/onboarding/bitvidTour.js +++ b/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.", diff --git a/js/ui/onboarding/tourEngine.js b/js/ui/onboarding/tourEngine.js index 03476d49..48373890 100644 --- a/js/ui/onboarding/tourEngine.js +++ b/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() { diff --git a/tests/onboarding-tour.test.mjs b/tests/onboarding-tour.test.mjs index 995e3aba..86c63c19 100644 --- a/tests/onboarding-tour.test.mjs +++ b/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"); +});