mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(onboarding): keep the tour from bouncing between the Coach and Insights
With no provider set up, a fresh self-host's default, the Coach module is on but /coach redirects to /insights because the Coach is not available. The tour read the raw module map, kept its Coach stop, pushed /coach, was sent back to /insights and pushed /coach again on the pathname change: a loop. The tour now reads the same capability-folded map as the navigation, so the Coach stop is not in the list. And a stop whose page redirects is dropped after one navigation, in the direction the person was going, instead of being pushed again.
This commit is contained in:
1 parent
676bb2eff7
commit
28d491174e
5 files changed
+231
-5
No files matched your search
@@ -13,6 +13,8 @@
|
||||
* regress the gate. v1.18.6.1 — the tour is first-time-auto-start
|
||||
* only; the former "manual restart from Settings" bypass was removed.
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
@@ -147,3 +149,23 @@ describe("tourIncludesAchievements() — v1.18.0 B5 module gate", () => {
|
||||
expect(tourIncludesAchievements({ sleep: false, mood: false })).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the launcher hands the tour the capability-folded module map", () => {
|
||||
// The raw `user.modules` says the Coach module is on for a fresh self-host
|
||||
// with no provider, while `/coach` redirects to `/insights` because the
|
||||
// Coach capability is unavailable: the tour pushed `/coach`, was sent back,
|
||||
// and pushed again. The launcher must read `useNavModules()`, which folds
|
||||
// the capability into `coach`, exactly as the nav does.
|
||||
const source = readFileSync(
|
||||
join(process.cwd(), "src/components/onboarding/tour-launcher.tsx"),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
it("reads useNavModules()", () => {
|
||||
expect(source).toMatch(/\buseNavModules\(\)/);
|
||||
});
|
||||
|
||||
it("does not pass the raw account module map to the tour", () => {
|
||||
expect(source).not.toMatch(/modules\s*=\s*user\?\.modules/);
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,7 @@ import { useCallback, useEffect, useState } from "react";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
import { useNavModules } from "@/hooks/use-nav-modules";
|
||||
import { apiPost } from "@/lib/api/api-fetch";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { OnboardingTour } from "./tour";
|
||||
@@ -217,6 +218,7 @@ interface TourLauncherProps {
|
||||
|
||||
export function TourLauncher({ ready = true }: TourLauncherProps) {
|
||||
const { user, isAuthenticated, isLoading } = useAuth();
|
||||
const navModules = useNavModules();
|
||||
const queryClient = useQueryClient();
|
||||
const pathname = usePathname();
|
||||
|
||||
@@ -350,9 +352,12 @@ export function TourLauncher({ ready = true }: TourLauncherProps) {
|
||||
|
||||
if (showTour !== true) return null;
|
||||
|
||||
// The resolved module map gates which stops appear (default-on) and
|
||||
// keeps the "Schritt n/total" counter honest.
|
||||
const modules = user?.modules;
|
||||
// The module map the nav reads gates which stops appear (default-on) and
|
||||
// keeps the "Schritt n/total" counter honest. It folds the Coach
|
||||
// capability in: with no provider or no consent the Coach module is on
|
||||
// but `/coach` redirects, so the raw `user.modules` would keep a stop the
|
||||
// tour can never stand on.
|
||||
const modules = navModules;
|
||||
// v1.18.6 — resume the full tour from the persisted checkpoint.
|
||||
const resumeFromStopId = user?.onboardingTourProgress?.lastStopId ?? null;
|
||||
|
||||
|
||||
@@ -51,6 +51,8 @@ import {
|
||||
prevStep,
|
||||
skipTour,
|
||||
stepCounter,
|
||||
dropRedirectedStop,
|
||||
stopRouteRedirected,
|
||||
type TourModuleMap,
|
||||
type TourState,
|
||||
type TourStop,
|
||||
@@ -257,6 +259,21 @@ export function OnboardingTour({
|
||||
initTourState(stops, filterToStop ? null : resumeFromStopId),
|
||||
);
|
||||
const [rect, setRect] = useState<SpotlightRect | null>(null);
|
||||
// Which stop the tour last navigated for, from where, and whether that
|
||||
// stop's page was reached; and which way the person was travelling, so a
|
||||
// stop whose page redirects is stepped past in the same direction.
|
||||
const navRef = useRef<{
|
||||
stopId: string | null;
|
||||
pushedFrom: string | null;
|
||||
arrived: boolean;
|
||||
}>({ stopId: null, pushedFrom: null, arrived: false });
|
||||
const directionRef = useRef<"forward" | "back">("forward");
|
||||
// The navigation effect reads the latest `onClose` without re-running on
|
||||
// its identity (the launcher passes a fresh closure every render).
|
||||
const onCloseRef = useRef(onClose);
|
||||
useEffect(() => {
|
||||
onCloseRef.current = onClose;
|
||||
}, [onClose]);
|
||||
|
||||
const stop = currentStop(state);
|
||||
|
||||
@@ -285,8 +302,42 @@ export function OnboardingTour({
|
||||
// stops (the two dashboard stops while on `/`, or the wrap-up which has
|
||||
// no route) skip the push. `filterToStop` re-entry never navigates — it
|
||||
// runs on the page the user is already viewing.
|
||||
if (!filterToStop && stop.route && stop.route !== pathname) {
|
||||
router.push(stop.route);
|
||||
//
|
||||
// One push per stop. A page that sends the person elsewhere (the Coach
|
||||
// page redirects to `/insights` while the Coach is unavailable) would
|
||||
// otherwise be pushed again on every pathname change, a loop between the
|
||||
// two pages; such a stop is dropped and the tour moves on.
|
||||
if (!filterToStop && stop.route) {
|
||||
if (navRef.current.stopId !== stop.id) {
|
||||
navRef.current = { stopId: stop.id, pushedFrom: null, arrived: false };
|
||||
}
|
||||
const nav = navRef.current;
|
||||
if (pathname === stop.route) {
|
||||
nav.arrived = true;
|
||||
} else if (
|
||||
stopRouteRedirected({
|
||||
stopRoute: stop.route,
|
||||
pathname,
|
||||
pushedFrom: nav.pushedFrom,
|
||||
arrived: nav.arrived,
|
||||
})
|
||||
) {
|
||||
const direction = directionRef.current;
|
||||
queueMicrotask(() =>
|
||||
setState((prev) => {
|
||||
if (currentStop(prev)?.id !== stop.id) return prev;
|
||||
const next = dropRedirectedStop(prev, direction);
|
||||
if (isTourFinished(next)) {
|
||||
queueMicrotask(() => onCloseRef.current("completed"));
|
||||
}
|
||||
return next;
|
||||
}),
|
||||
);
|
||||
return;
|
||||
} else if (nav.pushedFrom === null) {
|
||||
nav.pushedFrom = pathname;
|
||||
router.push(stop.route);
|
||||
}
|
||||
}
|
||||
|
||||
const measureNow = () => {
|
||||
@@ -397,6 +448,7 @@ export function OnboardingTour({
|
||||
}, [onClose]);
|
||||
|
||||
const handleNext = useCallback(() => {
|
||||
directionRef.current = "forward";
|
||||
setState((prev) => {
|
||||
const next = nextStep(prev);
|
||||
if (isTourFinished(next) && next.outcome === "completed") {
|
||||
@@ -407,6 +459,7 @@ export function OnboardingTour({
|
||||
}, [onClose]);
|
||||
|
||||
const handlePrev = useCallback(() => {
|
||||
directionRef.current = "back";
|
||||
setState((prev) => prevStep(prev));
|
||||
}, []);
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@ import {
|
||||
prevStep,
|
||||
skipTour,
|
||||
stepCounter,
|
||||
dropRedirectedStop,
|
||||
stopRouteRedirected,
|
||||
} from "../tour-state";
|
||||
|
||||
const FULL_ORDER = [
|
||||
@@ -204,3 +206,104 @@ describe("tour-state", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("a stop whose page sends the person elsewhere", () => {
|
||||
const coach = "/coach";
|
||||
|
||||
it("is not redirected before the tour navigated for it", () => {
|
||||
expect(
|
||||
stopRouteRedirected({
|
||||
stopRoute: coach,
|
||||
pathname: "/insights",
|
||||
pushedFrom: null,
|
||||
arrived: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("is not redirected while the navigation is still on its way", () => {
|
||||
expect(
|
||||
stopRouteRedirected({
|
||||
stopRoute: coach,
|
||||
pathname: "/insights",
|
||||
pushedFrom: "/insights",
|
||||
arrived: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("is not redirected once it stands on its page", () => {
|
||||
expect(
|
||||
stopRouteRedirected({
|
||||
stopRoute: coach,
|
||||
pathname: coach,
|
||||
pushedFrom: "/insights",
|
||||
arrived: true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("is redirected when its page sent the person back to where they came from", () => {
|
||||
// The Coach page answers an unavailable Coach with a redirect to
|
||||
// /insights, the previous stop's page: /insights → /coach → /insights.
|
||||
expect(
|
||||
stopRouteRedirected({
|
||||
stopRoute: coach,
|
||||
pathname: "/insights",
|
||||
pushedFrom: "/insights",
|
||||
arrived: true,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("is redirected when the navigation landed on a third page", () => {
|
||||
expect(
|
||||
stopRouteRedirected({
|
||||
stopRoute: coach,
|
||||
pathname: "/",
|
||||
pushedFrom: "/insights",
|
||||
arrived: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("is dropped going forward: the next stop takes its place and Back never lands on it", () => {
|
||||
const steps = buildTourStops();
|
||||
const at = steps.findIndex((s) => s.id === "coach");
|
||||
const state = dropRedirectedStop(
|
||||
{ index: at, steps, outcome: null },
|
||||
"forward",
|
||||
);
|
||||
expect(currentStop(state)?.id).toBe("integrations");
|
||||
expect(state.steps.map((s) => s.id)).not.toContain("coach");
|
||||
expect(currentStop(prevStep(state))?.id).toBe("insights");
|
||||
});
|
||||
|
||||
it("is dropped going back: the previous stop takes over", () => {
|
||||
const steps = buildTourStops();
|
||||
const at = steps.findIndex((s) => s.id === "coach");
|
||||
const state = dropRedirectedStop(
|
||||
{ index: at, steps, outcome: null },
|
||||
"back",
|
||||
);
|
||||
expect(currentStop(state)?.id).toBe("insights");
|
||||
});
|
||||
|
||||
it("completes the tour when the dropped stop was the last one", () => {
|
||||
const steps = buildTourStops().slice(0, 3);
|
||||
const state = dropRedirectedStop(
|
||||
{ index: 2, steps, outcome: null },
|
||||
"forward",
|
||||
);
|
||||
expect(state.outcome).toBe("completed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("the tour's Coach stop follows the Coach's capability", () => {
|
||||
it("drops the Coach stop from a capability-folded module map", () => {
|
||||
// `useNavModules()` folds an unavailable Coach capability (no provider,
|
||||
// no consent) into `coach: false`; the tour reads that map.
|
||||
const ids = buildTourStops({ modules: { coach: false } }).map((s) => s.id);
|
||||
expect(ids).not.toContain("coach");
|
||||
});
|
||||
});
|
||||
@@ -289,6 +289,49 @@ export function prevStep(state: TourState): TourState {
|
||||
return { ...state, index: state.index - 1 };
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the page a stop navigated to sent the person somewhere else.
|
||||
*
|
||||
* A stop's page can refuse the visit on its own (the Coach page answers an
|
||||
* unavailable Coach with a redirect to `/insights`), and the overlay would
|
||||
* otherwise push the stop's route again on every pathname change: a loop
|
||||
* between the two pages. `pushedFrom` is the pathname the tour navigated
|
||||
* from for this stop (`null` while it has not navigated), `arrived` whether
|
||||
* the stop's own page was reached since. Before arrival, a pathname still
|
||||
* equal to `pushedFrom` is a navigation on its way, not a redirect.
|
||||
*/
|
||||
export function stopRouteRedirected(args: {
|
||||
stopRoute: string;
|
||||
pathname: string;
|
||||
pushedFrom: string | null;
|
||||
arrived: boolean;
|
||||
}): boolean {
|
||||
if (args.pushedFrom === null) return false;
|
||||
if (args.pathname === args.stopRoute) return false;
|
||||
return args.arrived || args.pathname !== args.pushedFrom;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop the current stop after its page redirected, and carry on in the
|
||||
* direction the person was travelling. The stop leaves the list, so Back
|
||||
* never lands on it again and the counter stays honest; dropping the last
|
||||
* stop going forward completes the tour.
|
||||
*/
|
||||
export function dropRedirectedStop(
|
||||
state: TourState,
|
||||
direction: "forward" | "back",
|
||||
): TourState {
|
||||
if (state.outcome !== null) return state;
|
||||
const steps = state.steps.filter((_, i) => i !== state.index);
|
||||
if (direction === "back") {
|
||||
return { ...state, steps, index: Math.max(0, state.index - 1) };
|
||||
}
|
||||
if (state.index >= steps.length) {
|
||||
return { ...state, steps, index: steps.length, outcome: "completed" };
|
||||
}
|
||||
return { ...state, steps };
|
||||
}
|
||||
|
||||
/** User explicitly dismissed the tour from any step. */
|
||||
export function skipTour(state: TourState): TourState {
|
||||
if (state.outcome !== null) return state;
|
||||
|
||||
Reference in new issue
Block a user