From 495bb41c05585e4a7f8feac6bec354f8dc17b97b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 11 Jul 2024 08:24:10 +0100 Subject: [PATCH] fix: Reduce React re-renders when checking `currentCard()` (#3398) --- .../src/@planx/components/Section/Public.tsx | 2 +- .../@planx/components/shared/Preview/Card.tsx | 6 +- editor.planx.uk/src/components/Header.tsx | 9 +- editor.planx.uk/src/lib/feedback.ts | 3 +- .../FlowEditor/lib/__tests__/clones.test.ts | 16 +-- .../FlowEditor/lib/__tests__/filters.test.ts | 17 ++- .../lib/__tests__/preview/canGoBack.test.ts | 105 +++++++----------- .../__tests__/preview/overrideAnswer.test.ts | 4 +- .../__tests__/preview/previousCard.test.ts | 8 +- .../removeNodesDependentOnPassport.test.ts | 23 ++-- .../__tests__/preview/upcomingCardIds.test.ts | 4 +- .../FlowEditor/lib/__tests__/setValue.test.ts | 20 ++-- .../FlowEditor/lib/analytics/provider.tsx | 6 +- .../src/pages/FlowEditor/lib/store/preview.ts | 20 ++-- .../src/pages/Preview/Questions.tsx | 8 +- .../src/pages/Preview/ReconciliationPage.tsx | 2 +- 16 files changed, 126 insertions(+), 127 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/Section/Public.tsx b/editor.planx.uk/src/@planx/components/Section/Public.tsx index 2b7831c3db..eab14c9496 100644 --- a/editor.planx.uk/src/@planx/components/Section/Public.tsx +++ b/editor.planx.uk/src/@planx/components/Section/Public.tsx @@ -33,7 +33,7 @@ export default function Component(props: Props) { state.breadcrumbs, state.cachedBreadcrumbs, state.changeAnswer, - state.currentCard(), + state.currentCard, state.currentSectionIndex, state.flow, state.flowName, diff --git a/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx b/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx index 6eb6ba8dda..72c8e261af 100644 --- a/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx @@ -48,7 +48,7 @@ const Card: React.FC = ({ ...props }) => { const theme = useTheme(); - const [path, currentCard] = useStore((state) => [ + const [path, visibleNode] = useStore((state) => [ state.path, state.currentCard, ]); @@ -57,9 +57,7 @@ const Card: React.FC = ({ const { track } = useAnalyticsTracking(); useEffect(() => { - // The Card component is only rendered when there's content the user will - // see - const visibleNode = currentCard(); + // The Card component is only rendered when there's content the user will see if (visibleNode?.id) track(visibleNode?.id); }, []); diff --git a/editor.planx.uk/src/components/Header.tsx b/editor.planx.uk/src/components/Header.tsx index fefadf8f55..467af46ad1 100644 --- a/editor.planx.uk/src/components/Header.tsx +++ b/editor.planx.uk/src/components/Header.tsx @@ -259,20 +259,19 @@ const Breadcrumbs: React.FC = () => { }; const NavBar: React.FC = () => { - const [index, sectionCount, title, hasSections, saveToEmail, path] = useStore( - (state) => [ + const [index, sectionCount, title, hasSections, saveToEmail, path, node] = + useStore((state) => [ state.currentSectionIndex, state.sectionCount, state.currentSectionTitle, state.hasSections, state.saveToEmail, state.path, - ], - ); + state.currentCard, + ]); const isSaveAndReturnLandingPage = path !== ApplicationPath.SingleSession && !saveToEmail; const isContentPage = useCurrentRoute()?.data?.isContentPage; - const { node } = useAnalyticsTracking(); const isSectionCard = node?.type == TYPES.Section; const isVisible = hasSections && diff --git a/editor.planx.uk/src/lib/feedback.ts b/editor.planx.uk/src/lib/feedback.ts index fec8766746..f4e6464512 100644 --- a/editor.planx.uk/src/lib/feedback.ts +++ b/editor.planx.uk/src/lib/feedback.ts @@ -23,13 +23,12 @@ export type FeedbackMetadata = { export async function getInternalFeedbackMetadata(): Promise { const { breadcrumbs, - currentCard, + currentCard: node, computePassport, fetchCurrentTeam, id: flowId, } = useStore.getState(); const { id: teamId } = await fetchCurrentTeam(); - const node = currentCard(); const userData = { breadcrumbs: breadcrumbs, passport: computePassport(), diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/clones.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/clones.test.ts index 3f608eda46..71f051ccf6 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/clones.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/clones.test.ts @@ -3,7 +3,7 @@ const { getState, setState } = vanillaStore; import forwardsFlow from "./mocks/flowWithClones.json"; import reverseFlow from "./mocks/flowWithReverseClones.json"; -const { record, previousCard, currentCard, upcomingCardIds, resetPreview } = +const { record, previousCard, getCurrentCard, upcomingCardIds, resetPreview } = getState(); beforeEach(() => { @@ -54,14 +54,14 @@ describe("Clone order in flow (backwards)", () => { // Traverse forward to final node record("question", { answers: ["leftChoice"] }); record("clone", { answers: ["finalNode"] }); - expect(currentCard()?.id).toBe("finalNode"); + expect(getCurrentCard()?.id).toBe("finalNode"); // Traverse back one-by-one to first node - let previous = previousCard(currentCard()); + let previous = previousCard(getCurrentCard()); expect(previous).toBe("clone"); record(previous!); - previous = previousCard(currentCard()); + previous = previousCard(getCurrentCard()); expect(previous).toBe("question"); record(previous!); @@ -79,18 +79,18 @@ describe("Clone order in flow (backwards)", () => { record("question", { answers: ["rightChoice"] }); record("rightNotice", { answers: ["clone"] }); record("clone", { answers: ["finalNode"] }); - expect(currentCard()?.id).toBe("finalNode"); + expect(getCurrentCard()?.id).toBe("finalNode"); // Traverse back one-by-one to first node - let previous = previousCard(currentCard()); + let previous = previousCard(getCurrentCard()); expect(previous).toBe("clone"); record(previous!); - previous = previousCard(currentCard()); + previous = previousCard(getCurrentCard()); expect(previous).toBe("rightNotice"); record(previous!); - previous = previousCard(currentCard()); + previous = previousCard(getCurrentCard()); expect(previous).toBe("question"); record(previous!); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/filters.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/filters.test.ts index dc1d3451d7..30e90ac03d 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/filters.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/filters.test.ts @@ -4,8 +4,13 @@ import flowWithBranchingFilters from "./mocks/flowWithBranchingFilters.json"; import flowWithRootFilter from "./mocks/flowWithRootFilter.json"; const { getState, setState } = vanillaStore; -const { upcomingCardIds, resetPreview, record, currentCard, collectedFlags } = - getState(); +const { + upcomingCardIds, + resetPreview, + record, + getCurrentCard, + collectedFlags, +} = getState(); // https://i.imgur.com/k0kkKox.png describe("A filter on the root of the graph", () => { @@ -74,9 +79,9 @@ describe("A filter on a branch", () => { record("fork", { answers: ["filter2"] }); // XXX: Test fails here - // The currentCard returns as "immunityFlag2" which we should not land on - + // getCurrentCard returns as "immunityFlag2" which we should not land on - // the flags on the first filter are skipped, we go direct from "immunityPath1" to "fork" - expect(currentCard()?.id).toBe("immunityPath2"); + expect(getCurrentCard()?.id).toBe("immunityPath2"); }); }); @@ -97,7 +102,7 @@ describe("Nodes on a filter path should only be auto-answered when the path matc record("TiIuAVIXsV", { answers: ["hdaeOVIXsV"], auto: false }); // land on the correct result component - expect(currentCard()?.id).toBe("seN42VIXsV"); + expect(getCurrentCard()?.id).toBe("seN42VIXsV"); expect(getState().resultData()["Planning permission"]).toHaveProperty( "flag.value", "PLANNING_PERMISSION_REQUIRED", @@ -131,7 +136,7 @@ describe("Nodes on a filter path should only be auto-answered when the path matc upcomingCardIds(); // land on the correct result component - expect(currentCard()?.id).toBe("seN42VIXsV"); + expect(getCurrentCard()?.id).toBe("seN42VIXsV"); expect(getState().resultData()["Planning permission"]).toHaveProperty( "flag.value", "PLANNING_PERMISSION_REQUIRED", diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/canGoBack.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/canGoBack.test.ts index be23cda4d3..6a12ab655d 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/canGoBack.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/canGoBack.test.ts @@ -3,6 +3,8 @@ import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { Store, vanillaStore } from "../../store"; const { getState, setState } = vanillaStore; +const { canGoBack, getCurrentCard, resetPreview, record, changeAnswer } = + getState(); // https://imgur.com/VFV64ax const flow: Store.flow = { @@ -58,7 +60,7 @@ const flow: Store.flow = { }; beforeEach(() => { - getState().resetPreview(); + resetPreview(); setState({ flow, }); @@ -66,90 +68,69 @@ beforeEach(() => { describe("can go back if", () => { test("the previous component was manually answered", () => { - setState({ - breadcrumbs: { - Question: { - auto: false, - answers: ["NoFeeAnswerPath"], - }, - }, + record("Question", { + auto: false, + answers: ["NoFeeAnswerPath"], }); - expect(getState().canGoBack(getState().currentCard())).toStrictEqual(true); + + expect(canGoBack(getCurrentCard())).toStrictEqual(true); }); test("the user skipped the payment component", () => { - setState({ - breadcrumbs: { - Question: { - auto: false, - answers: ["NoFeeAnswerPath"], - }, - Pay: { - auto: true, - }, - }, + record("Question", { + auto: false, + answers: ["NoFeeAnswerPath"], }); - expect(getState().canGoBack(getState().currentCard())).toStrictEqual(true); + record("Pay", { auto: true }); + + expect(canGoBack(getCurrentCard())).toStrictEqual(true); }); }); describe("cannot go back if", () => { test("it's the very first component", () => { - expect(getState().canGoBack(getState().currentCard())).toStrictEqual(false); + expect(canGoBack(getCurrentCard())).toStrictEqual(false); }); test("the only previous component was auto-answered", () => { - setState({ - breadcrumbs: { - Question: { - auto: true, - answers: ["NoFeeAnswerPath"], - }, - }, + record("Question", { + auto: true, + answers: ["NoFeeAnswerPath"], }); - expect(getState().canGoBack(getState().currentCard())).toStrictEqual(false); + + expect(canGoBack(getCurrentCard())).toStrictEqual(false); }); test("the applicant made a payment", () => { - setState({ - breadcrumbs: { - Question: { - auto: false, - answers: ["FeeAnswerPath"], - }, - Calculate: { - auto: true, - data: { - fee: 10, - }, - }, - Pay: { - auto: false, - }, + record("Question", { + auto: false, + answers: ["NoFeeAnswerPath"], + }); + record("Calculate", { + auto: true, + data: { + fee: 10, }, }); - expect(getState().canGoBack(getState().currentCard())).toStrictEqual(false); + record("Pay", { auto: false }); + + expect(canGoBack(getCurrentCard())).toStrictEqual(false); }); test("changing a component's answer", () => { - setState({ - breadcrumbs: { - Question: { - auto: false, - answers: ["FeeAnswerPath"], - }, - Calculate: { - auto: true, - data: { - fee: 10, - }, - }, - Pay: { - auto: false, - }, + record("Question", { + auto: false, + answers: ["FeeAnswerPath"], + }); + record("Calculate", { + auto: true, + data: { + fee: 10, }, - changedNode: "Confirmation", }); - expect(getState().canGoBack(getState().currentCard())).toStrictEqual(false); + record("Pay", { auto: false }); + changeAnswer("Confirmation"); + + expect(canGoBack(getCurrentCard())).toStrictEqual(false); }); }); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/overrideAnswer.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/overrideAnswer.test.ts index e0f53e3992..c7bcb0da1b 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/overrideAnswer.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/overrideAnswer.test.ts @@ -2,7 +2,7 @@ import { vanillaStore } from "../../store"; const { getState, setState } = vanillaStore; -const { overrideAnswer, currentCard, upcomingCardIds, record } = getState(); +const { overrideAnswer, getCurrentCard, upcomingCardIds, record } = getState(); test("it clears the correct breadcrumb and navigates back to the right node", async () => { // set up initial state, confirm our passport computes as expected @@ -38,7 +38,7 @@ test("it clears the correct breadcrumb and navigates back to the right node", as }); // confirm we've navigated back to the right node, and that PropertyInformation is queued up again in upcoming cards - expect(currentCard()?.id).toEqual("FirstPropertyTypeQuestionNodeId"); + expect(getCurrentCard()?.id).toEqual("FirstPropertyTypeQuestionNodeId"); expect(upcomingCardIds()).toContain("PropertyInformationNodeId"); // select a new answer, confirm our passport has updated diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/previousCard.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/previousCard.test.ts index 5ccedeeb2b..92ccc71a37 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/previousCard.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/previousCard.test.ts @@ -2,7 +2,7 @@ import { vanillaStore } from "../../store"; const { getState, setState } = vanillaStore; -const { resetPreview, previousCard, currentCard } = getState(); +const { resetPreview, previousCard, getCurrentCard } = getState(); beforeEach(() => { resetPreview(); @@ -24,7 +24,7 @@ const setup = (args = {}) => describe("store.previousCard is", () => { test("undefined when there are no breadcrumbs", () => { setup(); - expect(previousCard(currentCard())).toBeUndefined(); + expect(previousCard(getCurrentCard())).toBeUndefined(); }); test("undefined when cards were automatically answered", () => { @@ -34,7 +34,7 @@ describe("store.previousCard is", () => { b: { auto: true }, }, }); - expect(previousCard(currentCard())).toBeUndefined(); + expect(previousCard(getCurrentCard())).toBeUndefined(); }); test("the most recent human-answered card id", () => { @@ -45,6 +45,6 @@ describe("store.previousCard is", () => { }, }); - expect(previousCard(currentCard())).toEqual("a"); + expect(previousCard(getCurrentCard())).toEqual("a"); }); }); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts index 092553572a..dcf8e20d8b 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/removeNodesDependentOnPassport.test.ts @@ -15,8 +15,14 @@ let flowWithPassportComponents = cloneDeep( flowWithPassportComponentsMock, ) as Store.flow; -const { record, resetPreview, previousCard, currentCard, changeAnswer } = - getState(); +const { + record, + resetPreview, + previousCard, + getCurrentCard, + changeAnswer, + setCurrentCard, +} = getState(); beforeEach(() => { resetPreview(); @@ -61,7 +67,7 @@ describe("removeNodesDependentOnPassport", () => { }); describe("nodesDependentOnPassport with record", () => { - test("should remove Draw Boundary and Planning contraints from cachedBreadcrumbs", () => { + test("should remove Draw Boundary and Planning constraints from cachedBreadcrumbs", () => { const cachedBreadcrumbs = { ...breadcrumbsDependentOnPassport, } as Store.cachedBreadcrumbs; @@ -182,7 +188,7 @@ describe("nodesDependentOnPassport with record", () => { }, }); - expect(currentCard()?.id).toEqual("drawBoundary"); + expect(getCurrentCard()?.id).toEqual("drawBoundary"); }); test("should clear _nodesPendingEdit after edition", () => { @@ -229,8 +235,11 @@ describe("nodesDependentOnPassport with previousCard", () => { _nodesPendingEdit: [], }); - expect(currentCard()?.id).toEqual("drawBoundary"); - expect(previousCard(currentCard())).toEqual("text"); + // Manually call setCurrentCard() as we're not using record() as part of our setup + setCurrentCard(); + + expect(getCurrentCard()?.id).toEqual("drawBoundary"); + expect(previousCard(getCurrentCard())).toEqual("text"); }); test("To be last pushed to the breadcrumbs when changing answer", () => { @@ -247,7 +256,7 @@ describe("nodesDependentOnPassport with previousCard", () => { _nodesPendingEdit, }); - expect(previousCard(currentCard())).toEqual("findProperty"); + expect(previousCard(getCurrentCard())).toEqual("findProperty"); }); }); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/upcomingCardIds.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/upcomingCardIds.test.ts index a72b347792..6c3a1f5e05 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/upcomingCardIds.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/preview/upcomingCardIds.test.ts @@ -3,7 +3,7 @@ import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { Store, vanillaStore } from "../../store"; const { getState, setState } = vanillaStore; -const { upcomingCardIds, resetPreview, record, currentCard } = getState(); +const { upcomingCardIds, resetPreview, record, getCurrentCard } = getState(); const flow: Store.flow = { _root: { @@ -75,7 +75,7 @@ test.skip("A node is only auto-answered when it is the first upcomingCardId(), n record("SetValue", { data: { fruit: ["apple"] }, auto: true }); clickContinue(); - expect(currentCard()?.id).toBe("Content"); + expect(getCurrentCard()?.id).toBe("Content"); // "AutomatedQuestion" should still be queued up, not already answered based on SetValue expect(visitedNodes()).not.toContain("AutomatedQuestion"); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/setValue.test.ts b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/setValue.test.ts index 09052ced51..72983ec6fd 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/setValue.test.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/__tests__/setValue.test.ts @@ -4,7 +4,7 @@ import { cloneDeep, merge } from "lodash"; import { Store, vanillaStore } from "../store"; const { getState, setState } = vanillaStore; -const { resetPreview, record, computePassport, currentCard } = getState(); +const { resetPreview, record, computePassport, getCurrentCard } = getState(); const baseFlow: Store.flow = { _root: { @@ -76,7 +76,7 @@ describe("SetValue component", () => { expect(breadcrumbKeys).toContain("setValue1"); // Middle of flow reached - expect(currentCard()?.id).toEqual("middleOfService"); + expect(getCurrentCard()?.id).toEqual("middleOfService"); // Passport correctly populated expect(computePassport()?.data?.myKey).toHaveLength(1); @@ -94,7 +94,7 @@ describe("SetValue component", () => { expect(breadcrumbKeys).toContain("setValue2"); // End of flow reached - expect(currentCard()?.id).toEqual("endOfService"); + expect(getCurrentCard()?.id).toEqual("endOfService"); // Passport correctly populated expect(computePassport()?.data?.myKey).toHaveLength(1); @@ -133,7 +133,7 @@ describe("SetValue component", () => { expect(breadcrumbKeys).toContain("setValue1"); // Middle of flow reached - expect(currentCard()?.id).toEqual("middleOfService"); + expect(getCurrentCard()?.id).toEqual("middleOfService"); // Passport correctly populated expect(computePassport()?.data?.myKey).toHaveLength(1); @@ -155,7 +155,7 @@ describe("SetValue component", () => { expect(breadcrumbKeys).toContain("setValue2"); // End of flow reached - expect(currentCard()?.id).toEqual("endOfService"); + expect(getCurrentCard()?.id).toEqual("endOfService"); // Passport correctly populated expect(computePassport()?.data?.myKey).toHaveLength(2); @@ -212,7 +212,7 @@ describe("SetValue component", () => { expect(breadcrumbKeys).toContain("setValue1"); // Middle of flow reached - expect(currentCard()?.id).toEqual("middleOfService"); + expect(getCurrentCard()?.id).toEqual("middleOfService"); // Passport correctly populated - value not present expect(computePassport()?.data?.myKey).toBeUndefined(); @@ -236,7 +236,7 @@ describe("SetValue component", () => { record("setValue3", { data: { myKey: ["mySecondValue"] } }); // End of flow reached - expect(currentCard()?.id).toEqual("endOfService"); + expect(getCurrentCard()?.id).toEqual("endOfService"); // Passport correctly populated - value no longer set expect(computePassport()?.data?.myKey).toBeUndefined(); @@ -265,7 +265,7 @@ describe("SetValue component", () => { record("setValue3", { data: { myKey: ["myUnsetValue"] } }); // End of flow reached - expect(currentCard()?.id).toEqual("endOfService"); + expect(getCurrentCard()?.id).toEqual("endOfService"); // Passport correctly populated - passport variable not removed as values do not match expect(computePassport()?.data?.myKey).toEqual("mySecondValue"); @@ -320,7 +320,7 @@ describe("SetValue component", () => { expect(breadcrumbKeys).toContain("setValue1"); // Middle of flow reached - expect(currentCard()?.id).toEqual("middleOfService"); + expect(getCurrentCard()?.id).toEqual("middleOfService"); // Passport correctly populated - value not present expect(computePassport()?.data?.myKey).toBeUndefined(); @@ -344,7 +344,7 @@ describe("SetValue component", () => { record("setValue3", { data: { myKey: ["mySecondValue"] } }); // End of flow reached - expect(currentCard()?.id).toEqual("endOfService"); + expect(getCurrentCard()?.id).toEqual("endOfService"); // Passport correctly populated - key:value pair removed expect(computePassport()?.data).not.toHaveProperty("myKey"); diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/analytics/provider.tsx b/editor.planx.uk/src/pages/FlowEditor/lib/analytics/provider.tsx index 5ba9bbf4ed..b4090bef66 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/analytics/provider.tsx +++ b/editor.planx.uk/src/pages/FlowEditor/lib/analytics/provider.tsx @@ -51,7 +51,6 @@ let lastVisibleNodeAnalyticsLogId: number | undefined = undefined; const analyticsContext = createContext<{ createAnalytics: (type: AnalyticsType) => Promise; trackEvent: (eventData: EventData) => Promise; - node: Store.node | null; track: ( nodeId: string, direction?: AnalyticsLogDirection, @@ -60,7 +59,6 @@ const analyticsContext = createContext<{ }>({ createAnalytics: () => Promise.resolve(), trackEvent: () => Promise.resolve(), - node: null, track: () => Promise.resolve(), }); const { Provider } = analyticsContext; @@ -90,7 +88,6 @@ export const AnalyticsProvider: React.FC<{ children: React.ReactNode }> = ({ state.id, state.flow, ]); - const node = currentCard(); const isAnalyticsEnabled = new URL(window.location.href).searchParams.get("analytics") !== "false"; const shouldTrackAnalytics = @@ -138,7 +135,6 @@ export const AnalyticsProvider: React.FC<{ children: React.ReactNode }> = ({ = ({ }); const id = response.data.insert_analytics_one.id; setAnalyticsId(id); - const currentNodeId = currentCard()?.id; + const currentNodeId = currentCard?.id; if (currentNodeId) track(currentNodeId, type, id); } diff --git a/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts b/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts index be77eeeb9b..f17ba45b38 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts @@ -39,7 +39,9 @@ export interface PreviewStore extends Store.Store { upToNodeId: Store.nodeId, visited?: Array, ) => Array; - currentCard: () => Store.node | null; + currentCard: ({ id: Store.nodeId } & Store.node) | null; + setCurrentCard: () => void; + getCurrentCard: () => ({ id: Store.nodeId } & Store.node) | null; hasPaid: () => boolean; previousCard: ( node: Store.node | null, @@ -136,18 +138,15 @@ export const previewStore: StateCreator< return res; }, - currentCard() { + setCurrentCard() { const { upcomingCardIds, flow } = get(); const upcoming = upcomingCardIds(); if (upcoming.length > 0) { const id = upcoming[0]; - return { - id, - ...flow[id], - }; + set({ currentCard: { id, ...flow[id] } }); } else { - return null; + set({ currentCard: null }); } }, @@ -287,6 +286,7 @@ export const previewStore: StateCreator< _nodesPendingEdit, changedNode, updateSectionData, + setCurrentCard, } = get(); if (!flow[id]) throw new Error(`id "${id}" not found`); @@ -369,6 +369,7 @@ export const previewStore: StateCreator< }); } } + setCurrentCard(); updateSectionData(); }, @@ -379,6 +380,7 @@ export const previewStore: StateCreator< resumeSession(session: Session) { set({ ...session }); + get().setCurrentCard(); get().updateSectionData(); }, @@ -652,6 +654,10 @@ export const previewStore: StateCreator< return currentRequestedFiles || emptyFileList; }, + + currentCard: null, + + getCurrentCard: () => get().currentCard, }); const knownNots = ( diff --git a/editor.planx.uk/src/pages/Preview/Questions.tsx b/editor.planx.uk/src/pages/Preview/Questions.tsx index 194a5f7edf..ac1f400730 100644 --- a/editor.planx.uk/src/pages/Preview/Questions.tsx +++ b/editor.planx.uk/src/pages/Preview/Questions.tsx @@ -63,6 +63,8 @@ const Questions = ({ previewEnvironment }: QuestionsProps) => { canGoBack, setPreviewEnvironment, getType, + node, + setCurrentCard, ] = useStore((state) => [ state.previousCard, state.record, @@ -75,9 +77,11 @@ const Questions = ({ previewEnvironment }: QuestionsProps) => { state.canGoBack, state.setPreviewEnvironment, state.getType, + state.currentCard, + state.setCurrentCard, ]); const isStandalone = previewEnvironment === "standalone"; - const { createAnalytics, node, trackEvent } = useAnalyticsTracking(); + const { createAnalytics, trackEvent } = useAnalyticsTracking(); const [gotFlow, setGotFlow] = useState(false); const isUsingLocalStorage = useStore((state) => state.path) === ApplicationPath.SingleSession; @@ -89,6 +93,8 @@ const Questions = ({ previewEnvironment }: QuestionsProps) => { // Initial setup useEffect(() => { + setCurrentCard(); + if (!isStandalone) return; if (isUsingLocalStorage) { diff --git a/editor.planx.uk/src/pages/Preview/ReconciliationPage.tsx b/editor.planx.uk/src/pages/Preview/ReconciliationPage.tsx index 75d13added..2e52f43042 100644 --- a/editor.planx.uk/src/pages/Preview/ReconciliationPage.tsx +++ b/editor.planx.uk/src/pages/Preview/ReconciliationPage.tsx @@ -38,7 +38,7 @@ const ReconciliationPage: React.FC = ({ state.flow, state.hasSections, state.sectionNodes, - state.currentCard(), + state.currentCard, state.changeAnswer, state.record, state.computePassport(),