Skip to content

Commit

Permalink
Rewrite number input to fix float/int issue (streamlit#717)
Browse files Browse the repository at this point in the history 
* Fix NumberInput changing types on submit
* Use separate float/int data for number_input
* Separate value from formatting
* Use has_min and has_max in number_input
* Use double instead of float to support large numbers on number_input
  • Loading branch information
@jrhone
jrhone authored Nov 29, 2019
1 parent 16f7d5a commit d031c06
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 110 deletions.
8 changes: 5 additions & 3 deletions e2e/specs/number_input.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,30 +37,32 @@ describe("st.number_input", () => {
it("has correct default values", () => {
cy.get(".stMarkdown").should(
"have.text",
'value 1: " 0 "' + 'value 2: " 1 "' + 'value 3: " 1 "'
'value 1: " 0.0 "' + 'value 2: " 1 "' + 'value 3: " 1 "'
);
});

it("sets value correctly on enter keypress", () => {
cy.get(".stNumberInput input")
.first()
.clear()
.type("10{enter}");

cy.get(".stMarkdown").should(
"have.text",
'value 1: " 10 "' + 'value 2: " 1 "' + 'value 3: " 1 "'
'value 1: " 10.0 "' + 'value 2: " 1 "' + 'value 3: " 1 "'
);
});

it("sets value correctly on blur", () => {
cy.get(".stNumberInput input")
.first()
.clear()
.type("10")
.blur();

cy.get(".stMarkdown").should(
"have.text",
'value 1: " 10 "' + 'value 2: " 1 "' + 'value 3: " 1 "'
'value 1: " 10.0 "' + 'value 2: " 1 "' + 'value 3: " 1 "'
);
});
});
Binary file modified frontend/cypress/snapshots/linux/2x/number_input.spec.ts/number_input0.snap.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
138 changes: 103 additions & 35 deletions frontend/src/components/widgets/NumberInput/NumberInput.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import React from "react"
import { shallow } from "enzyme"
import { Input as UIInput } from "baseui/input"
import { Map as ImmutableMap } from "immutable"
import { fromJS } from "immutable"
import { WidgetStateManager } from "lib/WidgetStateManager"

import NumberInput, { Props } from "./NumberInput"
Expand All @@ -28,47 +28,103 @@ jest.mock("lib/WidgetStateManager")
const sendBackMsg = jest.fn()
const preventDefault = jest.fn()
const getProps = (elementProps: object = {}): Props => ({
element: ImmutableMap({
element: fromJS({
label: "Label",
min: -Infinity,
max: +Infinity,
has_min: false,
has_max: false,
...elementProps,
}),
width: 0,
disabled: false,
widgetMgr: new WidgetStateManager(sendBackMsg),
})

const getIntProps = (elementProps: object = {}): Props => {
return getProps({
intData: {
default: 10,
min: 0,
max: 0,
},
...elementProps,
})
}

const getFloatProps = (elementProps: object = {}): Props => {
return getProps({
floatData: {
default: 10.0,
min: 0.0,
max: 0.0,
},
...elementProps,
})
}

describe("NumberInput", () => {
it("renders without crashing", () => {
const props = getProps()
const props = getIntProps()
const wrapper = shallow(<NumberInput {...props} />)

expect(wrapper).toBeDefined()
})

it("Should show a label", () => {
const props = getProps()
const props = getIntProps()
const wrapper = shallow(<NumberInput {...props} />)

expect(wrapper.find("label").text()).toBe(props.element.get("label"))
})

it("Should set min/max defaults", () => {
const props = getIntProps()
const wrapper = shallow(<NumberInput {...props} />)

expect(wrapper.instance().getMin()).toBe(-Infinity)
expect(wrapper.instance().getMax()).toBe(+Infinity)
})

describe("FloatData", () => {
it("Should change the state when ArrowDown", () => {
const props = getFloatProps({
format: "%0.2f",
floatData: {
default: 11.0,
step: 0.1,
},
})
const wrapper = shallow(<NumberInput {...props} />)
const InputWrapper = wrapper.find(UIInput)

// @ts-ignore
InputWrapper.props().onKeyDown({
key: "ArrowDown",
preventDefault: preventDefault,
})

expect(preventDefault).toHaveBeenCalled()
expect(wrapper.state("value")).toBe(10.9)
expect(wrapper.state("dirty")).toBe(false)
})
})

describe("Value", () => {
it("Should pass a default value", () => {
const props = getProps({
default: 10,
const props = getIntProps({
intData: {
default: 10,
},
})
const wrapper = shallow(<NumberInput {...props} />)

expect(wrapper.find(UIInput).props().value).toBe(
props.element.get("default")
)
expect(wrapper.find(UIInput).props().value).toBe("10")
})

it("Should call onChange", () => {
const props = getProps({
default: 10,
const props = getIntProps({
intData: {
default: 10,
},
})
const wrapper = shallow(<NumberInput {...props} />)

Expand All @@ -87,8 +143,10 @@ describe("NumberInput", () => {
})

it("Should set value on Enter", () => {
const props = getProps({
default: 10,
const props = getIntProps({
intData: {
default: 10,
},
})
const wrapper = shallow(<NumberInput {...props} />)

Expand All @@ -110,9 +168,11 @@ describe("NumberInput", () => {

describe("Step", () => {
it("Should have an step", () => {
const props = getProps({
default: 10,
step: 1,
const props = getIntProps({
intData: {
default: 10,
step: 1,
},
})
const wrapper = shallow(<NumberInput {...props} />)

Expand All @@ -121,10 +181,12 @@ describe("NumberInput", () => {
})

it("Should change the state when ArrowUp", () => {
const props = getProps({
default: 10,
step: 1,
const props = getIntProps({
format: "%d",
intData: {
default: 10,
step: 1,
},
})
const wrapper = shallow(<NumberInput {...props} />)
const InputWrapper = wrapper.find(UIInput)
Expand All @@ -136,15 +198,17 @@ describe("NumberInput", () => {
})

expect(preventDefault).toHaveBeenCalled()
expect(wrapper.state("value")).toBe("11")
expect(wrapper.state("value")).toBe(11)
expect(wrapper.state("dirty")).toBe(false)
})

it("Should change the state when ArrowDown", () => {
const props = getProps({
default: 10,
step: 1,
const props = getIntProps({
format: "%d",
intData: {
default: 10,
step: 1,
},
})
const wrapper = shallow(<NumberInput {...props} />)
const InputWrapper = wrapper.find(UIInput)
Expand All @@ -156,45 +220,49 @@ describe("NumberInput", () => {
})

expect(preventDefault).toHaveBeenCalled()
expect(wrapper.state("value")).toBe("9")
expect(wrapper.state("value")).toBe(9)
expect(wrapper.state("dirty")).toBe(false)
})

it("stepDown button onClick", () => {
const props = getProps({
default: 10,
step: 1,
const props = getIntProps({
format: "%d",
intData: {
default: 10,
step: 1,
},
})
const wrapper = shallow(<NumberInput {...props} />)
const enhancer = wrapper.find(".controls .step-down")

enhancer.simulate("click")

expect(wrapper.state("dirty")).toBe(false)
expect(wrapper.state("value")).toBe("9")
expect(wrapper.state("value")).toBe(9)
expect(preventDefault).toHaveBeenCalled()
})

it("stepUp button onClick", () => {
const props = getProps({
default: 10,
step: 1,
const props = getIntProps({
format: "%d",
intData: {
default: 10,
step: 1,
},
})
const wrapper = shallow(<NumberInput {...props} />)
const enhancer = wrapper.find(".controls .step-up")

enhancer.simulate("click")

expect(wrapper.state("dirty")).toBe(false)
expect(wrapper.state("value")).toBe("11")
expect(wrapper.state("value")).toBe(11)
expect(preventDefault).toHaveBeenCalled()
})
})

it("Should show a message when it's dirty", () => {
const props = getProps()
const props = getIntProps()
const wrapper = shallow(<NumberInput {...props} />)

wrapper.setState({
Expand Down
Loading

0 comments on commit d031c06

Please sign in to comment.