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

Fix resetting to blank value #840

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mgedmin
Copy link

@mgedmin mgedmin commented Aug 10, 2022

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

  • I want to have a Reset button that clears two Datetime controls used for date range filtering.
  • Resetting to a blank value used to work and broke once I upgraded to react-datetime 3.0.1.
  • Cannot reset value #760 indicates that other people want this fixed too.

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?

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 arqex#760.
This is needed to get the tests to pass, AFAIC.
@mgedmin
Copy link
Author

mgedmin commented Aug 10, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot reset value
1 participant