[NumericInput] Add onButtonClick prop #1178
Conversation
Add onButtonClick prop, add testPreview: documentation |
| @@ -110,6 +110,9 @@ export interface INumericInputProps extends IIntentProps, IProps { | |||
| /** The value to display in the input field. */ | |||
| value?: number | string; | |||
|
|
|||
| /** The callback invoked when the value changes as a result of a button click. */ | |||
| onButtonClick?(valueAsNumber: number, valueAsString: string): void; | |||
|
|
|||
| /** The callback invoked when the value changes. */ | |||
There was a problem hiding this comment.
add more details here to distinguish from oBC: "when the value changes due to typing, arrow keys, or button clicks."
| @@ -477,22 +483,26 @@ export class NumericInput extends AbstractComponent<HTMLInputProps & INumericInp | |||
| } | |||
|
|
|||
| private invokeOnValueChangeCallback(value: string) { | |||
| const valueAsString = value; | |||
| const valueAsNumber = +value; // coerces non-numeric strings to NaN | |||
There was a problem hiding this comment.
can we keep this comment around? perhaps in invokeOnValueChangeCallback, to remind us why we're not using parseInt.
| } | ||
|
|
||
| private invokeOnButtonClickCallback(value: string) { | ||
| Utils.safeInvoke(this.props.onButtonClick, +value, value); |
There was a problem hiding this comment.
ok let's get clever and do both in one method:
private invokeValueCallback(value: string, callback = this.props.onButtonClick) {
Utils.safeInvoke(callback, +value, value);
}types of oBC and oVC are the same so by assigning a default value you don't have to redeclare the function signature here!
Respond to CR feedbackPreview: documentation | table |
|
|
||
| private invokeOnButtonClickCallback(value: string) { | ||
| Utils.safeInvoke(this.props.onButtonClick, +value, value); | ||
| private invokeValueCallback(value: string, callback: (valueAsNumber: number, valueAsString: string) => void) { |
There was a problem hiding this comment.
wait i don't understand why you can't set this.props.onValueChange as the default value for callback parameter?
maybe the invokeValueCallback in incrementValue is causing those double-events?
There was a problem hiding this comment.
Ah, finally got it. Say you define only one of the callbacks:
<NumericInput onButtonClick={...} /> or
<NumericInput onValueChange={...} />
If you define a default callback, Utils.safeInvoke will never receive an undefined function for whichever callback is undefined; it'll simply re-invoke the default function whether in handle[*]ButtonClick or incrementValue. Given that, I think we should go with the explicit type definition implemented above. ^
Fixes #607 (see #607 (comment))
Checklist
Changes proposed in this pull request:
onButtonClickprop that fires when either button is "clicked" (via mouse, focus +Space, focus +Enter, etc.)Reviewers should focus on: