Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: conditionally show/hide "Continue" button on Confirmation using isFinalCard #4029

Merged
merged 7 commits into from
Dec 3, 2024
Merged
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
89 changes: 89 additions & 0 deletions editor.planx.uk/src/@planx/components/Confirmation/Public.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
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";
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(() => ({
Expand Down Expand Up @@ -33,3 +40,85 @@ it("should not have any accessibility violations", async () => {
const results = await axe(container);
expect(results).toHaveNoViolations();
});

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);

const { user } = setup(
<ConfirmationComponent
color={{ text: "#000", background: "rgba(1, 99, 96, 0.1)" }}
heading="heading"
description="description"
nextSteps={[
{ title: "title1", description: "description1" },
{ title: "title2", description: "description2" },
{ title: "title3", description: "description3" },
]}
moreInfo="more info"
contactInfo="contact info"
handleSubmit={handleSubmit}
/>,
);

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(
<ConfirmationComponent
color={{ text: "#000", background: "rgba(1, 99, 96, 0.1)" }}
heading="heading"
description="description"
nextSteps={[
{ title: "title1", description: "description1" },
{ title: "title2", description: "description2" },
{ title: "title3", description: "description3" },
]}
moreInfo="more info"
contactInfo="contact info"
handleSubmit={handleSubmit}
/>,
);

expect(screen.queryByText("Continue")).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ interface PresentationalProps extends Props {
}

export function Presentational(props: PresentationalProps) {
const isFinalCard = useStore().isFinalCard();
return (
<Box width="100%">
<Banner
Expand All @@ -99,7 +100,7 @@ export function Presentational(props: PresentationalProps) {
</Box>
)}
</Banner>
<Card>
<Card handleSubmit={isFinalCard ? undefined : props.handleSubmit}>
<SummaryListTable>
{Object.entries(props.applicableDetails).map(([k, v], i) => (
<React.Fragment key={`detail-${i}`}>
Expand Down
24 changes: 24 additions & 0 deletions editor.planx.uk/src/@planx/components/shared/Preview/Card.test.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 = <h1>Confirmation Page</h1>;
setup(<Card handleSubmit={handleSubmit} children={children}></Card>);

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 = <Button>Testing 123</Button>;
Expand Down
14 changes: 12 additions & 2 deletions editor.planx.uk/src/@planx/components/shared/Preview/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -48,12 +49,21 @@ const Card: React.FC<Props> = ({
...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
// 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;
path === ApplicationPath.SaveAndReturn && handleSubmit && !hasSent;
const { track } = useAnalyticsTracking();

useEffect(() => {
Expand Down
5 changes: 2 additions & 3 deletions editor.planx.uk/src/pages/FlowEditor/lib/store/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member Author

@jessicamcinchak jessicamcinchak Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one's been a long time coming (original commit) & more promising to re-introduce now since upcomingCardIds has been recently refactored to only be responsible for navigation and no longer optimistic auto-answering

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very happy to see this commented back in 👍

},

restore: false,
Expand Down
17 changes: 7 additions & 10 deletions editor.planx.uk/src/pages/Preview/Node.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,13 @@ interface Props {
}

const Node: React.FC<Props> = (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;
Copy link
Member Author

@jessicamcinchak jessicamcinchak Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, in the near future, we can/will re-introduce isFinalCard at the Preview/Node level, but we'll want to do more thorough performance reviews and add test coverage before doing this - so have adjusted this to keep current behavior in the meantime and instead only reference isFinalCard within the Confirmation component itself rather than any component (which should be significantly less expensive / less risky to introduce slugishness)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree - this is the right approach here 👍


const nodeId = props.node.id;
const previouslySubmittedData =
Expand Down
Loading