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

[NumericInput] allows min === max #3296

Merged
merged 2 commits into from
Jan 23, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 3 additions & 4 deletions packages/core/src/common/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@ export const HOTKEYS_WARN_DECORATOR_NO_METHOD = ns + ` @HotkeysTarget-decorated
export const HOTKEYS_WARN_DECORATOR_NEEDS_REACT_ELEMENT =
ns + ` "@HotkeysTarget-decorated components must return a single JSX.Element or an empty render.`;

export const NUMERIC_INPUT_MIN_MAX =
ns + ` <NumericInput> requires min to be strictly less than max if both are defined.`;
export const NUMERIC_INPUT_MIN_MAX = ns + ` <NumericInput> requires min to be no greater than max if both are defined.`;
export const NUMERIC_INPUT_MINOR_STEP_SIZE_BOUND =
ns + ` <NumericInput> requires minorStepSize to be strictly less than stepSize.`;
ns + ` <NumericInput> requires minorStepSize to be no greater than stepSize.`;
export const NUMERIC_INPUT_MAJOR_STEP_SIZE_BOUND =
ns + ` <NumericInput> requires majorStepSize to be strictly greater than stepSize.`;
ns + ` <NumericInput> requires stepSize to be no greater than majorStepSize.`;
export const NUMERIC_INPUT_MINOR_STEP_SIZE_NON_POSITIVE =
ns + ` <NumericInput> requires minorStepSize to be strictly greater than zero.`;
export const NUMERIC_INPUT_MAJOR_STEP_SIZE_NON_POSITIVE =
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/forms/numericInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export class NumericInput extends AbstractPureComponent<HTMLInputProps & INumeri

protected validateProps(nextProps: HTMLInputProps & INumericInputProps) {
const { majorStepSize, max, min, minorStepSize, stepSize } = nextProps;
if (min != null && max != null && min >= max) {
if (min != null && max != null && min > max) {
throw new Error(Errors.NUMERIC_INPUT_MIN_MAX);
}
if (stepSize == null) {
Expand Down
19 changes: 19 additions & 0 deletions packages/core/test/controls/numericInputTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,25 @@ describe("<NumericInput>", () => {
});
});

describe("if min === max", () => {
it("never changes value", () => {
const onValueChangeSpy = spy();
const component = mount(<NumericInput min={2} max={2} onValueChange={onValueChangeSpy} />);
// repeated interactions, no change in state
component
.find(Button)
.first()
.simulate("mousedown")
.simulate("mousedown")
.simulate("mousedown")
.simulate("mousedown")
.simulate("mousedown");
expect(component.state().value).to.equal("2");
expect(onValueChangeSpy.callCount).to.equal(5);
expect(onValueChangeSpy.args[0]).to.deep.equal([2, "2"]);
});
});

describe("clampValueOnBlur", () => {
it("does not clamp or invoke onValueChange on blur if clampValueOnBlur=false", () => {
// should be false by default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import * as React from "react";

import {
H5,
HTMLSelect,
Intent,
INumericInputProps,
Expand All @@ -29,7 +30,7 @@ const MIN_VALUES = [
{ label: "None", value: -Infinity },
{ label: "-10", value: -10 },
{ label: "0", value: 0 },
{ label: "10", value: 10 },
{ label: "20", value: 20 },
];

const MAX_VALUES = [
Expand Down Expand Up @@ -66,7 +67,6 @@ export class NumericInputBasicExample extends React.PureComponent<IExampleProps,
private handleMaxChange = handleNumberChange(max => this.setState({ max }));
private handleMinChange = handleNumberChange(min => this.setState({ min }));
private handleIntentChange = handleStringChange((intent: Intent) => this.setState({ intent }));

private handleButtonPositionChange = handleStringChange((buttonPosition: INumericInputProps["buttonPosition"]) =>
this.setState({ buttonPosition }),
);
Expand Down Expand Up @@ -110,14 +110,14 @@ export class NumericInputBasicExample extends React.PureComponent<IExampleProps,

return (
<>
<Label>Modifiers</Label>
<H5>Props</H5>
{this.renderSwitch("Disabled", disabled, this.toggleDisabled)}
{this.renderSwitch("Fill", fill, this.toggleFullWidth)}
{this.renderSwitch("Large", large, this.toggleLargeSize)}
{this.renderSwitch("Left icon", leftIcon != null, this.toggleLeftIcon)}
{this.renderSwitch("Numeric characters only", allowNumericCharactersOnly, this.toggleNumericCharsOnly)}
{this.renderSwitch("Select all on focus", selectAllOnFocus, this.toggleSelectAllOnFocus)}
{this.renderSwitch("Select all on increment", selectAllOnIncrement, this.toggleSelectAllOnIncrement)}
{this.renderSwitch("Disabled", disabled, this.toggleDisabled)}
{this.renderSwitch("Left icon", leftIcon != null, this.toggleLeftIcon)}
{this.renderSwitch("Fill container", fill, this.toggleFullWidth)}
{this.renderSwitch("Large", large, this.toggleLargeSize)}
{this.renderSelectMenu("Minimum value", min, MIN_VALUES, this.handleMinChange)}
{this.renderSelectMenu("Maximum value", max, MAX_VALUES, this.handleMaxChange)}
{this.renderSelectMenu(
Expand All @@ -137,19 +137,17 @@ export class NumericInputBasicExample extends React.PureComponent<IExampleProps,

private renderSelectMenu(
label: string,
selectedValue: number | string,
value: number | string,
options: IOptionProps[],
onChange: React.FormEventHandler<HTMLElement>,
onChange: React.FormEventHandler,
) {
return (
<Label>
{label}
<HTMLSelect value={selectedValue} onChange={onChange} options={options} />
<HTMLSelect {...{ value, onChange, options }} />
Copy link
Member

Choose a reason for hiding this comment

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

that's kinda weird

</Label>
);
}

private handleValueChange = (_valueAsNumber: number, valueAsString: string) => {
this.setState({ value: valueAsString });
};
private handleValueChange = (_v: number, value: string) => this.setState({ value });
}