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: Whole number option on Number Input #3798

Merged
merged 6 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
21 changes: 17 additions & 4 deletions editor.planx.uk/src/@planx/components/NumberInput/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ import Switch from "@mui/material/Switch";
import { ComponentType as TYPES } from "@opensystemslab/planx-core/types";
import type { NumberInput } from "@planx/components/NumberInput/model";
import { parseNumberInput } from "@planx/components/NumberInput/model";
import {
EditorProps,
ICONS,
} from "@planx/components/ui";
import { EditorProps, ICONS } from "@planx/components/ui";
import { useFormik } from "formik";
import React from "react";
import { ModalFooter } from "ui/editor/ModalFooter";
Expand Down Expand Up @@ -91,6 +88,22 @@ export default function NumberInputComponent(props: Props): FCReturn {
label="Allow negative numbers to be input"
/>
</InputRow>
<InputRow>
<FormControlLabel
control={
<Switch
checked={formik.values.onlyWholeNumbers}
onChange={() =>
formik.setFieldValue(
"onlyWholeNumbers",
!formik.values.onlyWholeNumbers,
)
}
/>
}
label="Only allow whole numbers"
/>
</InputRow>
</ModalSectionContent>
</ModalSection>
<ModalFooter formik={formik} />
Expand Down
50 changes: 50 additions & 0 deletions editor.planx.uk/src/@planx/components/NumberInput/Public.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,56 @@ test("allows negative numbers to be input when toggled on by editor", async () =
expect(handleSubmit).toHaveBeenCalledWith({ data: { fahrenheit: -10 } });
});

test("requires only whole numbers to be input when toggled on by editor", async () => {
const handleSubmit = vi.fn();

const { user } = setup(
<NumberInput
fn="fahrenheit"
title="What's the temperature?"
handleSubmit={handleSubmit}
onlyWholeNumbers={true}
/>,
);

expect(screen.getByRole("heading")).toHaveTextContent(
"What's the temperature?",
);

const textArea = screen.getByLabelText("What's the temperature?");

await user.type(textArea, "10.06");
await user.click(screen.getByTestId("continue-button"));

expect(screen.getByText("Enter a whole number")).toBeInTheDocument();
expect(handleSubmit).not.toHaveBeenCalledWith({
data: { fahrenheit: 10.06 },
});
Copy link
Member

Choose a reason for hiding this comment

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

nit: not.toHaveBeenCalled() might be more meaningful & definitive check here? Because we want to check if the error blocks submission, not that it was possibly automatically rounded to fahrenheit: 10 and successfully submitted for example!

});

test("allows only whole numbers to be input when toggled on by editor", async () => {
const handleSubmit = vi.fn();

const { user } = setup(
<NumberInput
fn="fahrenheit"
title="What's the temperature?"
handleSubmit={handleSubmit}
onlyWholeNumbers={true}
/>,
);

expect(screen.getByRole("heading")).toHaveTextContent(
"What's the temperature?",
);

const textArea = screen.getByLabelText("What's the temperature?");

await user.type(textArea, "10");
await user.click(screen.getByTestId("continue-button"));
expect(handleSubmit).toHaveBeenCalledWith({ data: { fahrenheit: 10 } });
});

test("requires a value before being able to continue", async () => {
const handleSubmit = vi.fn();

Expand Down
15 changes: 15 additions & 0 deletions editor.planx.uk/src/@planx/components/NumberInput/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface NumberInput extends BaseNodeData {
fn?: string;
units?: string;
allowNegatives?: boolean;
onlyWholeNumbers?: boolean;
}

export type UserData = number;
Expand All @@ -28,6 +29,7 @@ export const parseNumberInput = (
fn: data?.fn || "",
units: data?.units,
allowNegatives: data?.allowNegatives || false,
onlyWholeNumbers: data?.onlyWholeNumbers || false,
RODO94 marked this conversation as resolved.
Show resolved Hide resolved
...parseBaseNodeData(data),
});

Expand All @@ -52,6 +54,19 @@ export const numberInputValidationSchema = (input: NumberInput) =>
}
return value === "0" ? true : Boolean(parseNumber(value));
},
})
.test({
name: "check for a whole number",
message: "Enter a whole number",
test: (value: string | undefined) => {
if (!value) {
return false;
}
if (input.onlyWholeNumbers && !Number.isInteger(Number(value))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried a few different ways of validating the whole number. Using the Number.isInteger() seemed the most relevant to what I wanted to check, rather than a string check for a decimal point or anything else.

Copy link
Member

Choose a reason for hiding this comment

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

Number.isInteger() makes sense to me !

return false;
}
return true;
},
});

export const validationSchema = (input: NumberInput) =>
Expand Down
Loading