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 enabled continue button on DrawBoundary map page #3138

Merged
merged 2 commits into from
May 13, 2024
Merged

fix(a11y): always enabled continue button on DrawBoundary map page #3138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64f05e7
add error wrapper to map and always enable continue
jessicamcinchak May 11, 2024
30658c2
consistent error msg formatting
jessicamcinchak May 13, 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
9 changes: 7 additions & 2 deletions editor.planx.uk/src/@planx/components/DrawBoundary/Public/Public.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ it("should not have any accessibility violations", async () => {
expect(results).toHaveNoViolations();
});

test("shows the file upload option by default and requires user data to continue", async () => {
test("shows the file upload option by default and requires user data to continue from either page", async () => {
const handleSubmit = jest.fn();

const { user } = setup(
Expand All @@ -134,7 +134,12 @@ test("shows the file upload option by default and requires user data to continue

// Draw a boundary screen
expect(screen.getByTestId("upload-file-button")).toBeInTheDocument();
expect(screen.getByTestId("continue-button")).toBeDisabled();
expect(screen.getByTestId("continue-button")).toBeEnabled();

await user.click(screen.getByTestId("continue-button"));
expect(
screen.getByTestId("error-message-draw-boundary-map"),
).toBeInTheDocument();

// Navigate to upload a file screen
await user.click(screen.getByTestId("upload-file-button"));
Expand Down
139 changes: 78 additions & 61 deletions editor.planx.uk/src/@planx/components/DrawBoundary/Public/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const slotsSchema = array()
.required()
.test({
name: "nonUploading",
message: "Upload a location plan.",
message: "Upload a location plan",
test: (slots?: Array<FileUploadSlot>) => {
return Boolean(
slots &&
Expand All @@ -60,6 +60,7 @@ export default function Component(props: Props) {
passport.data?.["property.boundary.title.area"];
const [boundary, setBoundary] = useState<Boundary>(previousBoundary);
const [area, setArea] = useState<number | undefined>(previousArea);
const [mapValidationError, setMapValidationError] = useState<string>();

// Buffer applied to the address point to clip this map extent
// and applied to the site boundary and written to the passport to later clip the map extent in overview documents
Expand Down Expand Up @@ -114,26 +115,45 @@ export default function Component(props: Props) {
}, [page, setArea, setBoundary, setSlots]);

/**
* Declare a ref to hold a mutable copy the up-to-date validation error.
* Declare refs to hold a mutable copy the up-to-date validation errors
* The intention is to prevent frequent unnecessary update loops that clears the
* validation error state if it is already empty.
*/
const validationErrorRef = useRef(fileValidationError);
const fileValidationErrorRef = useRef(fileValidationError);
useEffect(() => {
validationErrorRef.current = fileValidationError;
fileValidationErrorRef.current = fileValidationError;
}, [fileValidationError]);

useEffect(() => {
if (validationErrorRef.current) {
if (fileValidationErrorRef.current) {
setFileValidationError(undefined);
}
}, [slots]);

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

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

const validateAndSubmit = () => {
const newPassportData: Store.userData["data"] = {};

// Used the map
if (page === "draw") {
if (!props.hideFileUpload && !boundary) {
setMapValidationError("Draw a boundary");
}

if (props.hideFileUpload && !boundary) {
props.handleSubmit?.({ data: { ...newPassportData } });
}

if (boundary && props.dataFieldBoundary) {
newPassportData[props.dataFieldBoundary] = boundary;
newPassportData[`${props.dataFieldBoundary}.buffered`] = buffer(
Expand Down Expand Up @@ -165,10 +185,6 @@ export default function Component(props: Props) {

props.handleSubmit?.({ data: { ...newPassportData } });
}

if (props.hideFileUpload && !boundary) {
props.handleSubmit?.({ data: { ...newPassportData } });
}
}

// Uploaded a file
Expand Down Expand Up @@ -198,19 +214,16 @@ export default function Component(props: Props) {
};

return (
<Card
handleSubmit={validateAndSubmit}
isValid={
props.hideFileUpload
? true
: Boolean((page === "draw" && boundary) || page === "upload")
}
>
{getBody(bufferInMeters, fileValidationError)}
<Card handleSubmit={validateAndSubmit} isValid={true}>
{getBody(bufferInMeters, mapValidationError, fileValidationError)}
</Card>
);

function getBody(bufferInMeters: number, fileValidationError?: string) {
function getBody(
bufferInMeters: number,
mapValidationError?: string,
fileValidationError?: string,
) {
if (page === "draw") {
return (
<>
Expand All @@ -223,50 +236,54 @@ export default function Component(props: Props) {
definitionImg={props.definitionImg}
/>
<FullWidthWrapper>
<MapContainer environment={environment} size="large">
<p style={visuallyHidden}>
An interactive map centred on your address, pre-populated with a
red boundary that includes the entire property, using
information from the Land Registry. You can accept this boundary
as your location plan by continuing, you can amend it by
clicking and dragging the points, or you can erase it by
clicking the reset button and draw a new custom boundary.
</p>
{!props.hideFileUpload && (
<ErrorWrapper error={mapValidationError} id="draw-boundary-map">
<MapContainer environment={environment} size="large">
<p style={visuallyHidden}>
If you prefer to upload a file instead of using the
interactive map, please click "Upload a location plan instead"
below to navigate to the file upload.
An interactive map centred on your address, pre-populated with
a red boundary that includes the entire property, using
information from the Land Registry. You can accept this
boundary as your location plan by continuing, you can amend it
by clicking and dragging the points, or you can erase it by
clicking the reset button and draw a new custom boundary.
</p>
)}
{/* @ts-ignore */}
<my-map
id="draw-boundary-map"
ariaLabelOlFixedOverlay="An interactive map for providing your location plan boundary"
drawMode
drawPointer="crosshair"
drawGeojsonData={JSON.stringify(boundary)}
drawGeojsonDataBuffer={10}
clipGeojsonData={
addressPoint &&
JSON.stringify(
buffer(addressPoint, bufferInMeters, { units: "meters" }),
)
}
zoom={20}
maxZoom={23}
latitude={Number(passport?.data?._address?.latitude)}
longitude={Number(passport?.data?._address?.longitude)}
showCentreMarker
markerLatitude={Number(passport?.data?._address?.latitude)}
markerLongitude={Number(passport?.data?._address?.longitude)}
resetControlImage="trash"
osProxyEndpoint={`${process.env.REACT_APP_API_URL}/proxy/ordnance-survey`}
osCopyright={`Basemap subject to Crown copyright and database rights ${new Date().getFullYear()} OS (0)100024857`}
drawGeojsonDataCopyright={`<a href="https://www.planning.data.gov.uk/dataset/title-boundary" target="_blank" style="color:#0010A4;">Title boundary</a> subject to Crown copyright and database rights ${new Date().getFullYear()} OS (0)100026316`}
collapseAttributions={self.innerWidth < 500 ? true : undefined}
/>
</MapContainer>
{!props.hideFileUpload && (
<p style={visuallyHidden}>
If you prefer to upload a file instead of using the
interactive map, please click "Upload a location plan
instead" below to navigate to the file upload.
</p>
)}
{/* @ts-ignore */}
<my-map
id="draw-boundary-map"
ariaLabelOlFixedOverlay="An interactive map for providing your location plan boundary"
drawMode
drawPointer="crosshair"
drawGeojsonData={JSON.stringify(boundary)}
drawGeojsonDataBuffer={10}
clipGeojsonData={
addressPoint &&
JSON.stringify(
buffer(addressPoint, bufferInMeters, { units: "meters" }),
)
}
zoom={20}
maxZoom={23}
latitude={Number(passport?.data?._address?.latitude)}
longitude={Number(passport?.data?._address?.longitude)}
showCentreMarker
markerLatitude={Number(passport?.data?._address?.latitude)}
markerLongitude={Number(passport?.data?._address?.longitude)}
resetControlImage="trash"
osProxyEndpoint={`${process.env.REACT_APP_API_URL}/proxy/ordnance-survey`}
osCopyright={`Basemap subject to Crown copyright and database rights ${new Date().getFullYear()} OS (0)100024857`}
drawGeojsonDataCopyright={`<a href="https://www.planning.data.gov.uk/dataset/title-boundary" target="_blank" style="color:#0010A4;">Title boundary</a> subject to Crown copyright and database rights ${new Date().getFullYear()} OS (0)100026316`}
collapseAttributions={
self.innerWidth < 500 ? true : undefined
}
/>
</MapContainer>
</ErrorWrapper>
<MapFooter>
<Typography variant="body1">
The property boundary you have drawn is{" "}
Expand Down
2 changes: 1 addition & 1 deletion editor.planx.uk/src/@planx/components/FileUpload/Public.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ test("shows error if user tries to continue before adding files", async () => {
);

await user.click(screen.getByTestId("continue-button"));
expect(screen.getByText("Upload at least one file.")).toBeInTheDocument();
expect(screen.getByText("Upload at least one file")).toBeInTheDocument();

// Blocked by validation error
expect(handleSubmit).toHaveBeenCalledTimes(0);
Expand Down
2 changes: 1 addition & 1 deletion editor.planx.uk/src/@planx/components/FileUpload/Public.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const slotsSchema = array()
.required()
.test({
name: "nonUploading",
message: "Upload at least one file.",
message: "Upload at least one file",
Copy link
Member Author

Choose a reason for hiding this comment

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

minor un-related tidy up here: our error messages typically don't use punctuation, so just updating this one too for consistency !

test: (slots?: Array<FileUploadSlot>) => {
return Boolean(
slots &&
Expand Down
Loading