diff --git a/.changeset/heavy-donkeys-greet.md b/.changeset/heavy-donkeys-greet.md new file mode 100644 index 000000000..9d4279ee0 --- /dev/null +++ b/.changeset/heavy-donkeys-greet.md @@ -0,0 +1,7 @@ +--- +"@knocklabs/react-core": patch +"@knocklabs/client": patch +"@knocklabs/react": patch +--- + +[Guides] Use `readyToTarget` to coordinate fetching guides in guide toolbar v2 diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 635fd7095..ed107effc 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -13,6 +13,7 @@ import { formatFilters, formatGroupStage, formatState, + getToolbarRunConfigFromUrl, mockDefaultGroup, newUrl, predicateUrlPatterns, @@ -600,6 +601,12 @@ export class KnockGuideClient { setDebug(debugOpts?: Omit) { this.knock.log("[Guide] .setDebug()"); + + if (!this.knock.isAuthenticated()) { + this.knock.log("[Guide] Not authenticated, cannot start debugging"); + return; + } + const shouldRefetch = !this.store.state.debug?.debugging; // Clear the stage before updating the store state to let the next render @@ -622,7 +629,6 @@ export class KnockGuideClient { `[Guide] Start debugging, refetching guides and resubscribing to the websocket channel`, ); this.fetch({ force: true }); - this.subscribe(); } } @@ -636,12 +642,16 @@ export class KnockGuideClient { this.store.setState((state) => ({ ...state, debug: undefined })); + if (!this.knock.isAuthenticated()) { + this.knock.log("[Guide] Not authenticated, will not refetch"); + return; + } + if (shouldRefetch) { this.knock.log( `[Guide] Stop debugging, refetching guides and resubscribing to the websocket channel`, ); this.fetch({ force: true }); - this.subscribe(); } } @@ -1480,4 +1490,8 @@ export class KnockGuideClient { this.replaceStateFn = undefined; } } + + static getToolbarRunConfigFromUrl() { + return getToolbarRunConfigFromUrl(); + } } diff --git a/packages/client/src/clients/guide/helpers.ts b/packages/client/src/clients/guide/helpers.ts index 7e163dcd2..1858b346e 100644 --- a/packages/client/src/clients/guide/helpers.ts +++ b/packages/client/src/clients/guide/helpers.ts @@ -3,6 +3,7 @@ import { GuideActivationUrlRuleData, GuideData, GuideGroupData, + KnockGuide, KnockGuideActivationUrlPattern, SelectFilterParams, StoreState, @@ -225,3 +226,40 @@ export const predicateUrlPatterns = ( } }, predicateDefault); }; + +// Use this param to start Toolbar v2 and enter into a debugging session when +// it is present and set to true. +const TOOLBAR_QUERY_PARAM = "knock_guide_toolbar"; + +// Optional, when present pin/focus on this guide. +const GUIDE_KEY_PARAM = "focused_guide_key"; + +export type ToolbarV2RunConfig = { + isVisible: boolean; + focusedGuideKeys?: Record; +}; + +export const getToolbarRunConfigFromUrl = (): ToolbarV2RunConfig => { + const fallback = { isVisible: false }; + + if (typeof window === "undefined" || !window.location) { + return fallback; + } + + const urlSearchParams = new URLSearchParams(window.location.search); + const toolbarParamValue = urlSearchParams.get(TOOLBAR_QUERY_PARAM); + const guideKeyParamValue = urlSearchParams.get(GUIDE_KEY_PARAM); + + if (toolbarParamValue === null) { + return fallback; + } + + const config: ToolbarV2RunConfig = { + isVisible: toolbarParamValue === "true", + }; + if (guideKeyParamValue) { + config.focusedGuideKeys = { [guideKeyParamValue]: true }; + } + + return config; +}; diff --git a/packages/client/src/clients/guide/index.ts b/packages/client/src/clients/guide/index.ts index effa92f9d..0834dd155 100644 --- a/packages/client/src/clients/guide/index.ts +++ b/packages/client/src/clients/guide/index.ts @@ -4,6 +4,7 @@ export { checkActivatable, } from "./client"; export { checkStateIfThrottled } from "./helpers"; +export type { ToolbarV2RunConfig } from "./helpers"; export type { KnockGuide, KnockGuideStep, diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index 770f1c157..81ef8cdb3 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -4714,9 +4714,9 @@ describe("KnockGuideClient", () => { expect(client.store.state.debug!.debugging!).toBe(true); - // calls fetch and subscribe when not already debugging + // calls fetch when not already debugging; subscribe is managed by V2 toolbar expect(fetchSpy).toHaveBeenCalled(); - expect(subscribeSpy).toHaveBeenCalled(); + expect(subscribeSpy).not.toHaveBeenCalled(); }); test("sets debug state with provided options", () => { @@ -4806,6 +4806,21 @@ describe("KnockGuideClient", () => { expect(client.store.state.debug!.ignoreDisplayInterval).toBe(false); }); + + test("returns early without setting debug when not authenticated", () => { + vi.mocked(mockKnock.isAuthenticated).mockReturnValue(false); + const client = new KnockGuideClient(mockKnock, channelId); + client.store.state.debug = undefined; + + const fetchSpy = vi + .spyOn(client, "fetch") + .mockImplementation(() => Promise.resolve({ status: "ok" })); + + client.setDebug(); + + expect(client.store.state.debug).toBe(undefined); + expect(fetchSpy).not.toHaveBeenCalled(); + }); }); describe("unsetDebug", () => { @@ -4844,9 +4859,9 @@ describe("KnockGuideClient", () => { expect(client.store.state.debug).toBe(undefined); - // calls fetch and subscribe when was debugging + // calls fetch when was debugging; subscribe is managed by V2 toolbar expect(fetchSpy).toHaveBeenCalled(); - expect(subscribeSpy).toHaveBeenCalled(); + expect(subscribeSpy).not.toHaveBeenCalled(); }); test("does not call fetch and subscribe when was not debugging", () => { @@ -4868,6 +4883,26 @@ describe("KnockGuideClient", () => { expect(subscribeSpy).not.toHaveBeenCalled(); }); + test("clears debug state but does not fetch when not authenticated", () => { + const client = new KnockGuideClient(mockKnock, channelId); + client.store.state.debug = { debugging: true }; + + vi.mocked(mockKnock.isAuthenticated).mockReturnValue(false); + + const fetchSpy = vi + .spyOn(client, "fetch") + .mockImplementation(() => Promise.resolve({ status: "ok" })); + const subscribeSpy = vi + .spyOn(client, "subscribe") + .mockImplementation(() => {}); + + client.unsetDebug(); + + expect(client.store.state.debug).toBe(undefined); + expect(fetchSpy).not.toHaveBeenCalled(); + expect(subscribeSpy).not.toHaveBeenCalled(); + }); + test("does not call fetch and subscribe when debug exists but debugging is false", () => { const client = new KnockGuideClient(mockKnock, channelId); client.store.state.debug = { debugging: false }; diff --git a/packages/client/test/clients/guide/helpers.test.ts b/packages/client/test/clients/guide/helpers.test.ts index ed9a875f6..b7b932eb3 100644 --- a/packages/client/test/clients/guide/helpers.test.ts +++ b/packages/client/test/clients/guide/helpers.test.ts @@ -1,8 +1,9 @@ -import { describe, expect, test } from "vitest"; +import { describe, expect, test, vi, afterEach } from "vitest"; import { URLPattern } from "urlpattern-polyfill"; import { evaluateUrlRule, + getToolbarRunConfigFromUrl, predicateUrlRules, predicateUrlPatterns, } from "../../../src/clients/guide/helpers"; @@ -679,3 +680,58 @@ describe("predicateUrlPatterns", () => { }); }); }); + +describe("getToolbarRunConfigFromUrl", () => { + const stubWindow = (search: string) => { + vi.stubGlobal("window", { location: { search } }); + }; + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + test("returns isVisible true when URL param is 'true'", () => { + stubWindow("?knock_guide_toolbar=true"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: true }); + }); + + test("returns isVisible false when URL param is 'false'", () => { + stubWindow("?knock_guide_toolbar=false"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: false }); + }); + + test("returns default config when URL param is absent", () => { + stubWindow(""); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: false }); + }); + + test("includes focusedGuideKeys when focused_guide_key URL param is present", () => { + stubWindow("?knock_guide_toolbar=true&focused_guide_key=my_guide"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ + isVisible: true, + focusedGuideKeys: { my_guide: true }, + }); + }); + + test("does not include focusedGuideKeys when focused_guide_key param is absent", () => { + stubWindow("?knock_guide_toolbar=true"); + + const config = getToolbarRunConfigFromUrl(); + + expect(config).toEqual({ isVisible: true }); + expect(config).not.toHaveProperty("focusedGuideKeys"); + }); +}); diff --git a/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx b/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx index b69f3ba33..17594e70e 100644 --- a/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx +++ b/packages/react-core/src/modules/guide/context/KnockGuideProvider.tsx @@ -75,7 +75,11 @@ export const KnockGuideProvider: React.FC< ]); React.useEffect(() => { - if (readyToTarget) { + // When the toolbar v2 is visible, defer fetch/subscribe to the toolbar + // so it can drive the debugging session lifecycle. + const toolbarRunConfig = KnockGuideClient.getToolbarRunConfigFromUrl(); + + if (readyToTarget && !toolbarRunConfig.isVisible) { knockGuideClient.fetch(); if (listenForUpdates) knockGuideClient.subscribe(); } diff --git a/packages/react-core/test/guide/KnockGuideProvider.test.tsx b/packages/react-core/test/guide/KnockGuideProvider.test.tsx index 9dc45fae0..899342712 100644 --- a/packages/react-core/test/guide/KnockGuideProvider.test.tsx +++ b/packages/react-core/test/guide/KnockGuideProvider.test.tsx @@ -1,6 +1,6 @@ import { renderHook } from "@testing-library/react"; import React from "react"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { KnockGuideProvider } from "../../src/modules/guide/context/KnockGuideProvider"; import { useGuideContext } from "../../src/modules/guide/hooks/useGuideContext"; @@ -29,6 +29,8 @@ var subscribeMock: ReturnType; // eslint-disable-next-line no-var var cleanupMock: ReturnType; // eslint-disable-next-line no-var +var getToolbarRunConfigFromUrlMock: ReturnType; +// eslint-disable-next-line no-var var MockKnockGuideClient: new () => unknown; vi.mock("@knocklabs/client", () => { @@ -36,7 +38,10 @@ vi.mock("@knocklabs/client", () => { subscribeMock = vi.fn(); cleanupMock = vi.fn(); + getToolbarRunConfigFromUrlMock = vi.fn(() => ({ isVisible: false })); + MockKnockGuideClient = class { + static getToolbarRunConfigFromUrl = getToolbarRunConfigFromUrlMock; fetch = fetchMock; subscribe = subscribeMock; cleanup = cleanupMock; @@ -50,6 +55,13 @@ describe("KnockGuideProvider", () => { // Deliberately omitting negative-case test that asserts throwing without KnockProvider // as React swallows errors thrown during rendering, making the assertion unreliable + beforeEach(() => { + fetchMock.mockClear(); + subscribeMock.mockClear(); + cleanupMock.mockClear(); + getToolbarRunConfigFromUrlMock.mockReturnValue({ isVisible: false }); + }); + it("provides context and triggers fetch/subscribe when ready", () => { const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => React.createElement( @@ -65,4 +77,34 @@ describe("KnockGuideProvider", () => { expect(fetchMock).toHaveBeenCalled(); expect(subscribeMock).toHaveBeenCalled(); }); + + it("does not subscribe when listenForUpdates is false", () => { + const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => + React.createElement( + KnockGuideProvider, + { channelId: "feed", readyToTarget: true, listenForUpdates: false }, + children, + ); + + renderHook(() => useGuideContext(), { wrapper }); + + expect(fetchMock).toHaveBeenCalled(); + expect(subscribeMock).not.toHaveBeenCalled(); + }); + + it("defers fetch/subscribe to toolbar v2 when toolbar is visible", () => { + getToolbarRunConfigFromUrlMock.mockReturnValue({ isVisible: true }); + + const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => + React.createElement( + KnockGuideProvider, + { channelId: "feed", readyToTarget: true }, + children, + ); + + renderHook(() => useGuideContext(), { wrapper }); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(subscribeMock).not.toHaveBeenCalled(); + }); }); diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx index 46d1a0d2a..65b1cd651 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx +++ b/packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx @@ -1,3 +1,4 @@ +import { KnockGuideClient } from "@knocklabs/client"; import { useGuideContext } from "@knocklabs/react-core"; import { Button } from "@telegraph/button"; import { Icon } from "@telegraph/icon"; @@ -22,7 +23,7 @@ import "../styles.css"; import { FocusChin } from "./FocusChin"; import { GuideContextDetails } from "./GuideContextDetails"; import { GuideRow } from "./GuideRow"; -import { DisplayOption, getRunConfig, sharedTooltipProps } from "./helpers"; +import { DisplayOption, sharedTooltipProps } from "./helpers"; import { useDraggable } from "./useDraggable"; import { InspectionResultOk, @@ -84,12 +85,19 @@ const filterGuides = ( }); }; -export const V2 = () => { +type Props = { + readyToTarget: boolean; + listenForUpdates: boolean; +}; + +export const V2 = ({ readyToTarget, listenForUpdates }: Props) => { const { client } = useGuideContext(); const [displayOption, setDisplayOption] = React.useState("only-active"); - const [runConfig, setRunConfig] = React.useState(() => getRunConfig()); + const [runConfig, setRunConfig] = React.useState(() => + KnockGuideClient.getToolbarRunConfigFromUrl(), + ); const [isCollapsed, setIsCollapsed] = React.useState(false); const [isContextPanelOpen, setIsContextPanelOpen] = React.useState(false); @@ -104,8 +112,16 @@ export const V2 = () => { React.useEffect(() => { const { isVisible = false, focusedGuideKeys = {} } = runConfig || {}; const isDebugging = client.store.state.debug?.debugging; - if (isVisible && !isDebugging) { + + const stopDebug = () => { + client.unsetDebug(); + listenForUpdates ? client.subscribe() : client.unsubscribe(); + }; + + // Always subscribe while debugging. + if (isVisible && !isDebugging && readyToTarget) { client.setDebug({ focusedGuideKeys }); + client.subscribe(); // If focused, switch to all guides so you can see in the list. if (Object.keys(focusedGuideKeys).length > 0) { @@ -113,10 +129,12 @@ export const V2 = () => { } } - return () => { - client.unsetDebug(); - }; - }, [runConfig, client, setDisplayOption]); + if (!isVisible && isDebugging) { + stopDebug(); + } + + return () => stopDebug(); + }, [readyToTarget, listenForUpdates, runConfig, client, setDisplayOption]); // Toggle collapsed state with Ctrl + . React.useEffect(() => { @@ -344,7 +362,6 @@ export const V2 = () => { leadingIcon={{ icon: LogOut, alt: "Exit" }} onClick={() => { setRunConfig((curr) => ({ ...curr, isVisible: false })); - client.unsetDebug(); }} > Exit diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts b/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts index da805edb1..9097b9bef 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts +++ b/packages/react/src/modules/guide/components/Toolbar/V2/helpers.ts @@ -1,51 +1,9 @@ -import { KnockGuide } from "@knocklabs/client"; - -import { checkForWindow } from "../../../../../modules/core"; - export const sharedTooltipProps = { delayDuration: 1000, }; export type DisplayOption = "all-guides" | "only-active" | "only-eligible"; -// Use this param to start Toolbar and enter into a debugging session when -// it is present and set to true. -const TOOLBAR_QUERY_PARAM = "knock_guide_toolbar"; - -// Optional, when present pin/focus on this guide. -const GUIDE_KEY_PARAM = "focused_guide_key"; - -export type ToolbarV2RunConfig = { - isVisible: boolean; - focusedGuideKeys?: Record; -}; - -export const getRunConfig = (): ToolbarV2RunConfig => { - const fallback = { isVisible: false }; - - const win = checkForWindow(); - if (!win || !win.location) { - return fallback; - } - - const urlSearchParams = new URLSearchParams(win.location.search); - const toolbarParamValue = urlSearchParams.get(TOOLBAR_QUERY_PARAM); - const guideKeyParamValue = urlSearchParams.get(GUIDE_KEY_PARAM); - - if (toolbarParamValue === null) { - return fallback; - } - - const config: ToolbarV2RunConfig = { - isVisible: toolbarParamValue === "true", - }; - if (guideKeyParamValue) { - config.focusedGuideKeys = { [guideKeyParamValue]: true }; - } - - return config; -}; - export const FOCUS_ERRORS = { focusUnknownGuide: "No such guide exists", focusUncommittedGuide: "This guide has not been committed", diff --git a/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts b/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts index c9c044ef3..8622dcfdb 100644 --- a/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts +++ b/packages/react/src/modules/guide/components/Toolbar/V2/useInspectGuideClientStore.ts @@ -4,12 +4,13 @@ import { KnockGuideClientStoreState, KnockGuideIneligibilityMarker, KnockGuideSelectionResult, + type ToolbarV2RunConfig, checkActivatable, checkStateIfThrottled, } from "@knocklabs/client"; import { useGuideContext, useStore } from "@knocklabs/react-core"; -import { FOCUS_ERRORS, ToolbarV2RunConfig } from "./helpers"; +import { FOCUS_ERRORS } from "./helpers"; const byKey = (items: T[]) => { return items.reduce((acc, item) => ({ ...acc, [item.key]: item }), {}); diff --git a/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx b/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx index 7ef57306a..05bccb40d 100644 --- a/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx +++ b/packages/react/src/modules/guide/providers/KnockGuideProvider.tsx @@ -16,16 +16,27 @@ type Props = KnockGuideProviderProps & { export const KnockGuideProvider: React.FC> = ({ children, toolbar = "v2", + readyToTarget, + listenForUpdates = true, ...props }) => { return ( {children} - {toolbar === "v2" ? : } + {toolbar === "v2" ? ( + + ) : ( + + )} ); }; diff --git a/packages/react/test/guide/Toolbar/V2/helpers.test.ts b/packages/react/test/guide/Toolbar/V2/helpers.test.ts deleted file mode 100644 index d8908c831..000000000 --- a/packages/react/test/guide/Toolbar/V2/helpers.test.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { describe, expect, test, vi, afterEach } from "vitest"; - -import { getRunConfig } from "../../../../src/modules/guide/components/Toolbar/V2/helpers"; - -describe("Toolbar V2 helpers", () => { - afterEach(() => { - vi.unstubAllGlobals(); - vi.restoreAllMocks(); - }); - - describe("getRunConfig", () => { - test("returns isVisible true when URL param is 'true'", () => { - Object.defineProperty(window, "location", { - value: { search: "?knock_guide_toolbar=true" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: true }); - }); - - test("returns isVisible false when URL param is 'false'", () => { - Object.defineProperty(window, "location", { - value: { search: "?knock_guide_toolbar=false" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: false }); - }); - - test("returns default config when URL param is absent", () => { - Object.defineProperty(window, "location", { - value: { search: "" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: false }); - }); - - test("includes focusedGuideKeys when focused_guide_key URL param is present", () => { - Object.defineProperty(window, "location", { - value: { - search: "?knock_guide_toolbar=true&focused_guide_key=my_guide", - }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ - isVisible: true, - focusedGuideKeys: { my_guide: true }, - }); - }); - - test("does not include focusedGuideKeys when focused_guide_key param is absent", () => { - Object.defineProperty(window, "location", { - value: { search: "?knock_guide_toolbar=true" }, - writable: true, - configurable: true, - }); - - const config = getRunConfig(); - - expect(config).toEqual({ isVisible: true }); - expect(config).not.toHaveProperty("focusedGuideKeys"); - }); - }); -});