From 677f0778e79e6149faede402794bf3dda60925e5 Mon Sep 17 00:00:00 2001 From: Anso Date: Sat, 2 May 2026 04:01:54 -0400 Subject: [PATCH] refactor(frontend): extract shared parts from PaidGate and AdmiralGate (#876) PaidGate and AdmiralGate were ~95% identical: same state machine (unlocked / compact-blurred / dismissed-pill / full-upsell-card), same 24h localStorage-backed dismissal logic, differing only in license predicate, dismiss-storage key, icon, and copy strings. Two reviewers flagged the duplication after PRs #874 and #875 landed identical changes in both files; the rule-of-three threshold is met. Extract the shared parts compositionally rather than as one big config- driven gate (the latter would just inline both gates' contents behind 8 props of indirection): - frontend/src/hooks/useDismissalState.ts owns the localStorage dismissal pattern. Lives under hooks/ to dodge the react-refresh/only-export-components lint rule that would fire if a hook coexisted with components in the same file. Validates the stored timestamp via Number.isFinite so a hand-edited or stale-extension garbage value defaults to "show the upsell" instead of crashing. - frontend/src/components/tierUpsell.tsx exports CompactBlurredLock, DismissedPill, and FullUpsellCard plus a shared TierGateProps interface. The compact-mode JSDoc lives on TierGateProps so the doc string lives in exactly one place. PaidGate and AdmiralGate become ~50-line compositions reading like a state machine. Public API of both gates is byte-stable: all 13+ consumers across the app continue to use and exactly as before. Two pre-existing security/polish issues fixed in passing while there is one source of truth for the affected JSX: - FullUpsellCard's window.open now passes 'noopener,noreferrer' to prevent the destination tab from accessing window.opener (reverse tabnabbing). - The Number.parseInt + Number.isFinite guard replaces a bare parseInt that would have happily accepted any prefix-numeric input. Adds a Vitest spec for useDismissalState covering: empty / recent / expired / non-numeric storage values, dismiss() / restore() side effects, the 24h boundary on fresh mount, and key independence. --- frontend/src/components/AdmiralGate.tsx | 115 ++++---------- frontend/src/components/PaidGate.tsx | 124 +++++---------- frontend/src/components/tierUpsell.tsx | 147 ++++++++++++++++++ .../hooks/__tests__/useDismissalState.test.ts | 92 +++++++++++ frontend/src/hooks/useDismissalState.ts | 40 +++++ 5 files changed, 353 insertions(+), 165 deletions(-) create mode 100644 frontend/src/components/tierUpsell.tsx create mode 100644 frontend/src/hooks/__tests__/useDismissalState.test.ts create mode 100644 frontend/src/hooks/useDismissalState.ts diff --git a/frontend/src/components/AdmiralGate.tsx b/frontend/src/components/AdmiralGate.tsx index fba8d428..4fd3f86a 100644 --- a/frontend/src/components/AdmiralGate.tsx +++ b/frontend/src/components/AdmiralGate.tsx @@ -1,106 +1,55 @@ -import { type ReactNode, useState } from 'react'; import { ShipWheel } from 'lucide-react'; -import { Button } from '@/components/ui/button'; import { useLicense } from '@/context/LicenseContext'; - -interface AdmiralGateProps { - children: ReactNode; - featureName?: string; - // Inline compact lock for list items (e.g. a single SSO provider card). Skips - // the full-page upsell and dismiss timer; always renders the blurred + pill style. - compact?: boolean; -} +import { useDismissalState } from '@/hooks/useDismissalState'; +import { + CompactBlurredLock, + DismissedPill, + FullUpsellCard, + type TierGateProps, +} from './tierUpsell'; const DISMISS_KEY = 'sencho-admiral-upgrade-prompt-dismissed'; -const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000; // 24 hours -function isDismissedFromStorage(): boolean { - const dismissedAt = localStorage.getItem(DISMISS_KEY); - return !!dismissedAt && Date.now() - parseInt(dismissedAt, 10) < DISMISS_DURATION_MS; -} - -export function AdmiralGate({ children, featureName = 'This feature', compact = false }: AdmiralGateProps) { +/** + * Gate for Admiral-tier-only features. Mirrors PaidGate's state machine + * but with a stricter license predicate (requires variant === 'admiral') + * and Admiral-themed icon/copy. + */ +export function AdmiralGate({ children, featureName = 'This feature', compact = false }: TierGateProps) { const { isPaid, license } = useLicense(); - const [dismissed, setDismissed] = useState(isDismissedFromStorage); + const { dismissed, dismiss, restore } = useDismissalState(DISMISS_KEY); if (isPaid && license?.variant === 'admiral') return <>{children}; - // Inline lock for list items (single SSO provider card, etc.). Renders the - // children blurred behind a small pill so the surrounding list keeps its - // shape; the IP exposure is minor because the children are tiny inline UI, - // and the visual continuity is intentional. Dismissal does not apply here. + const pillText = 'Upgrade to Admiral to unlock'; + if (compact) { return ( -
-
- {children} -
-
-
- - Upgrade to Admiral to unlock -
-
-
+ + {children} + ); } - // Post-dismissal placeholder for full-page consumers. Renders only the - // pill so the lazy-loaded children behind an Admiral view never mount - // during the dismissal window. Clicking the pill clears the dismissal - // flag and restores the full upsell card so users who dismissed - // accidentally or want to revisit pricing have a way back without - // clearing localStorage. if (dismissed) { - return ( -
- -
- ); + return ; } return ( -
-
- -
-
-

{featureName} requires Sencho Admiral

-

+ Unlock team features like LDAP / Active Directory, audit logging, API tokens, and unlimited user accounts with a Sencho Admiral license. For enterprise pricing or questions, contact{' '} licensing@sencho.io. -

-
-
- - -
-
+ + } + ctaIcon={ShipWheel} + ctaLabel="Get Admiral" + ctaHref="https://sencho.io/pricing" + onDismiss={dismiss} + /> ); } diff --git a/frontend/src/components/PaidGate.tsx b/frontend/src/components/PaidGate.tsx index 3104458d..22d4af01 100644 --- a/frontend/src/components/PaidGate.tsx +++ b/frontend/src/components/PaidGate.tsx @@ -1,105 +1,65 @@ -import { type ReactNode, useState } from 'react'; import { Compass } from 'lucide-react'; -import { Button } from '@/components/ui/button'; import { useLicense } from '@/context/LicenseContext'; - -interface PaidGateProps { - children: ReactNode; - featureName?: string; - // Inline compact lock for list items (e.g. a single SSO provider card). Skips - // the full-page upsell and dismiss timer; always renders the blurred + pill style. - compact?: boolean; -} +import { useDismissalState } from '@/hooks/useDismissalState'; +import { + CompactBlurredLock, + DismissedPill, + FullUpsellCard, + type TierGateProps, +} from './tierUpsell'; const DISMISS_KEY = 'sencho-upgrade-prompt-dismissed'; -const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000; // 24 hours (session-like) -function isDismissedFromStorage(): boolean { - const dismissedAt = localStorage.getItem(DISMISS_KEY); - return !!dismissedAt && Date.now() - parseInt(dismissedAt, 10) < DISMISS_DURATION_MS; -} - -export function PaidGate({ children, featureName = 'This feature', compact = false }: PaidGateProps) { +/** + * Gate for any paid-tier feature (Skipper or Admiral). Composes the + * shared tier-upsell primitives in `./tierUpsell` according to the + * current state: + * + * isPaid render children (unlocked). + * compact blurred children + small pill (inline list items). + * dismissed pill-only placeholder for 24h, click to restore. + * default full upsell card with View Plans CTA. + * + * The compact branch retains the blurred-children render because it is + * used for tiny inline UI where the visual continuity is intentional. + * The dismissed and default branches do not render children, so any + * lazy chunks behind the gate are never fetched on those paths. + */ +export function PaidGate({ children, featureName = 'This feature', compact = false }: TierGateProps) { const { isPaid } = useLicense(); - const [dismissed, setDismissed] = useState(isDismissedFromStorage); + const { dismissed, dismiss, restore } = useDismissalState(DISMISS_KEY); if (isPaid) return <>{children}; - // Inline lock for list items (single SSO provider card, etc.). Renders the - // children blurred behind a small pill so the surrounding list keeps its - // shape; the IP exposure is minor because the children are tiny inline UI, - // and the visual continuity is intentional. Dismissal does not apply here. + const pillText = `Upgrade to unlock ${featureName}`; + if (compact) { return ( -
-
- {children} -
-
-
- - Upgrade to unlock {featureName} -
-
-
+ + {children} + ); } - // Post-dismissal placeholder for full-page consumers. Renders only the - // pill so the lazy-loaded children behind a paid view never mount during - // the dismissal window. Clicking the pill clears the dismissal flag and - // restores the full upsell card so users who dismissed accidentally or - // want to revisit pricing have a way back without clearing localStorage. if (dismissed) { - return ( -
- -
- ); + return ; } return ( -
-
- -
-
-

{featureName} requires a paid license

-

+ Unlock features like fleet management, viewer accounts, one-click Google / GitHub / Okta SSO, and more with a Skipper or Admiral license. For enterprise pricing or questions, contact{' '} licensing@sencho.io. -

-
-
- - -
-
+ + } + ctaIcon={Compass} + ctaLabel="View Plans" + ctaHref="https://sencho.io/pricing" + onDismiss={dismiss} + /> ); } diff --git a/frontend/src/components/tierUpsell.tsx b/frontend/src/components/tierUpsell.tsx new file mode 100644 index 00000000..463b0deb --- /dev/null +++ b/frontend/src/components/tierUpsell.tsx @@ -0,0 +1,147 @@ +import { type ReactNode } from 'react'; +import { type LucideIcon } from 'lucide-react'; +import { Button } from '@/components/ui/button'; + +/** + * Shared rendering parts used by `PaidGate` and `AdmiralGate`. The two + * gates only differ in their license predicate, dismissal-storage key, + * icon, and copy strings; everything else (the compact-blurred-lock + * JSX, the dismissed-pill JSX, the full upsell card layout) is + * identical. Dismissal-state logic lives in `useDismissalState` under + * `frontend/src/hooks/`. + * + * These primitives let each gate compose its state machine in ~25 + * lines: + * + * if (isUnlocked) return children; + * if (compact) return {children}; + * if (dismissed) return ; + * return ; + * + * A future third gate (Skipper-only, or some hypothetical Enterprise + * tier) can mix and match these without re-writing the same JSX. + */ + +/** + * Public props shape for both `PaidGate` and `AdmiralGate`. Hoisted + * here so the `compact` doc string lives in exactly one place. + */ +export interface TierGateProps { + children: ReactNode; + featureName?: string; + /** + * Inline compact lock for list items (e.g. a single SSO provider + * card). Skips the full-page upsell and dismiss timer; always + * renders the blurred + pill style so the surrounding list keeps + * its shape. + */ + compact?: boolean; +} + +/** + * Inline list-item lock. Renders children blurred behind a small pill + * so the surrounding list keeps its shape. Used for tiny inline UI + * like a single SSO provider card; the IP exposure of the blurred + * children is minor and the visual continuity is intentional. + * Dismissal does not apply here. + */ +export function CompactBlurredLock({ + icon: Icon, + pillText, + children, +}: { + icon: LucideIcon; + pillText: string; + children: ReactNode; +}) { + return ( +
+
+ {children} +
+
+
+ + {pillText} +
+
+
+ ); +} + +/** + * Post-dismissal placeholder. Renders only the pill so any lazy-loaded + * children behind a paid view never mount during the 24h dismissal + * window. The pill is a button: clicking it calls `onClick` (typically + * the gate's `restore` action) so the full upsell card returns and the + * user has a way back without clearing localStorage. + */ +export function DismissedPill({ + icon: Icon, + pillText, + onClick, +}: { + icon: LucideIcon; + pillText: string; + onClick: () => void; +}) { + return ( +
+ +
+ ); +} + +/** + * Full-page upsell card. Centered icon chip + title + body (which can + * carry inline links) + Dismiss / CTA actions. The CTA opens the + * pricing page in a new tab with `noopener,noreferrer` to prevent the + * destination from accessing `window.opener` (reverse tabnabbing). + * Dismiss invokes `onDismiss`, which the gate uses to flip dismissal + * state. + */ +export function FullUpsellCard({ + icon: Icon, + title, + body, + ctaIcon: CtaIcon, + ctaLabel, + ctaHref, + onDismiss, +}: { + icon: LucideIcon; + title: string; + body: ReactNode; + ctaIcon: LucideIcon; + ctaLabel: string; + ctaHref: string; + onDismiss: () => void; +}) { + return ( +
+
+ +
+
+

{title}

+

{body}

+
+
+ + +
+
+ ); +} diff --git a/frontend/src/hooks/__tests__/useDismissalState.test.ts b/frontend/src/hooks/__tests__/useDismissalState.test.ts new file mode 100644 index 00000000..43a8f409 --- /dev/null +++ b/frontend/src/hooks/__tests__/useDismissalState.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; +import { useDismissalState } from '../useDismissalState'; + +const KEY = 'test-dismiss-key'; +const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000; + +describe('useDismissalState', () => { + beforeEach(() => { + localStorage.clear(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('returns dismissed=false when no timestamp is stored', () => { + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(false); + }); + + it('returns dismissed=true when a recent timestamp is stored', () => { + localStorage.setItem(KEY, String(Date.now() - 1000)); + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(true); + }); + + it('returns dismissed=false when the stored timestamp is past the 24h window', () => { + localStorage.setItem(KEY, String(Date.now() - DISMISS_DURATION_MS - 1)); + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(false); + }); + + it('returns dismissed=false when the stored value is non-numeric garbage', () => { + // Future-proofing: a stale extension or hand-edit could leave a non- + // numeric value at the key; the hook must not crash and must default + // to "show the upsell." + localStorage.setItem(KEY, 'not-a-number'); + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(false); + }); + + it('dismiss() flips dismissed to true and writes a parseable timestamp', () => { + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(false); + + act(() => result.current.dismiss()); + + expect(result.current.dismissed).toBe(true); + const stored = localStorage.getItem(KEY); + expect(stored).not.toBeNull(); + expect(Number.isFinite(Number.parseInt(stored as string, 10))).toBe(true); + }); + + it('restore() flips dismissed to false and removes the storage entry', () => { + localStorage.setItem(KEY, String(Date.now())); + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(true); + + act(() => result.current.restore()); + + expect(result.current.dismissed).toBe(false); + expect(localStorage.getItem(KEY)).toBeNull(); + }); + + it('respects the 24h boundary: a fresh mount past expiry sees dismissed=false', () => { + // Dismiss now, then advance time past the window, then mount fresh. + // The original hook instance keeps its `dismissed: true` state in + // React (lazy initializer runs once), so the boundary is observable + // only on a fresh mount, not via re-render of the same hook. + const { result, unmount } = renderHook(() => useDismissalState(KEY)); + act(() => result.current.dismiss()); + expect(result.current.dismissed).toBe(true); + + unmount(); + vi.advanceTimersByTime(DISMISS_DURATION_MS + 1000); + + const fresh = renderHook(() => useDismissalState(KEY)); + expect(fresh.result.current.dismissed).toBe(false); + }); + + it('different keys are independent', () => { + const { result: a } = renderHook(() => useDismissalState('key-a')); + const { result: b } = renderHook(() => useDismissalState('key-b')); + + act(() => a.current.dismiss()); + + expect(a.current.dismissed).toBe(true); + expect(b.current.dismissed).toBe(false); + }); +}); diff --git a/frontend/src/hooks/useDismissalState.ts b/frontend/src/hooks/useDismissalState.ts new file mode 100644 index 00000000..5e36f28a --- /dev/null +++ b/frontend/src/hooks/useDismissalState.ts @@ -0,0 +1,40 @@ +import { useState } from 'react'; + +const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000; + +/** + * Tracks the localStorage-backed dismissal flag for an upsell gate. + * `dismiss()` writes the current timestamp and flips state to dismissed; + * `restore()` removes the timestamp so the next render falls through to + * the full upsell card. The dismissal window is 24h; after expiry, + * `localStorage.getItem(key)` still returns a stale timestamp but the + * lazy initializer treats it as expired and returns `dismissed: false`, + * so the gate shows the full upsell again on next mount. + * + * Each hook instance owns its own React state. If two `` + * mounts share the same key in the same tab, dismissing one does not + * sync to the other until that other re-mounts. Cross-instance pub-sub + * is not provided; the dismissal is a soft UX nicety, not a security + * boundary. + */ +export function useDismissalState(key: string) { + const [dismissed, setDismissed] = useState(() => { + const dismissedAt = localStorage.getItem(key); + if (!dismissedAt) return false; + const ts = Number.parseInt(dismissedAt, 10); + if (!Number.isFinite(ts)) return false; + return Date.now() - ts < DISMISS_DURATION_MS; + }); + + const dismiss = () => { + localStorage.setItem(key, Date.now().toString()); + setDismissed(true); + }; + + const restore = () => { + localStorage.removeItem(key); + setDismissed(false); + }; + + return { dismissed, dismiss, restore }; +}