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(a11y): always enable continue button on FindProperty map page #3141

Merged
merged 1 commit into from
May 14, 2024
+161 −123
Merged

fix(a11y): always enable continue button on FindProperty map page #3141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
133 changes: 79 additions & 54 deletions editor.planx.uk/src/@planx/components/FindProperty/Public/Map.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import {
MapContainer,
MapFooter,
} from "@planx/components/shared/Preview/MapContainer";
import { GeoJSONObject } from "@turf/helpers";
import { useStore } from "pages/FlowEditor/lib/store";
import React, { useEffect, useState } from "react";
import React, { useEffect, useRef, useState } from "react";
import InputLabel from "ui/public/InputLabel";
import ErrorWrapper from "ui/shared/ErrorWrapper";
import Input from "ui/shared/Input";

import { DEFAULT_NEW_ADDRESS_LABEL, SiteAddress } from "../model";
Expand All @@ -23,10 +23,15 @@ interface PlotNewAddressProps {
setAddress: React.Dispatch<React.SetStateAction<SiteAddress | undefined>>;
setPage: React.Dispatch<React.SetStateAction<"os-address" | "new-address">>;
initialProposedAddress?: SiteAddress;
boundary?: GeoJSONObject | undefined;
id?: string;
description?: string;
descriptionLabel?: string;
mapValidationError?: string;
setMapValidationError: React.Dispatch<
React.SetStateAction<string | undefined>
>;
showSiteDescriptionError: boolean;
setShowSiteDescriptionError: React.Dispatch<React.SetStateAction<boolean>>;
}

type Coordinates = {
Expand Down Expand Up @@ -55,8 +60,6 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
const [siteDescription, setSiteDescription] = useState<string | null>(
props.initialProposedAddress?.title ?? null,
);
const [showSiteDescriptionError, setShowSiteDescriptionError] =
useState<boolean>(false);

const [environment, boundaryBBox] = useStore((state) => [
state.previewEnvironment,
Expand Down Expand Up @@ -85,6 +88,17 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
};
}, [setCoordinates]);

const mapValidationErrorRef = useRef(props.mapValidationError);
useEffect(() => {
mapValidationErrorRef.current = props.mapValidationError;
}, [props.mapValidationError]);

useEffect(() => {
if (mapValidationErrorRef.current) {
props.setMapValidationError(undefined);
}
}, [coordinates]);

useEffect(() => {
// when we have all required address parts, call setAddress to enable the "Continue" button
if (siteDescription && coordinates) {
Expand All @@ -99,7 +113,7 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
}, [coordinates, siteDescription]);

const handleSiteDescriptionCheck = () => {
if (!siteDescription) setShowSiteDescriptionError(true);
if (!siteDescription) props.setShowSiteDescriptionError(true);
};

const handleSiteDescriptionInputChange = (
Expand All @@ -111,54 +125,65 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {

return (
<>
<MapContainer environment={environment} size="large">
<p style={visuallyHidden}>
An interactive map centred on the local authority district, showing
the Ordnance Survey basemap. Click to place a point representing your
proposed site location.
</p>
{/* @ts-ignore */}
<my-map
id="plot-new-address-map"
ariaLabelOlFixedOverlay="An interactive map for providing your site location"
data-testid="map-web-component"
zoom={14}
drawMode
drawType="Point"
geojsonData={JSON.stringify(props?.boundary)}
geojsonColor="#efefef"
geojsonBuffer={10}
resetControlImage="trash"
showScale
showNorthArrow
osProxyEndpoint={`${process.env.REACT_APP_API_URL}/proxy/ordnance-survey`}
clipGeojsonData={JSON.stringify(boundaryBBox)}
collapseAttributions={window.innerWidth < 500 ? true : undefined}
/>
<MapFooter>
<ErrorWrapper error={props.mapValidationError} id="plot-new-address-map">
<MapContainer environment={environment} size="large">
<p style={visuallyHidden}>
An interactive map centred on the local authority district, showing
the Ordnance Survey basemap. Click to place a point representing
your proposed site location.
</p>
{/* @ts-ignore */}
<my-map
id="plot-new-address-map"
ariaLabelOlFixedOverlay="An interactive map for providing your site location"
data-testid="map-web-component"
zoom={14}
drawMode
drawType="Point"
drawGeojsonData={
coordinates &&
JSON.stringify({
type: "Feature",
geometry: {
type: "Point",
coordinates: [coordinates?.longitude, coordinates?.latitude],
},
properties: {},
})
}
jessicamcinchak marked this conversation as resolved.
Show resolved Hide resolved
drawGeojsonDataBuffer={20}
resetControlImage="trash"
showScale
showNorthArrow
osProxyEndpoint={`${process.env.REACT_APP_API_URL}/proxy/ordnance-survey`}
clipGeojsonData={JSON.stringify(boundaryBBox)}
collapseAttributions={window.innerWidth < 500 ? true : undefined}
/>
</MapContainer>
</ErrorWrapper>
<MapFooter>
<Typography variant="body2">
The coordinate location of your address point is:{" "}
<strong>
{`${
(coordinates?.x && Math.round(coordinates.x)) || 0
} Easting (X), ${
(coordinates?.y && Math.round(coordinates.y)) || 0
} Northing (Y)`}
</strong>
</Typography>
<Link
component="button"
onClick={() => {
props.setPage("os-address");
props.setAddress(undefined);
}}
>
<Typography variant="body2">
The coordinate location of your address point is:{" "}
<strong>
{`${
(coordinates?.x && Math.round(coordinates.x)) || 0
} Easting (X), ${
(coordinates?.y && Math.round(coordinates.y)) || 0
} Northing (Y)`}
</strong>
I want to select an existing address
</Typography>
<Link
component="button"
onClick={() => {
props.setPage("os-address");
props.setAddress(undefined);
}}
>
<Typography variant="body2">
I want to select an existing address
</Typography>
</Link>
</MapFooter>
</MapContainer>
</Link>
</MapFooter>
<DescriptionInput data-testid="new-address-input" mt={2}>
<InputLabel
label={props.descriptionLabel || DEFAULT_NEW_ADDRESS_LABEL}
Expand All @@ -171,7 +196,7 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
id={`${props.id}-siteDescription`}
value={siteDescription || ""}
errorMessage={
showSiteDescriptionError && !siteDescription
props.showSiteDescriptionError && !siteDescription
? `Enter a site description such as "Land at..."`
: ""
}
Expand All @@ -183,7 +208,7 @@ export default function PlotNewAddress(props: PlotNewAddressProps): FCReturn {
inputProps={{
"aria-describedby": [
props.description ? DESCRIPTION_TEXT : "",
showSiteDescriptionError && !siteDescription
props.showSiteDescriptionError && !siteDescription
? `${ERROR_MESSAGE}-${props.id}`
: "",
]
Expand Down
15 changes: 11 additions & 4 deletions editor.planx.uk/src/@planx/components/FindProperty/Public/Public.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,9 @@ describe("render states", () => {
const descriptionInput = screen.getByTestId("new-address-input");
expect(descriptionInput).toBeInTheDocument();

// expect continue to be disabled because an address has not been selected
expect(screen.getByTestId("continue-button")).toBeDisabled();
// Continue button is always enabled, but validation prevents submit until we have complete address details
expect(screen.getByTestId("continue-button")).toBeEnabled();
await user.click(screen.getByTestId("continue-button"));
expect(handleSubmit).not.toHaveBeenCalled();
});

Expand Down Expand Up @@ -416,9 +417,15 @@ describe("plotting a new address that does not have a uprn yet", () => {
screen.getByText(`Enter a site description such as "Land at..."`),
).toBeInTheDocument();

// expect continue to be disabled because we have incomplete address details
expect(screen.getByTestId("continue-button")).toBeDisabled();
// Continue button is always enabled, but validation prevents submit until we have complete address details
expect(screen.getByTestId("continue-button")).toBeEnabled();
await user.click(screen.getByTestId("continue-button"));
expect(handleSubmit).not.toHaveBeenCalled();

// continue should trigger map error wrapper too
expect(
screen.getByTestId("error-message-plot-new-address-map"),
).toBeInTheDocument();
});

it("recovers previously submitted address when clicking the back button and lands on the map page", async () => {
Expand Down
Loading
Loading