From a5ce53939267eecace2e9b4b7e709391ce2fec33 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Mon, 2 Dec 2024 17:57:57 +0100 Subject: [PATCH 1/7] conditionally show/hide continue button on Confirmation component using isFinalCard --- .../@planx/components/Confirmation/Public.tsx | 3 ++- .../src/pages/FlowEditor/lib/store/preview.ts | 5 ++--- editor.planx.uk/src/pages/Preview/Node.tsx | 17 +++++++---------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/Confirmation/Public.tsx b/editor.planx.uk/src/@planx/components/Confirmation/Public.tsx index f6cd94fd2c..661d663f4f 100644 --- a/editor.planx.uk/src/@planx/components/Confirmation/Public.tsx +++ b/editor.planx.uk/src/@planx/components/Confirmation/Public.tsx @@ -85,6 +85,7 @@ interface PresentationalProps extends Props { } export function Presentational(props: PresentationalProps) { + const isFinalCard = useStore().isFinalCard(); return ( )} - + {Object.entries(props.applicableDetails).map(([k, v], i) => ( 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 ef3a4f164c..a0f502d524 100644 --- a/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts +++ b/editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts @@ -670,9 +670,8 @@ export const previewStore: StateCreator< }, isFinalCard: () => { - // Temporarily always returns false until upcomingCardIds is optimised - // OSL Slack explanation: https://bit.ly/3x38IRY - return false; + const { upcomingCardIds } = get(); + return upcomingCardIds().length === 1; }, restore: false, diff --git a/editor.planx.uk/src/pages/Preview/Node.tsx b/editor.planx.uk/src/pages/Preview/Node.tsx index caf83b6c6b..c61cfdbe3b 100644 --- a/editor.planx.uk/src/pages/Preview/Node.tsx +++ b/editor.planx.uk/src/pages/Preview/Node.tsx @@ -75,16 +75,13 @@ interface Props { } const Node: React.FC = (props) => { - const [childNodesOf, isFinalCard, resetPreview, cachedBreadcrumbs] = useStore( - (state) => [ - state.childNodesOf, - state.isFinalCard(), - state.resetPreview, - state.cachedBreadcrumbs, - ], - ); - - const handleSubmit = isFinalCard ? undefined : props.handleSubmit; + const [childNodesOf, resetPreview, cachedBreadcrumbs] = useStore((state) => [ + state.childNodesOf, + state.resetPreview, + state.cachedBreadcrumbs, + ]); + + const handleSubmit = props.handleSubmit; const nodeId = props.node.id; const previouslySubmittedData = From 62bcbc6ca822a8bd3e2ee506587a0f5d3cdeb823 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 3 Dec 2024 08:44:35 +0100 Subject: [PATCH 2/7] never show SaveAndReturnButton on Confirmation nodes --- .../src/@planx/components/shared/Preview/Card.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 72c8e261af..dca2e20744 100644 --- a/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx @@ -3,6 +3,7 @@ import Button from "@mui/material/Button"; import Container from "@mui/material/Container"; import Fade from "@mui/material/Fade"; import { styled, Theme, useTheme } from "@mui/material/styles"; +import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { useAnalyticsTracking } from "pages/FlowEditor/lib/analytics/provider"; import { useStore } from "pages/FlowEditor/lib/store"; import React, { useEffect } from "react"; @@ -52,8 +53,11 @@ const Card: React.FC = ({ state.path, state.currentCard, ]); + const showSaveResumeButton = - path === ApplicationPath.SaveAndReturn && handleSubmit; + path === ApplicationPath.SaveAndReturn && + handleSubmit && + visibleNode?.type !== TYPES.Confirmation; const { track } = useAnalyticsTracking(); useEffect(() => { From 6f4fcf15e1ec7972fffd8820af689530bf9cb93e Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 3 Dec 2024 09:15:16 +0100 Subject: [PATCH 3/7] hide SaveAndReturnButton if breadcrumbs have Send --- .../@planx/components/shared/Preview/Card.tsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) 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 dca2e20744..963d5aa7c8 100644 --- a/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx @@ -49,15 +49,23 @@ const Card: React.FC = ({ ...props }) => { const theme = useTheme(); - const [path, visibleNode] = useStore((state) => [ + const [path, visibleNode, breadcrumbs, flow] = useStore((state) => [ state.path, state.currentCard, + state.breadcrumbs, + state.flow, ]); + // Check if we have a Send node in our breadcrumbs + // In the frontend this is a better/more immediate proxy for "Submitted" because actual send events that populate lowcal_sessions.submitted_at are async + const hasSent = Object.keys(breadcrumbs) + .reverse() + .some( + (breadcrumbNodeId: string) => flow[breadcrumbNodeId]?.type === TYPES.Send, + ); + const showSaveResumeButton = - path === ApplicationPath.SaveAndReturn && - handleSubmit && - visibleNode?.type !== TYPES.Confirmation; + path === ApplicationPath.SaveAndReturn && handleSubmit && !hasSent; const { track } = useAnalyticsTracking(); useEffect(() => { From 9afebe9d116823677661a8a2a927dc4621aa031b Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 3 Dec 2024 09:28:04 +0100 Subject: [PATCH 4/7] clearer code comment --- editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 963d5aa7c8..d49ad2c13a 100644 --- a/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx @@ -57,7 +57,7 @@ const Card: React.FC = ({ ]); // Check if we have a Send node in our breadcrumbs - // In the frontend this is a better/more immediate proxy for "Submitted" because actual send events that populate lowcal_sessions.submitted_at are async + // This is a better/more immediate proxy for "Submitted" because actual send events that populate lowcal_sessions.submitted_at are queued via Hasura const hasSent = Object.keys(breadcrumbs) .reverse() .some( From 5f44a70c9713109801fac756905d78f829dbf65a Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 3 Dec 2024 11:36:21 +0100 Subject: [PATCH 5/7] don't reverse(), add tests --- .../components/Confirmation/Public.test.tsx | 6 +++++ .../components/shared/Preview/Card.test.tsx | 24 +++++++++++++++++++ .../@planx/components/shared/Preview/Card.tsx | 10 ++++---- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx b/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx index 5d98b00970..6439b92a99 100644 --- a/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx @@ -33,3 +33,9 @@ it("should not have any accessibility violations", async () => { const results = await axe(container); expect(results).toHaveNoViolations(); }); + +it.todo("should hide the 'Continue' button if it's the final card in the flow"); + +it.todo( + "should show the 'Continue' button if it is not the final card in the flow", +); diff --git a/editor.planx.uk/src/@planx/components/shared/Preview/Card.test.tsx b/editor.planx.uk/src/@planx/components/shared/Preview/Card.test.tsx index 949d8abe02..b7c40da8e8 100644 --- a/editor.planx.uk/src/@planx/components/shared/Preview/Card.test.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Preview/Card.test.tsx @@ -1,4 +1,5 @@ import Button from "@mui/material/Button"; +import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; import { act, screen, waitFor } from "@testing-library/react"; import { FullStore, useStore } from "pages/FlowEditor/lib/store"; import React from "react"; @@ -50,6 +51,29 @@ describe("Card component", () => { expect(screen.queryByText(saveButtonText)).not.toBeInTheDocument(); }); + it("hides the Save/Resume option if the user has already passed Send", () => { + act(() => + setState({ + path: ApplicationPath.SaveAndReturn, + flow: { + _root: { edges: ["Send", "Confirmation", "Feedback", "Notice"] }, + Send: { type: TYPES.Send }, + Confirmation: { type: TYPES.Confirmation }, + Feedback: { type: TYPES.Feedback }, + Notice: { type: TYPES.Notice }, + }, + breadcrumbs: { Send: { auto: false } }, + }), + ); + const children =

Confirmation Page

; + setup(); + + expect(screen.queryByText("Confirmation Page")).toBeInTheDocument(); + expect(screen.queryByText("Continue")).toBeInTheDocument(); + expect(screen.queryByText(resumeButtonText)).not.toBeInTheDocument(); + expect(screen.queryByText(saveButtonText)).not.toBeInTheDocument(); + }); + it("updates state to navigate to the 'Resume' page if the 'Resume' button is clicked", async () => { act(() => setState({ path: ApplicationPath.SaveAndReturn })); const children = ; 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 d49ad2c13a..de4372a8b8 100644 --- a/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx +++ b/editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx @@ -57,12 +57,10 @@ const Card: React.FC = ({ ]); // Check if we have a Send node in our breadcrumbs - // This is a better/more immediate proxy for "Submitted" because actual send events that populate lowcal_sessions.submitted_at are queued via Hasura - const hasSent = Object.keys(breadcrumbs) - .reverse() - .some( - (breadcrumbNodeId: string) => flow[breadcrumbNodeId]?.type === TYPES.Send, - ); + // This is a better/more immediate proxy for "submitted" in the frontend because actual send events that populate lowcal_sessions.submitted_at are queued via Hasura + const hasSent = Object.keys(breadcrumbs).some( + (breadcrumbNodeId: string) => flow[breadcrumbNodeId]?.type === TYPES.Send, + ); const showSaveResumeButton = path === ApplicationPath.SaveAndReturn && handleSubmit && !hasSent; From ca4e7149c429a1cb490c9a36eeeb6e831f38cb3c Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 3 Dec 2024 11:51:54 +0100 Subject: [PATCH 6/7] more tests --- .../components/Confirmation/Public.test.tsx | 90 ++++++++++++++++++- 1 file changed, 86 insertions(+), 4 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx b/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx index 6439b92a99..62b37f0717 100644 --- a/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx @@ -1,3 +1,6 @@ +import { ComponentType as TYPES } from "@opensystemslab/planx-core/types"; +import { act, screen, waitFor } from "@testing-library/react"; +import { FullStore, useStore } from "pages/FlowEditor/lib/store"; import React from "react"; import { setup } from "testUtils"; import { vi } from "vitest"; @@ -5,6 +8,10 @@ import { axe } from "vitest-axe"; import ConfirmationComponent from "./Public"; +const { getState, setState } = useStore; + +let initialState: FullStore; + vi.mock("@opensystemslab/planx-core", () => { return { CoreDomainClient: vi.fn().mockImplementation(() => ({ @@ -34,8 +41,83 @@ it("should not have any accessibility violations", async () => { expect(results).toHaveNoViolations(); }); -it.todo("should hide the 'Continue' button if it's the final card in the flow"); +describe("Confirmation component", () => { + const handleSubmit = vi.fn(); + + beforeAll(() => (initialState = getState())); + + afterEach(() => waitFor(() => setState(initialState))); + + it("hides the 'Continue' button if it's the final card in the flow", () => { + act(() => + setState({ + flow: { + _root: { edges: ["Send", "Confirmation"] }, + Send: { type: TYPES.Send }, + Confirmation: { type: TYPES.Confirmation }, + }, + breadcrumbs: { Send: { auto: false } }, + }), + ); + + expect(getState().upcomingCardIds()).toEqual(["Confirmation"]); + expect(getState().isFinalCard()).toEqual(true); -it.todo( - "should show the 'Continue' button if it is not the final card in the flow", -); + const { user } = setup( + , + ); + + expect(screen.queryByText("Continue")).not.toBeInTheDocument(); + }); + + it("shows the 'Continue' button if there are nodes following it", () => { + act(() => + setState({ + flow: { + _root: { edges: ["Send", "Confirmation", "Feedback", "Notice"] }, + Send: { type: TYPES.Send }, + Confirmation: { type: TYPES.Confirmation }, + Feedback: { type: TYPES.Feedback }, + Notice: { type: TYPES.Notice }, + }, + breadcrumbs: { Send: { auto: false } }, + }), + ); + + expect(getState().upcomingCardIds()).toEqual([ + "Confirmation", + "Feedback", + "Notice", + ]); + expect(getState().isFinalCard()).toEqual(false); + + const { user } = setup( + , + ); + + expect(screen.queryByText("Continue")).toBeInTheDocument(); + }); +}); From 51a7ec3a879f2c54eb44f28d142e428e0a511209 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 3 Dec 2024 11:58:41 +0100 Subject: [PATCH 7/7] correct test syntax --- .../src/@planx/components/Confirmation/Public.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx b/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx index 62b37f0717..0fb6c1807d 100644 --- a/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx @@ -75,6 +75,7 @@ describe("Confirmation component", () => { ]} moreInfo="more info" contactInfo="contact info" + handleSubmit={handleSubmit} />, );