Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/frontend/src/components/portal/portal-layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { useShellLayout, type ShellLayout } from "@/lib/portal/use-shell-layout"
import { useViewerIsManager } from "@/lib/portal/use-viewer-is-manager";

/**
* Portal shell (Phase 1 buildout), behind the `insight.portal` flag so the
* default app is untouched. Composition (one SidebarProvider, all normal flow):
* Portal shell (Phase 1 buildout), rendered unless the reader opts out via
* `insight.portal`. Composition (one SidebarProvider, all normal flow):
* [ lens rail ] [ zone-contextual pane ] [ content ]
* Every zone renders through `<ZoneContent/>` (Person / People / Directions /
* Overview / … all portal-native); the route only carries the active person.
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/components/sidebar-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function SidebarSettings() {
>
<span className="flex items-center gap-2">
<PanelsTopLeft className="size-4" />
<span>Portal (preview)</span>
<span>Portal</span>
</span>
<Switch
checked={portal}
Expand Down
13 changes: 12 additions & 1 deletion src/frontend/src/lib/portal/portal-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ describe("portal preferences", () => {
expect(window.localStorage.getItem("insight.portal")).toBe("true");
});

it("enabled defaults ON when the key is absent", async () => {
const { readPortalEnabled } = await import("./portal-store");
expect(readPortalEnabled()).toBe(true);
});

it("an explicit opt-out is what the router reads back", async () => {
window.localStorage.setItem("insight.portal", "false");
const { readPortalEnabled } = await import("./portal-store");
expect(readPortalEnabled()).toBe(false);
});

it("show-planned defaults ON when the key is absent", () => {
window.localStorage.removeItem("insight.portal.showPlanned");
const { result } = renderHook(() => usePortalShowPlanned());
Expand Down Expand Up @@ -72,7 +83,7 @@ describe("blocked storage", () => {
});
try {
const { readPortalEnabled } = await import("./portal-store");
expect(readPortalEnabled()).toBe(false);
expect(readPortalEnabled()).toBe(true);
} finally {
getItem.mockRestore();
}
Expand Down
31 changes: 13 additions & 18 deletions src/frontend/src/lib/portal/portal-store.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useSyncExternalStore } from "react";

/**
* Portal PREFERENCES (feature-flagged behind `insight.portal`).
* Portal PREFERENCES (`insight.portal` — ON unless the reader opts out).
*
* `enabled` and `showPlanned` persist to localStorage (mirroring the metrics-v2
* flag pattern in feature-flags.ts). Nothing else lives here: every piece of
Expand Down Expand Up @@ -29,17 +29,16 @@ interface PortalState {
enabled: boolean;
/**
* Whether navigation shows entries we have not built yet (`unbuilt` in the
* nav model). Default ON while the whole portal is itself a preview: for us
* and for demos the dead entries ARE the roadmap. Turn it off — or flip the
* default — the day the portal stops being opt-in, so a customer never has
* to tell our backlog apart from their own missing data.
* nav model). Default ON: for us and for demos the dead entries ARE the
* roadmap. Flip it before a reader has to tell our backlog apart from their
* own missing data.
*/
showPlanned: boolean;
}

/** Router-safe read: `beforeLoad` runs outside React, so it cannot use a hook. */
export function readPortalEnabled(): boolean {
return readEnabled();
return readBoolPref(ENABLED_KEY);
}

/**
Expand All @@ -57,18 +56,14 @@ function readKey(key: string): string | null {
}
}

function readEnabled(): boolean {
return readKey(ENABLED_KEY) === "true";
}

/** Absent key = default ON (see `showPlanned`); only an explicit "false" hides. */
function readShowPlanned(): boolean {
return readKey(SHOW_PLANNED_KEY) !== "false";
/** Absent key = ON; both preferences are opt-OUT, so only "false" turns one off. */
function readBoolPref(key: string): boolean {
return readKey(key) !== "false";
}

let state: PortalState = {
enabled: readEnabled(),
showPlanned: readShowPlanned(),
enabled: readBoolPref(ENABLED_KEY),
showPlanned: readBoolPref(SHOW_PLANNED_KEY),
};

const listeners = new Set<() => void>();
Expand All @@ -93,7 +88,7 @@ function persist(key: string, value: string): void {
}
}

/** Turn the portal preview on or off for this reader. */
/** Turn the portal on or off for this reader. */
export function setPortalEnabled(enabled: boolean): void {
state = { ...state, enabled };
persist(ENABLED_KEY, enabled ? "true" : "false");
Expand All @@ -107,12 +102,12 @@ export function setPortalShowPlanned(show: boolean): void {
emit();
}

/** Whether this reader has the portal preview on. */
/** Whether this reader has the portal on. */
export function usePortalEnabled(): boolean {
return useSyncExternalStore(
subscribe,
() => state.enabled,
() => false,
readPortalEnabled,
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/routes/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const Route = createFileRoute("/")({
function IndexRoute() {
const { personId } = useViewer();
const portal = usePortalEnabled();
// `beforeLoad` only runs on navigation, so a reader who turns the preview on
// `beforeLoad` only runs on navigation, so a reader who turns the portal on
// while standing here would sit on the dashboard until a reload.
Comment on lines +21 to 22

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 8 'setPortalEnabled|usePortalEnabled|subscribe' \
  src/frontend/src/lib/portal/portal-store.ts \
  src/frontend/src/routes/index.tsx

Repository: constructorfabric/insight

Length of output: 6380


Correct the reload claim in the route comment.

IndexRoute subscribes through usePortalEnabled() via useSyncExternalStore(subscribe, ...), and setPortalEnabled() calls emit() before reading the store. If a reader enables the portal while this route is mounted, the hook re-renders and the route returns <Navigate to="/portal" replace /> without requiring a reload. Update the comment to describe beforeLoad as the initial-navigation fallback.

Proposed comment
-  // `beforeLoad` only runs on navigation, so a reader who turns the portal on
-  // while standing here would sit on the dashboard until a reload.
+  // `beforeLoad` handles the initial navigation. The subscribed store hook
+  // below redirects if the reader enables the portal while this route is open.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// `beforeLoad` only runs on navigation, so a reader who turns the portal on
// while standing here would sit on the dashboard until a reload.
// `beforeLoad` handles the initial navigation. The subscribed store hook
// below redirects if the reader enables the portal while this route is open.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/src/routes/index.tsx` around lines 21 - 22, Update the comment
near IndexRoute to remove the incorrect claim that enabling the portal while
mounted requires a reload. Explain that usePortalEnabled reacts to changes and
redirects immediately, while beforeLoad serves as the initial-navigation
fallback.

if (portal) return <Navigate to="/portal" replace />;
// An authenticated session always carries the person id (the gateway JWT
Expand Down
6 changes: 3 additions & 3 deletions src/frontend/src/routes/portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ export const Route = createFileRoute("/portal")({
});

function PortalRoute() {
// The root shell swaps in PortalLayout for this route while the preview is
// The root shell swaps in PortalLayout for this route while the portal is
// on, so this component only mounts with it OFF — a pasted /portal URL, or a
// viewer who turned the preview off while standing here. Either way the
// portal must not render: send them to the app they do have.
// viewer who opted out while standing here. Either way the portal must not
// render: send them to the app they do have.
Comment on lines +30 to +33

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the mount-state description.

The comment says PortalRoute only mounts while the portal is off. Line 35 returns <PortalLayout /> when usePortalEnabled() is true, including for direct /portal navigation. Describe the disabled redirect and enabled render paths separately.

Proposed comment
-  // The root shell swaps in PortalLayout for this route while the portal is
-  // on, so this component only mounts with it OFF — a pasted /portal URL, or a
-  // viewer who opted out while standing here. Either way the portal must not
-  // render: send them to the app they do have.
+  // When the portal is disabled, a direct /portal URL or an opt-out while
+  // already here redirects to /. When enabled, render PortalLayout.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// The root shell swaps in PortalLayout for this route while the portal is
// on, so this component only mounts with it OFF — a pasted /portal URL, or a
// viewer who turned the preview off while standing here. Either way the
// portal must not render: send them to the app they do have.
// viewer who opted out while standing here. Either way the portal must not
// render: send them to the app they do have.
// When the portal is disabled, a direct /portal URL or an opt-out while
// already here redirects to /. When enabled, render PortalLayout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/src/routes/portal.tsx` around lines 30 - 33, Update the comment
above PortalRoute to accurately describe both mount paths: when
usePortalEnabled() is false, redirect the user to the available app, and when it
is true—including direct /portal navigation—render PortalLayout. Remove the
inaccurate claim that this component only mounts with the portal disabled.

if (!usePortalEnabled()) return <Navigate to="/" replace />;
return <PortalLayout />;
}
Loading