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.

Sign up for GitHub

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

fix: remove defaultText prop from disclaimer component #3990

Merged
merged 3 commits into from
Nov 20, 2024
Merged

fix: remove defaultText prop from disclaimer component #3990

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
59d2cf2
Remove defaultText prop from disclaimer component
jamdelion Nov 20, 2024
38cbb4f
Remove disclaimer prop from test case
jamdelion Nov 20, 2024
dddb657
Fix ts-ignore
jamdelion Nov 20, 2024
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
12 changes: 5 additions & 7 deletions editor.planx.uk/src/@planx/components/PlanningConstraints/Presentational.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export type PresentationalProps = {
refreshConstraints: () => void;
inaccurateConstraints: InaccurateConstraints;
setInaccurateConstraints: (
value: React.SetStateAction<InaccurateConstraints>,
value: React.SetStateAction<InaccurateConstraints>
) => void;
};

Expand All @@ -44,10 +44,10 @@ export function Presentational(props: PresentationalProps) {
if (showError) return <ConstraintsFetchError error={error} {...props} />;

const positiveConstraints = Object.values(constraints).filter(
(v: Constraint) => v.text && v.value,
(v: Constraint) => v.text && v.value
);
const negativeConstraints = Object.values(constraints).filter(
(v: Constraint) => v.text && !v.value,
(v: Constraint) => v.text && !v.value
);

return (
Expand Down Expand Up @@ -82,8 +82,7 @@ export function Presentational(props: PresentationalProps) {
</SimpleExpand>
)}
<Disclaimer
text={disclaimer}
defaultText={DEFAULT_PLANNING_CONDITIONS_DISCLAIMER}
text={disclaimer || DEFAULT_PLANNING_CONDITIONS_DISCLAIMER}
/>
</>
)}
Expand Down Expand Up @@ -115,8 +114,7 @@ export function Presentational(props: PresentationalProps) {
/>
</SimpleExpand>
<Disclaimer
text={disclaimer}
defaultText={DEFAULT_PLANNING_CONDITIONS_DISCLAIMER}
text={disclaimer || DEFAULT_PLANNING_CONDITIONS_DISCLAIMER}
/>
</>
)}
Expand Down
48 changes: 24 additions & 24 deletions editor.planx.uk/src/@planx/components/PlanningConstraints/Public.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ const swrMock = (swr as jest.Mock).mock;
vi.mock("swr", () => ({
default: vi.fn((url: () => string) => {
const isGISRequest = url()?.startsWith(
`${import.meta.env.VITE_APP_API_URL}/gis`,
`${import.meta.env.VITE_APP_API_URL}/gis`
);
const isRoadsRequest = url()?.startsWith(
`${import.meta.env.VITE_APP_API_URL}/roads`,
`${import.meta.env.VITE_APP_API_URL}/roads`
);

if (isGISRequest) return { data: digitalLandResponseMock };
Expand All @@ -50,7 +50,7 @@ describe("error state", () => {
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>
</ErrorBoundary>,
</ErrorBoundary>
);

expect(getByTestId("error-summary-invalid-graph")).toBeInTheDocument();
Expand All @@ -66,7 +66,7 @@ describe("error state", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
/>
</ErrorBoundary>,
</ErrorBoundary>
);
const results = await axe(container);
expect(results).toHaveNoViolations();
Expand All @@ -82,7 +82,7 @@ describe("following a FindProperty component", () => {
teamIntegrations: {
hasPlanningData: true,
},
}),
})
);
});

Expand All @@ -96,11 +96,11 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={handleSubmit}
/>,
/>
);

expect(
getByRole("heading", { name: "Planning constraints" }),
getByRole("heading", { name: "Planning constraints" })
).toBeInTheDocument();

await user.click(getByTestId("continue-button"));
Expand All @@ -115,7 +115,7 @@ describe("following a FindProperty component", () => {
description="Things that might affect your project"
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
/>,
/>
);
const results = await axe(container);
expect(results).toHaveNoViolations();
Expand All @@ -129,7 +129,7 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>,
/>
);

expect(swr).toHaveBeenCalled();
Expand All @@ -150,7 +150,7 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>,
/>
);

expect(swr).toHaveBeenCalled();
Expand All @@ -171,7 +171,7 @@ describe("following a FindProperty component", () => {
teamIntegrations: {
hasPlanningData: true,
},
}),
})
);

setup(
Expand All @@ -181,7 +181,7 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>,
/>
);

expect(swr).toHaveBeenCalled();
Expand Down Expand Up @@ -211,12 +211,12 @@ describe("following a FindProperty component", () => {
fn="property.constraints.planning"
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={vi.fn()}
/>,
/>
);

// Positive constraints visible by default
expect(
getByRole("heading", { name: /These are the planning constraints/ }),
getByRole("heading", { name: /These are the planning constraints/ })
).toBeVisible();
expect(getByRole("button", { name: /Parks and gardens/ })).toBeVisible();

Expand All @@ -227,7 +227,7 @@ describe("following a FindProperty component", () => {
expect(showNegativeConstraintsButton).toBeVisible();

const negativeConstraintsContainer = getByTestId(
"negative-constraints-list",
"negative-constraints-list"
);
expect(negativeConstraintsContainer).not.toBeVisible();

Expand All @@ -241,19 +241,19 @@ describe("following a FindProperty component", () => {
});

test("default disclaimer text should render if none provided", async () => {
// @ts-ignore - we deliberately want to test the case where PlanningConstraints is missing the disclaimer prop
const { queryByText } = setup(
<PlanningConstraints
title="Planning constraints"
description="Things that might affect your project"
fn="property.constraints.planning"
disclaimer=""
handleSubmit={vi.fn()}
/>,
/>
Copy link
Contributor Author

@jamdelion jamdelion Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry lots of linting has happened here, this is the only bit I actually changed in this file

);
expect(
queryByText(
"This page does not include information about historic planning conditions that may apply to this property.",
),
"This page does not include information about historic planning conditions that may apply to this property."
)
).toBeVisible();
});
});
Expand All @@ -268,7 +268,7 @@ describe("demo state", () => {
hasPlanningData: false,
},
teamSlug: "demo",
}),
})
);
});
it("should render an error when teamSlug is demo", async () => {
Expand All @@ -282,21 +282,21 @@ describe("demo state", () => {
disclaimer="This page does not include information about historic planning conditions that may apply to this property."
handleSubmit={handleSubmit}
/>
</ErrorBoundary>,
</ErrorBoundary>
);

const errorMessage = queryByText(
"Planning Constraints are not enabled for demo users",
"Planning Constraints are not enabled for demo users"
);
expect(errorMessage).toBeVisible();

// Check planning constraints has not rendered
// reused positive constraints from basic layout test
expect(
queryByRole("heading", { name: /These are the planning constraints/ }),
queryByRole("heading", { name: /These are the planning constraints/ })
).not.toBeInTheDocument();
expect(
queryByRole("button", { name: /Parks and gardens/ }),
queryByRole("button", { name: /Parks and gardens/ })
).not.toBeInTheDocument();

// Ensure a demo user can continue on in the application
Expand Down
10 changes: 2 additions & 8 deletions editor.planx.uk/src/@planx/components/shared/Disclaimer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,7 @@ import { WarningContainer } from "@planx/components/shared/Preview/WarningContai
import React from "react";
import ReactMarkdownOrHtml from "ui/shared/ReactMarkdownOrHtml/ReactMarkdownOrHtml";

export const Disclaimer = ({
text,
defaultText,
}: {
text: string;
defaultText?: string;
}) => {
export const Disclaimer = ({ text }: { text: string }) => {
return (
<WarningContainer>
<ErrorOutline />
Expand All @@ -20,7 +14,7 @@ export const Disclaimer = ({
ml={2}
sx={{ "& p:first-of-type": { marginTop: 0 } }}
>
<ReactMarkdownOrHtml source={text || defaultText} openLinksOnNewTab />
<ReactMarkdownOrHtml source={text} openLinksOnNewTab />
</Typography>
</WarningContainer>
);
Expand Down
Loading