arqex / react-datetime

A lightweight but complete datetime picker react component.
2k stars 869 forks source link

Fix resetting to blank value #840

Open mgedmin opened 2 years ago

mgedmin commented 2 years ago

Description

When you change the value prop to a blank string, Datetime.getSelectedDate() tries to parse the value using the format string. This fails for blank values, and then getSelectedDate() falls back to the previously entered input value from this.state.inputValue, ignoring the new value prop and making the reset fail.

Closes #760.

Motivation and Context

Checklist

[x] I have not included any built dist files (us maintainers do that prior to a new release)
[x] I have added tests covering my changes
[x] All new and existing tests pass
[ ] My changes required the documentation to be updated
  [ ] I have updated the documentation accordingly
  [ ] I have updated the TypeScript 1.8 type definitions accordingly
  [ ] I have updated the TypeScript 2.0+ type definitions accordingly

I have tested that the fix works manually, by modifying the playground like this


diff --git a/src/playground/App.js b/src/playground/App.js
index 2673b5f..0b7ee8f 100644
--- a/src/playground/App.js
+++ b/src/playground/App.js
@@ -9,13 +9,14 @@ import Datetime from '../DateTime';

 class App extends React.Component { 
        state = {
-               date: new Date()
+               value: '',
        }

        render() {
                return (
                        <div>
-                               <Datetime />
+                               <button onClick={() => this.setState({value: ''})}>Reset</button>
+                               <Datetime value={this.state.value} onChange={(value) => this.setState({value})} />
                        </div>
                );
        }

I have attempted to write a unit test, but failed miserably. Here's my attempt:

diff --git a/test/tests.spec.js b/test/tests.spec.js
index 498afa2..8e53ca1 100644
--- a/test/tests.spec.js
+++ b/test/tests.spec.js
@@ -1408,6 +1408,20 @@ describe('Datetime', () => {
                                done();
                        });
                });
+
+               it('should allow the value to be reset to blank', done => {
+                       const value1 = moment('2022-08-10T13:49:22.121Z');
+
+                       let component = utils.createDatetime({ value: value1 });
+                       expect( component.instance().state.viewDate.toISOString() ).toBe(value1.toISOString());
+
+                       component.setProps({ value: '' });
+                       setTimeout( () => {
+                               console.log(component.find('.form-control').getDOMNode().outerHTML);
+                               expect(utils.getInputValue(component)).toEqual('');
+                               done();
+                       });
+               });
        });

        describe('View navigation', () => {

The test fails with

  Expected: ""
  Received: "08/10/2022 4:49 PM"]

and I can't get the form control to render anything other than

      <input type="text" class="form-control" value="08/10/2022 4:49 PM">

no matter how I mangle the renderInput() code in src/DateTime.js. I'm probably missing something stupid, like not running npm build prior to npm test.

Uhh. That was it.

Please advise: the README says There's no need to create a new build for each pull request, but if I don't do that, the new test fails. What should I do?

mgedmin commented 2 years ago

On the theory that it's easier to drop a commit with a git rebase than to redo it from scratch an unknown time later, I've committed the test, the playground update, and the rebuild separately.