From 80055740e2c4429af5e5ab08f8f4093008f28d7f Mon Sep 17 00:00:00 2001 From: Batiati Date: Tue, 16 Jun 2020 05:32:31 -0300 Subject: [PATCH] Allow slider render for integer/numeric types (#1796) Feature implemented + Tests + Docs --- .../BasicDeploymentForm.test.tsx | 22 + .../BasicDeploymentForm.tsx | 4 +- .../BasicDeploymentForm/SliderParam.test.tsx | 342 ++++++--- .../BasicDeploymentForm/SliderParam.tsx | 8 +- .../BasicDeploymentForm.test.tsx.snap | 682 ++++++++++++++++++ .../__snapshots__/SliderParam.test.tsx.snap | 114 +++ docs/developer/basic-form-support.md | 2 +- 7 files changed, 1055 insertions(+), 119 deletions(-) diff --git a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.test.tsx b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.test.tsx index fbbcbb247ea..ad3023f2d36 100644 --- a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.test.tsx +++ b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.test.tsx @@ -50,6 +50,28 @@ const defaultProps = { } as IBasicFormParam, ], }, + { + description: "renders a basic deployment with a integer disk size", + params: [ + { + path: "size", + value: 10, + type: "integer", + render: "slider", + } as IBasicFormParam, + ], + }, + { + description: "renders a basic deployment with a number disk size", + params: [ + { + path: "size", + value: 10.0, + type: "number", + render: "slider", + } as IBasicFormParam, + ], + }, { description: "renders a basic deployment with username, password, email and a generic string", params: [ diff --git a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.tsx b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.tsx index f3927e3ada0..163093ad957 100644 --- a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.tsx +++ b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/BasicDeploymentForm.tsx @@ -114,7 +114,9 @@ class BasicDeploymentForm extends React.Component { ); break; } - case "string": { + case "string": + case "integer": + case "number": { if (param.render === "slider") { const p = param as IBasicFormSliderParam; paramComponent = ( diff --git a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.test.tsx b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.test.tsx index dac1ba17043..ca6698e7e9c 100644 --- a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.test.tsx +++ b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.test.tsx @@ -7,168 +7,282 @@ import SliderParam from "./SliderParam"; const defaultProps = { id: "disk", label: "Disk Size", - param: { - value: "10Gi", - type: "string", - path: "disk", - } as IBasicFormParam, handleBasicFormParamChange: jest.fn(() => jest.fn()), min: 1, max: 100, unit: "Gi", }; -it("renders a disk size param with a default value", () => { - const wrapper = shallow(); - expect(wrapper.state("value")).toBe(10); - expect(wrapper).toMatchSnapshot(); -}); - -it("changes the value of the param when the slider changes", () => { - const param = { +const params: IBasicFormParam[] = [ + { value: "10Gi", type: "string", path: "disk", - } as IBasicFormParam; - const handleBasicFormParamChange = jest.fn(() => { - param.value = "20Gi"; - return jest.fn(); - }); + }, + { + value: 10, + type: "integer", + path: "disk", + }, + { + value: 10.0, + type: "number", + path: "disk", + }, +]; - const wrapper = shallow( - , - ); - expect(wrapper.state("value")).toBe(10); - - const slider = wrapper.find(Slider); - (slider.prop("onChange") as (values: number[]) => void)([20]); - - expect(param.value).toBe("20Gi"); - expect(handleBasicFormParamChange.mock.calls[0]).toEqual([ - { value: "20Gi", type: "string", path: "disk" }, - ]); +it("renders a disk size param with a default value", () => { + params.forEach(param => { + const wrapper = shallow(); + expect(wrapper.state("value")).toBe(10); + expect(wrapper).toMatchSnapshot(); + }); }); -it("updates state but does not change param value during slider update (only when dropped in a point)", () => { - const handleBasicFormParamChange = jest.fn(); - const wrapper = shallow( - , - ); - expect(wrapper.state("value")).toBe(10); +describe("when changing the slide", () => { + it("changes the value of the string param", () => { + params.forEach(param => { + const cloneParam = { ...param } as IBasicFormParam; + const expected = param.type === "string" ? "20Gi" : 20; + + const handleBasicFormParamChange = jest.fn(() => { + cloneParam.value = expected; + return jest.fn(); + }); - const slider = wrapper.find(Slider); - (slider.prop("onUpdate") as (values: number[]) => void)([20]); + const wrapper = shallow( + , + ); + + expect(wrapper.state("value")).toBe(10); + + const slider = wrapper.find(Slider); + (slider.prop("onChange") as (values: number[]) => void)([20]); + + expect(cloneParam.value).toBe(expected); + expect(handleBasicFormParamChange.mock.calls[0]).toEqual([ + { value: expected, type: param.type, path: param.path }, + ]); + }); + }); - expect(wrapper.state("value")).toBe(20); - expect(handleBasicFormParamChange).not.toHaveBeenCalled(); + it("changes the value of the string param without unit", () => { + params.forEach(param => { + const cloneParam = { ...param } as IBasicFormParam; + const expected = param.type === "string" ? "20" : 20; + + const handleBasicFormParamChange = jest.fn(() => { + cloneParam.value = expected; + return jest.fn(); + }); + + const wrapper = shallow( + , + ); + + expect(wrapper.state("value")).toBe(10); + + const slider = wrapper.find(Slider); + (slider.prop("onChange") as (values: number[]) => void)([20]); + + expect(cloneParam.value).toBe(expected); + expect(handleBasicFormParamChange.mock.calls[0]).toEqual([ + { value: expected, type: param.type, path: param.path }, + ]); + }); + }); }); -describe("when changing the value in the input", () => { - it("parses a number and forwards it", () => { - const valueChange = jest.fn(); - const handleBasicFormParamChange = jest.fn(() => valueChange); +it("updates state but does not change param value during slider update (only when dropped in a point)", () => { + params.forEach(param => { + const handleBasicFormParamChange = jest.fn(); const wrapper = shallow( - , + , ); expect(wrapper.state("value")).toBe(10); - const input = wrapper.find("input#disk"); - const event = { currentTarget: { value: "20" } } as React.FormEvent; - (input.prop("onChange") as (e: React.FormEvent) => void)(event); + const slider = wrapper.find(Slider); + (slider.prop("onUpdate") as (values: number[]) => void)([20]); expect(wrapper.state("value")).toBe(20); - expect(valueChange.mock.calls[0]).toEqual([{ currentTarget: { value: "20Gi" } }]); + expect(handleBasicFormParamChange).not.toHaveBeenCalled(); + }); +}); + +describe("when changing the value in the input", () => { + it("parses a number and forwards it", () => { + params.forEach(param => { + const valueChange = jest.fn(); + const handleBasicFormParamChange = jest.fn(() => valueChange); + const wrapper = shallow( + , + ); + expect(wrapper.state("value")).toBe(10); + + const input = wrapper.find("input#disk"); + const event = { currentTarget: { value: "20" } } as React.FormEvent; + (input.prop("onChange") as (e: React.FormEvent) => void)(event); + + expect(wrapper.state("value")).toBe(20); + + const expected = param.type === "string" ? "20Gi" : 20; + expect(valueChange.mock.calls[0]).toEqual([{ currentTarget: { value: expected } }]); + }); + }); + + it("parses a number and forwards it without unit", () => { + params.forEach(param => { + const valueChange = jest.fn(); + const handleBasicFormParamChange = jest.fn(() => valueChange); + const wrapper = shallow( + , + ); + expect(wrapper.state("value")).toBe(10); + + const input = wrapper.find("input#disk"); + const event = { currentTarget: { value: "20" } } as React.FormEvent; + (input.prop("onChange") as (e: React.FormEvent) => void)(event); + + expect(wrapper.state("value")).toBe(20); + + const expected = param.type === "string" ? "20" : 20; + expect(valueChange.mock.calls[0]).toEqual([{ currentTarget: { value: expected } }]); + }); }); it("ignores values in the input that are not digits", () => { - const valueChange = jest.fn(); - const handleBasicFormParamChange = jest.fn(() => valueChange); - const wrapper = shallow( - , - ); - expect(wrapper.state("value")).toBe(10); + params.forEach(param => { + const valueChange = jest.fn(); + const handleBasicFormParamChange = jest.fn(() => valueChange); + const wrapper = shallow( + , + ); + expect(wrapper.state("value")).toBe(10); - const input = wrapper.find("input#disk"); - const event = { currentTarget: { value: "foo20*#@$" } } as React.FormEvent; - (input.prop("onChange") as (e: React.FormEvent) => void)(event); + const input = wrapper.find("input#disk"); + const event = { currentTarget: { value: "foo20*#@$" } } as React.FormEvent; + (input.prop("onChange") as (e: React.FormEvent) => void)(event); - expect(wrapper.state("value")).toBe(20); - expect(valueChange.mock.calls[0]).toEqual([{ currentTarget: { value: "20Gi" } }]); + expect(wrapper.state("value")).toBe(20); + + const expected = param.type === "string" ? "20Gi" : 20; + expect(valueChange.mock.calls[0]).toEqual([{ currentTarget: { value: expected } }]); + }); }); it("accept decimal values", () => { - const valueChange = jest.fn(); - const handleBasicFormParamChange = jest.fn(() => valueChange); - const wrapper = shallow( - , - ); - expect(wrapper.state("value")).toBe(10); + params.forEach(param => { + const valueChange = jest.fn(); + const handleBasicFormParamChange = jest.fn(() => valueChange); + const wrapper = shallow( + , + ); + expect(wrapper.state("value")).toBe(10); + + const input = wrapper.find("input#disk"); + const event = { currentTarget: { value: "20.5" } } as React.FormEvent; + (input.prop("onChange") as (e: React.FormEvent) => void)(event); - const input = wrapper.find("input#disk"); - const event = { currentTarget: { value: "20.5" } } as React.FormEvent; - (input.prop("onChange") as (e: React.FormEvent) => void)(event); + expect(wrapper.state("value")).toBe(20.5); - expect(wrapper.state("value")).toBe(20.5); - expect(valueChange.mock.calls[0]).toEqual([{ currentTarget: { value: "20.5Gi" } }]); + const expected = param.type === "string" ? "20.5Gi" : 20.5; + expect(valueChange.mock.calls[0]).toEqual([{ currentTarget: { value: expected } }]); + }); }); it("modifies the max value of the slider if the input is bigger than 100", () => { - const valueChange = jest.fn(); - const handleBasicFormParamChange = jest.fn(() => valueChange); - const wrapper = shallow( - , - ); - expect(wrapper.state("value")).toBe(10); + params.forEach(param => { + const valueChange = jest.fn(); + const handleBasicFormParamChange = jest.fn(() => valueChange); + const wrapper = shallow( + , + ); + expect(wrapper.state("value")).toBe(10); - const input = wrapper.find("input#disk"); - const event = { currentTarget: { value: "200" } } as React.FormEvent; - (input.prop("onChange") as (e: React.FormEvent) => void)(event); + const input = wrapper.find("input#disk"); + const event = { currentTarget: { value: "200" } } as React.FormEvent; + (input.prop("onChange") as (e: React.FormEvent) => void)(event); - expect(wrapper.state("value")).toBe(200); - const slider = wrapper.find(Slider); - expect(slider.prop("max")).toBe(200); + expect(wrapper.state("value")).toBe(200); + const slider = wrapper.find(Slider); + expect(slider.prop("max")).toBe(200); + }); }); }); it("uses the param minimum and maximum if defined", () => { - const param = { - value: "10Gi", - type: "string", - path: "disk", - minimum: 5, - maximum: 50, - } as IBasicFormParam; + params.forEach(param => { + const clonedParam = { ...param } as IBasicFormParam; + clonedParam.minimum = 5; + clonedParam.maximum = 50; - const wrapper = shallow(); + const wrapper = shallow(); - const slider = wrapper.find(Slider); - expect(slider.prop("min")).toBe(5); - expect(slider.prop("max")).toBe(50); + const slider = wrapper.find(Slider); + expect(slider.prop("min")).toBe(5); + expect(slider.prop("max")).toBe(50); + }); }); it("defaults to the min if the value is undefined", () => { - const param = { - type: "string", - path: "disk", - } as IBasicFormParam; + params.forEach(param => { + const cloneParam = { ...param } as IBasicFormParam; + cloneParam.value = undefined; - const wrapper = shallow(); - - expect(wrapper.state("value")).toBe(5); + const wrapper = shallow(); + expect(wrapper.state("value")).toBe(5); + }); }); it("updates the state when receiving new props", () => { - const handleBasicFormParamChange = jest.fn(); - const wrapper = shallow( - , - ); - expect(wrapper.state("value")).toBe(10); - - wrapper.setProps({ param: { value: "20Gi" } }); - expect(wrapper.state("value")).toBe(20); - expect(handleBasicFormParamChange).not.toHaveBeenCalled(); + params.forEach(param => { + const handleBasicFormParamChange = jest.fn(); + const wrapper = shallow( + , + ); + expect(wrapper.state("value")).toBe(10); + + const newValue = param.type === "string" ? "20Gi" : 20; + wrapper.setProps({ param: { value: newValue } }); + expect(wrapper.state("value")).toBe(20); + expect(handleBasicFormParamChange).not.toHaveBeenCalled(); + }); }); diff --git a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.tsx b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.tsx index 0ae3ff373e6..d3a1090c36d 100644 --- a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.tsx +++ b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/SliderParam.tsx @@ -18,9 +18,9 @@ export interface ISliderParamState { value: number; } -function toNumber(value: string) { +function toNumber(value: string | number) { // Force to return a Number from a string removing any character that is not a digit - return Number(value.replace(/[^\d\.]/g, "")); + return typeof value === "number" ? value : Number(value.replace(/[^\d\.]/g, "")); } function getDefaultValue(min: number, value?: string) { @@ -101,7 +101,9 @@ class SliderParam extends React.Component private handleParamChange = (value: number) => { this.props.handleBasicFormParamChange(this.props.param)({ - currentTarget: { value: `${value}${this.props.unit}` }, + currentTarget: { + value: this.props.param.type === "string" ? `${value}${this.props.unit}` : value, + }, } as React.FormEvent); }; } diff --git a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/__snapshots__/BasicDeploymentForm.test.tsx.snap b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/__snapshots__/BasicDeploymentForm.test.tsx.snap index 979e8b6b457..4210f7dbba3 100644 --- a/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/__snapshots__/BasicDeploymentForm.test.tsx.snap +++ b/dashboard/src/components/DeploymentFormBody/BasicDeploymentForm/__snapshots__/BasicDeploymentForm.test.tsx.snap @@ -834,6 +834,688 @@ exports[`renders a basic deployment with a generic string 1`] = ` `; +exports[`renders a basic deployment with a integer disk size 1`] = ` + +
+
+