closeOnSelect causes calendar to close on re-renders when no props change
- Dominant language
- JavaScript
- Stars
- 2k
- Forks
- 858
- PR merge metrics
- No merged PRs in 30d
Description
### I'm Submitting a ...
```
[x] Bug report
[ ] Feature request
[ ] Support request
```
### Steps to Reproduce
1. Render an instance of the Datetime component with the `closeOnSelect` prop set to true.
1. Click the associated input field to open the calendar popup.
1. Trigger a re-render of the component without modifying any props (e.g. when the props for the parent component change and it is re-rendered).
* _Note:_ Must be done while on the day, month, or year view.
### Expected Results
* The calendar popup should remain open.
### Actual Results
* The calendar popup is automatically closed.
### Minimal Reproduction of the Problem
I have a pending set of changes that correct this issue and as part of that set of changes I'm planning to add the following pair of tests which exhibit this issue:
```
it('closeOnSelect: closes the calendar when re-redering if the date was changed', (done) => {
const component = utils.createDatetime({ closeOnSelect: true });
// A unknown race condition is causing this test to fail without this time out,
// and when the test fails it says:
// 'Timeout - Async callback was not invoked within timeout'
// Ideally it would say something else but at least we know the tests are passing now
setTimeout(() => {
expect(utils.isOpen(component)).toBeFalsy();
utils.openDatepicker(component);
expect(utils.isOpen(component)).toBeTruthy();
const date = new Date(2000, 0, 15, 2, 2, 2, 2);
component.setProps({ closeOnSelect: true, value: date }, () => {
expect(utils.isOpen(component)).toBeFalsy();
done();
});
}, 0);
});
it('closeOnSelect: does NOT close the calendar when re-redering if props are unchanged', (done) => {
const component = utils.createDatetime({ closeOnSelect: true });
// A unknown race condition is causing this test to fail without this time out,
// and when the test fails it says:
// 'Timeout - Async callback was not invoked within timeout'
// Ideally it would say something else but at least we know the tests are passing now
setTimeout(() => {
expect(utils.isOpen(component)).toBeFalsy();
utils.openDatepicker(component);
expect(utils.isOpen(component)).toBeTruthy();
component.setProps(component.props, () => {
expect(utils.isOpen(component)).toBeTruthy();
done();
});
}, 0);
});
```
### Other Information (e.g. stacktraces, related issues, suggestions how to fix)
My proposal is to update the `componentWillReceiveProps` method to identify when the `value` prop actually changed (i.e. the date is explicitly being set by something outside the component) and to update the `closeOnSelect` condition ([line 157](https://github.com/YouCanBookMe/react-datetime/blob/master/DateTime.js#L157)) to only close the calendar when this is the case.
For more context or justification of the validity of supporting calls to `componentWillReceiveProps` with identical an identical set of props see the [(A => B) !=> (B => A) article](https://reactjs.org/blog/2016/01/08/A-implies-B-does-not-imply-B-implies-A.html) by Jim Sproch.
Contributor guide
Research direction
Inspect DateTime.js around line 157 and the componentWillReceiveProps method, then run the existing datetime component tests. Use the two described re-render scenarios to verify that changed values close the calendar while unchanged props leave it open.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, react
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100