[core] fix(InputGroup): better support for async controlled updates (#…

…4323)

packages/core/src/components/forms/asyncControllableInput.tsx

@@ -22,6 +22,8 @@ export interface IAsyncControllableInputProps
     inputRef?: React.LegacyRef<HTMLInputElement>;
 }
 
+type InputValue = IAsyncControllableInputProps["value"];
+
 export interface IAsyncControllableInputState {
     /**
      * Whether we are in the middle of a composition event.
@@ -34,13 +36,18 @@ export interface IAsyncControllableInputState {
      * It may be updated by a parent component.
      * @default ""
      */
-    externalValue: IAsyncControllableInputProps["value"];
+    value: InputValue;
+
+    /**
+     * The latest input value, which updates during IME composition. Defaults to props.value.
+     */
+    nextValue: InputValue;
 
     /**
-     * The latest input value, which updates during IME composition. If undefined, we use
-     * externalValue instead.
+     * Whether there is a pending update we are expecting from a parent component.
+     * @default false
      */
-    localValue: IAsyncControllableInputProps["value"];
+    hasPendingUpdate: boolean;
 }
 
 /**
@@ -50,8 +57,6 @@ export interface IAsyncControllableInputState {
  * asychronously. This might happen if a component chooses to do async validation of a value
  * returned by the input's `onChange` callback.
  *
- * Implementation adapted from https://jsfiddle.net/m792qtys/ (linked in the above issue thread).
- *
  * Note: this component does not apply any Blueprint-specific styling.
  */
 @polyfill
@@ -60,25 +65,60 @@ export class AsyncControllableInput extends React.PureComponent<
     IAsyncControllableInputState
 > {
     public state: IAsyncControllableInputState = {
-        externalValue: this.props.value,
+        hasPendingUpdate: false,
         isComposing: false,
-        localValue: this.props.value,
+        nextValue: this.props.value,
+        value: this.props.value,
     };
 
-    public static getDerivedStateFromProps({ value }: IAsyncControllableInputProps) {
-        return {
-            externalValue: value,
-        };
+    public static getDerivedStateFromProps(
+        nextProps: IAsyncControllableInputProps,
+        nextState: IAsyncControllableInputState,
+    ): Partial<IAsyncControllableInputState> | null {
+        if (nextState.isComposing || nextProps.value === undefined) {
+            // don't derive anything from props if:
+            // - in uncontrolled mode, OR
+            // - currently composing, since we'll do that after composition ends
+            return null;
+        }
+
+        const userTriggeredUpdate = nextState.nextValue !== nextState.value;
+
+        if (userTriggeredUpdate) {
+            if (nextProps.value === nextState.nextValue) {
+                // parent has processed and accepted our update
+                if (nextState.hasPendingUpdate) {
+                    return { value: nextProps.value, hasPendingUpdate: false };
+                } else {
+                    return { value: nextState.nextValue };
+                }
+            } else {
+                if (nextProps.value === nextState.value) {
+                    // we have sent the update to our parent, but it has not been processed yet. just wait.
+                    // DO NOT set nextValue here, since that will temporarily render a potentially stale controlled value,
+                    // causing the cursor to jump once the new value is accepted
+                    return { hasPendingUpdate: true };
+                }
+                // accept controlled update overriding user action
+                return { value: nextProps.value, nextValue: nextProps.value, hasPendingUpdate: false };
+            }
+        } else {
+            // accept controlled update, could be confirming or denying user action
+            return { value: nextProps.value, nextValue: nextProps.value, hasPendingUpdate: false };
+        }
     }
 
     public render() {
-        const { isComposing, externalValue, localValue } = this.state;
+        const { isComposing, hasPendingUpdate: pendingUpdate, value, nextValue } = this.state;
         const { inputRef, ...restProps } = this.props;
         return (
             <input
                 {...restProps}
                 ref={inputRef}
-                value={isComposing ? localValue : externalValue}
+                // render the pending value even if it is not confirmed by a parent's async controlled update
+                // so that the cursor does not jump to the end of input as reported in
+                // https://github.com/palantir/blueprint/issues/4298
+                value={isComposing || pendingUpdate ? nextValue : value}
                 onCompositionStart={this.handleCompositionStart}
                 onCompositionEnd={this.handleCompositionEnd}
                 onChange={this.handleChange}
@@ -91,7 +131,7 @@ export class AsyncControllableInput extends React.PureComponent<
             isComposing: true,
             // Make sure that localValue matches externalValue, in case externalValue
             // has changed since the last onChange event.
-            localValue: this.state.externalValue,
+            nextValue: this.state.value,
         });
         this.props.onCompositionStart?.(e);
     };
@@ -104,7 +144,7 @@ export class AsyncControllableInput extends React.PureComponent<
     private handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
         const { value } = e.target;
 
-        this.setState({ localValue: value });
+        this.setState({ nextValue: value });
         this.props.onChange?.(e);
     };
 }

packages/core/src/components/forms/text-inputs.md

@@ -9,7 +9,8 @@ Blueprint provides two ways to create a text input:
 
 @## Input group
 
-An input group allows you to add icons and buttons _within_ a text input to expand its
+Input groups are a basic building block used to render text inputs across many Blueprint components.
+They allow you to add icons and buttons _within_ a text input to expand its appearance and
 functionality. For example, you might use an input group to build a visibility toggle for a password
 field.
 
@@ -28,6 +29,12 @@ a controlled or uncontrolled fashion. In addition to its own props, it supports
 all valid props for HTML `<input>` elements and proxies them to that element in
 the DOM; the most common ones are detailed below.
 
+If controlled with the `value` prop, `InputGroup` has support for _asynchronous updates_, which may
+occur with some form handling libraries like `redux-form`. This is not generally encouraged (a value
+returned from `onChange` should generally be sent back to the component as a controlled `value` synchronously),
+but there is basic support for it. Note that the input cursor may jump to the end of the input if the speed
+of text entry (time between change events) is faster than the speed of the async update.
+
 @interface IInputGroupProps
 
 @### CSS

packages/core/test/forms/asyncControllableInputTests.tsx

@@ -23,60 +23,80 @@ import { spy } from "sinon";
 import { AsyncControllableInput } from "../../src/components/forms/asyncControllableInput";
 
 describe("<AsyncControllableInput>", () => {
-    it("renders an input", () => {
-        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
-        assert.strictEqual(wrapper.childAt(0).type(), "input");
-    });
+    describe("uncontrolled mode", () => {
+        it("renders an input", () => {
+            const handleChangeSpy = spy();
+            const wrapper = mount(<AsyncControllableInput type="text" defaultValue="hi" onChange={handleChangeSpy} />);
+            assert.strictEqual(wrapper.childAt(0).type(), "input");
+        });
 
-    it("accepts controlled updates", () => {
-        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
-        assert.strictEqual(wrapper.find("input").prop("value"), "hi");
-        wrapper.setProps({ value: "bye" });
-        assert.strictEqual(wrapper.find("input").prop("value"), "bye");
+        it("triggers onChange", () => {
+            const handleChangeSpy = spy();
+            const wrapper = mount(<AsyncControllableInput type="text" defaultValue="hi" onChange={handleChangeSpy} />);
+            const input = wrapper.find("input");
+            input.simulate("change", { target: { value: "bye" } });
+            const simulatedEvent: React.ChangeEvent<HTMLInputElement> = handleChangeSpy.getCall(0).lastArg;
+            assert.strictEqual(simulatedEvent.target.value, "bye");
+        });
     });
 
-    it("triggers onChange events during composition", () => {
-        const handleChangeSpy = spy();
-        const wrapper = mount(<AsyncControllableInput type="text" value="hi" onChange={handleChangeSpy} />);
-        const input = wrapper.find("input");
+    describe("controlled mode", () => {
+        it("renders an input", () => {
+            const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+            assert.strictEqual(wrapper.childAt(0).type(), "input");
+        });
 
-        input.simulate("compositionstart", { data: "" });
-        input.simulate("compositionupdate", { data: " " });
-        // some browsers trigger this change event during composition, so we test to ensure that our wrapper component does too
-        input.simulate("change", { target: { value: "hi " } });
-        input.simulate("compositionupdate", { data: " ." });
-        input.simulate("change", { target: { value: "hi ." } });
-        input.simulate("compositionend", { data: " ." });
+        it("accepts controlled updates", () => {
+            const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+            assert.strictEqual(wrapper.find("input").prop("value"), "hi");
+            wrapper.setProps({ value: "bye" });
+            assert.strictEqual(wrapper.find("input").prop("value"), "bye");
+        });
 
-        assert.strictEqual(handleChangeSpy.callCount, 2);
-    });
+        it("triggers onChange events during composition", () => {
+            const handleChangeSpy = spy();
+            const wrapper = mount(<AsyncControllableInput type="text" value="hi" onChange={handleChangeSpy} />);
+            const input = wrapper.find("input");
 
-    it("external updates do not override in-progress composition", async () => {
-        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
-        const input = wrapper.find("input");
+            input.simulate("compositionstart", { data: "" });
+            input.simulate("compositionupdate", { data: " " });
+            // some browsers trigger this change event during composition, so we test to ensure that our wrapper component does too
+            input.simulate("change", { target: { value: "hi " } });
+            input.simulate("compositionupdate", { data: " ." });
+            input.simulate("change", { target: { value: "hi ." } });
+            input.simulate("compositionend", { data: " ." });
 
-        input.simulate("compositionstart", { data: "" });
-        input.simulate("compositionupdate", { data: " " });
-        input.simulate("change", { target: { value: "hi " } });
+            assert.strictEqual(handleChangeSpy.callCount, 2);
+        });
 
-        await Promise.resolve();
-        wrapper.setProps({ value: "bye" }).update();
+        it("external updates DO NOT override in-progress composition", async () => {
+            const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+            const input = wrapper.find("input");
 
-        assert.strictEqual(wrapper.find("input").prop("value"), "hi ");
-    });
+            input.simulate("compositionstart", { data: "" });
+            input.simulate("compositionupdate", { data: " " });
+            input.simulate("change", { target: { value: "hi " } });
+
+            await Promise.resolve();
+            wrapper.setProps({ value: "bye" }).update();
+
+            assert.strictEqual(wrapper.find("input").prop("value"), "hi ");
+        });
 
-    it("external updates flush after composition ends", async () => {
-        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
-        const input = wrapper.find("input");
+        it("external updates flush after composition ends", async () => {
+            const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+            const input = wrapper.find("input");
 
-        input.simulate("compositionstart", { data: "" });
-        input.simulate("compositionupdate", { data: " " });
-        input.simulate("change", { target: { value: "hi " } });
-        input.simulate("compositionend", { data: " " });
+            input.simulate("compositionstart", { data: "" });
+            input.simulate("compositionupdate", { data: " " });
+            input.simulate("change", { target: { value: "hi " } });
+            input.simulate("compositionend", { data: " " });
 
-        await Promise.resolve();
-        wrapper.setProps({ value: "bye" }).update();
+            await Promise.resolve();
+            // we are "rejecting" the composition here by supplying a different controlled value
+            wrapper.setProps({ value: "bye" }).update();
 
-        assert.strictEqual(wrapper.find("input").prop("value"), "bye");
+            assert.strictEqual(wrapper.find("input").prop("value"), "bye");
+        });
     });
 });

packages/datetime/test/dateInputTests.tsx

@@ -674,8 +674,10 @@ describe("<DateInput>", () => {
         it("parseDate returns false renders invalid date", () => {
             const invalidParse = sinon.stub().returns(false);
             const wrapper = mount(<DateInput {...props} parseDate={invalidParse} />);
-            wrapper.find("input").simulate("change", { target: { value: "invalid" } });
-            assert.strictEqual(wrapper.find("input").prop("value"), "");
+            const input = wrapper.find("input");
+            input.simulate("change", { target: { value: "invalid" } });
+            input.simulate("blur");
+            assert.strictEqual(wrapper.find("input").prop("value"), DateInput.defaultProps.invalidDateMessage);
         });
     });