arqex / arqex/react-datetime

closeOnSelect causes calendar to close on re-renders when no props change

Open
#550 1 comment 2 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.