Skip to content
Draft
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
7 changes: 7 additions & 0 deletions .changeset/heavy-donkeys-greet.md
Original file line number Diff line number Diff line change
@@ -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
18 changes: 16 additions & 2 deletions packages/client/src/clients/guide/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
formatFilters,
formatGroupStage,
formatState,
getToolbarRunConfigFromUrl,
mockDefaultGroup,
newUrl,
predicateUrlPatterns,
Expand Down Expand Up @@ -600,6 +601,12 @@ export class KnockGuideClient {

setDebug(debugOpts?: Omit<DebugState, "debugging">) {
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
Expand All @@ -622,7 +629,6 @@ export class KnockGuideClient {
`[Guide] Start debugging, refetching guides and resubscribing to the websocket channel`,
);
this.fetch({ force: true });
this.subscribe();
}
}

Expand All @@ -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();
}
}

Expand Down Expand Up @@ -1480,4 +1490,8 @@ export class KnockGuideClient {
this.replaceStateFn = undefined;
}
}

static getToolbarRunConfigFromUrl() {
return getToolbarRunConfigFromUrl();
}
}
38 changes: 38 additions & 0 deletions packages/client/src/clients/guide/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
GuideActivationUrlRuleData,
GuideData,
GuideGroupData,
KnockGuide,
KnockGuideActivationUrlPattern,
SelectFilterParams,
StoreState,
Expand Down Expand Up @@ -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<KnockGuide["key"], true>;
};

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;
};
1 change: 1 addition & 0 deletions packages/client/src/clients/guide/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export {
checkActivatable,
} from "./client";
export { checkStateIfThrottled } from "./helpers";
export type { ToolbarV2RunConfig } from "./helpers";
export type {
KnockGuide,
KnockGuideStep,
Expand Down
43 changes: 39 additions & 4 deletions packages/client/test/clients/guide/guide.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand All @@ -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 };
Expand Down
58 changes: 57 additions & 1 deletion packages/client/test/clients/guide/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
44 changes: 43 additions & 1 deletion packages/react-core/test/guide/KnockGuideProvider.test.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -29,14 +29,19 @@ var subscribeMock: ReturnType<typeof vi.fn>;
// eslint-disable-next-line no-var
var cleanupMock: ReturnType<typeof vi.fn>;
// eslint-disable-next-line no-var
var getToolbarRunConfigFromUrlMock: ReturnType<typeof vi.fn>;
// eslint-disable-next-line no-var
var MockKnockGuideClient: new () => unknown;

vi.mock("@knocklabs/client", () => {
fetchMock = vi.fn();
subscribeMock = vi.fn();
cleanupMock = vi.fn();

getToolbarRunConfigFromUrlMock = vi.fn(() => ({ isVisible: false }));

MockKnockGuideClient = class {
static getToolbarRunConfigFromUrl = getToolbarRunConfigFromUrlMock;
fetch = fetchMock;
subscribe = subscribeMock;
cleanup = cleanupMock;
Expand All @@ -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(
Expand All @@ -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();
});
});
Loading
Loading