Simplify EditableText callback logic (palantir#629)

* simply logic for callbacks: cancelEditing only calls onCancel, toggleEditing only calls onConfirm.
rnadrag · Feb 9, 2017 · f4f0998 · f4f0998
1 parent f52269b
commit f4f0998
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 33 deletions.
diff --git a/packages/core/src/components/editable-text/editableText.tsx b/packages/core/src/components/editable-text/editableText.tsx
@@ -212,27 +212,16 @@ export class EditableText extends AbstractComponent<IEditableTextProps, IEditabl
     }
 
     public cancelEditing = () => {
-        const { lastValue, value } = this.state;
-        if (lastValue === value) {
-            this.setState({ isEditing: false });
-        } else {
-            this.setState({ isEditing: false, value: lastValue });
-            // invoke onCancel after onChange so consumers' onCancel can override their onChange
-            safeInvoke(this.props.onChange, lastValue);
-            safeInvoke(this.props.onCancel, lastValue);
-        }
+        const { lastValue } = this.state;
+        this.setState({ isEditing: false, value: lastValue });
+        safeInvoke(this.props.onCancel, lastValue);
     }
 
     public toggleEditing = () => {
         if (this.state.isEditing) {
-            const { lastValue, value } = this.state;
-            if (lastValue === value) {
-                this.setState({ isEditing: false });
-            } else {
-                this.setState({ isEditing: false, lastValue: value });
-                safeInvoke(this.props.onChange, value);
-                safeInvoke(this.props.onConfirm, value) ;
-            }
+            const { value } = this.state;
+            this.setState({ isEditing: false, lastValue: value });
+            safeInvoke(this.props.onConfirm, value);
         } else if (!this.props.disabled) {
             this.setState({ isEditing: true });
         }

diff --git a/packages/core/test/editable-text/editableTextTests.tsx b/packages/core/test/editable-text/editableTextTests.tsx
@@ -79,22 +79,6 @@ describe("<EditableText>", () => {
             assert.isTrue(confirmSpy.calledWith("hello"), `unexpected argument "${confirmSpy.args[0][0]}"`);
         });
 
-        it("does not call onCancel when value not changed and escape key pressed", () => {
-            const confirmSpy = sinon.spy();
-            shallow(<EditableText isEditing={true} onConfirm={confirmSpy} defaultValue="alphabet" />)
-                .find("input")
-                .simulate("keydown", { which: Keys.ESCAPE });
-            assert.isTrue(confirmSpy.notCalled);
-        });
-
-        it("does not call onConfirm when value not changed and enter key pressed", () => {
-            const confirmSpy = sinon.spy();
-            shallow(<EditableText isEditing={true} onConfirm={confirmSpy} defaultValue="alphabet" />)
-                .find("input")
-                .simulate("keydown", { which: Keys.ENTER });
-            assert.isTrue(confirmSpy.notCalled);
-        });
-
         it("calls onEdit when entering edit mode", () => {
             const editSpy = sinon.spy();
             shallow(<EditableText onEdit={editSpy} />)