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.

Sign up for GitHub

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

Jump to bottom

Move the call to onDatesChange before the call to onFocusChange #1525

Merged
merged 5 commits into from
Feb 6, 2019
Merged

Move the call to onDatesChange before the call to onFocusChange #1525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fe1d97c
Move the call to onDatesChange before the call to onFocusChange
nkinser Feb 4, 2019
c89cc3c
resolved rebased
nkinser Feb 5, 2019
aaf2ab6
added tests to verify onDatesChange only called once
nkinser Feb 5, 2019
21e9bb5
changed way I retrieve callCount
nkinser Feb 5, 2019
95ee636
adjust test
nkinser Feb 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/components/DayPickerRangeController.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,8 @@ export default class DayPickerRangeController extends React.PureComponent {
return;
}

onDatesChange({ startDate, endDate });

if (!keepOpenOnDateSelect) {
onFocusChange(null);
onClose({ startDate, endDate });
Expand All @@ -512,6 +514,8 @@ export default class DayPickerRangeController extends React.PureComponent {
}
}

onDatesChange({ startDate, endDate });
ljharb marked this conversation as resolved.
Show resolved Hide resolved

if (isEndDateDisabled && !isStartDateAfterEndDate) {
onFocusChange(null);
onClose({ startDate, endDate });
Expand All @@ -523,20 +527,26 @@ export default class DayPickerRangeController extends React.PureComponent {

if (!startDate) {
endDate = day;
onDatesChange({ startDate, endDate });
onFocusChange(START_DATE);
} else if (isInclusivelyAfterDay(day, firstAllowedEndDate)) {
endDate = day;
onDatesChange({ startDate, endDate });
if (!keepOpenOnDateSelect) {
onFocusChange(null);
onClose({ startDate, endDate });
}
} else if (disabled !== START_DATE) {
startDate = day;
endDate = null;
onDatesChange({ startDate, endDate });
} else {
onDatesChange({ startDate, endDate });
}
} else {
onDatesChange({ startDate, endDate });
}

onDatesChange({ startDate, endDate });
onBlur();
}

Expand Down
131 changes: 128 additions & 3 deletions test/components/DayPickerRangeController_spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1623,6 +1623,133 @@ describe('DayPickerRangeController', () => {
});
});

describe('props.onDatesChange only called once in onDayClick', () => {
it('calls props.onDatesChange once when focusedInput === START_DATE', () => {
const clickDate = moment(today);
const onDatesChangeStub = sinon.stub();
const wrapper = shallow((
<DayPickerRangeController
onDatesChange={onDatesChangeStub}
focusedInput={START_DATE}
endDate={null}
/>
));
wrapper.instance().onDayClick(clickDate);
expect(onDatesChangeStub).to.have.property('callCount', 1);
const args = onDatesChangeStub.getCall(0).args[0];
expect(args.startDate.format()).to.equal(clickDate.clone().format());
expect(args.endDate).to.equal(null);
});

it('calls props.onDatesChange once when focusedInput === END_DATE and there is no startDate', () => {
const clickDate = moment(today);
const onDatesChangeStub = sinon.stub();
const wrapper = shallow((
<DayPickerRangeController
onDatesChange={onDatesChangeStub}
focusedInput={END_DATE}
startDate={null}
/>
));
wrapper.instance().onDayClick(clickDate);
expect(onDatesChangeStub).to.have.property('callCount', 1);
const args = onDatesChangeStub.getCall(0).args[0];
expect(args.startDate).to.equal(null);
expect(args.endDate.format()).to.equal(clickDate.clone().format());
});

it('calls props.onDatesChange once when focusedInput === END_DATE and the day is a valid endDate', () => {
const clickDate = moment(today);
const startDate = clickDate.clone().subtract(2, 'days');
const onDatesChangeStub = sinon.stub();
const wrapper = shallow((
<DayPickerRangeController
onDatesChange={onDatesChangeStub}
focusedInput={END_DATE}
minimumNights={2}
startDate={startDate}
/>
));
wrapper.instance().onDayClick(clickDate);
expect(onDatesChangeStub).to.have.property('callCount', 1);
const args = onDatesChangeStub.getCall(0).args[0];
expect(args.startDate.format()).to.equal(startDate.clone().format());
expect(args.endDate.format()).to.equal(clickDate.clone().format());
});

it('calls props.onDatesChange once when focusedInput === END_DATE, the day is an invalid endDate, and disabled !== START_DATE', () => {
const clickDate = moment(today);
const onDatesChangeStub = sinon.stub();
const wrapper = shallow((
<DayPickerRangeController
onDatesChange={onDatesChangeStub}
focusedInput={END_DATE}
minimumNights={2}
startDate={clickDate.clone().add(1, 'days')}
endDate={null}
/>
));
wrapper.instance().onDayClick(clickDate);
expect(onDatesChangeStub).to.have.property('callCount', 1);
const args = onDatesChangeStub.getCall(0).args[0];
expect(args.startDate.format()).to.equal(clickDate.clone().format());
expect(args.endDate).to.equal(null);
});

it('calls props.onDatesChange once when focusedInput === END_DATE and the day is an invalid endDate', () => {
const clickDate = moment(today);
const startDate = clickDate.clone().add(1, 'days');
const onDatesChangeStub = sinon.stub();
const wrapper = shallow((
<DayPickerRangeController
onDatesChange={onDatesChangeStub}
focusedInput={END_DATE}
disabled={START_DATE}
minimumNights={2}
startDate={startDate}
endDate={null}
/>
));
wrapper.instance().onDayClick(clickDate);
expect(onDatesChangeStub).to.have.property('callCount', 1);
const args = onDatesChangeStub.getCall(0).args[0];
expect(args.startDate.format()).to.equal(startDate.clone().format());
expect(args.endDate).to.equal(null);
});

it('calls props.onDatesChange once when there is a startDateOffset', () => {
const clickDate = moment(today);
const onDatesChangeStub = sinon.stub();
const wrapper = shallow((
<DayPickerRangeController
onDatesChange={onDatesChangeStub}
startDateOffset={day => day.subtract(2, 'days')}
/>
));
wrapper.instance().onDayClick(clickDate);
expect(onDatesChangeStub).to.have.property('callCount', 1);
const args = onDatesChangeStub.getCall(0).args[0];
expect(args.startDate.format()).to.equal(clickDate.clone().subtract(2, 'days').format());
expect(args.endDate.format()).to.equal(clickDate.clone().format());
});

it('calls props.onDatesChange once when there is a endDateOffset', () => {
const clickDate = moment(today);
const onDatesChangeStub = sinon.stub();
const wrapper = shallow((
<DayPickerRangeController
onDatesChange={onDatesChangeStub}
endDateOffset={day => day.add(4, 'days')}
/>
));
wrapper.instance().onDayClick(clickDate);
expect(onDatesChangeStub).to.have.property('callCount', 1);
const args = onDatesChangeStub.getCall(0).args[0];
expect(args.startDate.format()).to.equal(clickDate.clone().format());
expect(args.endDate.format()).to.equal(clickDate.clone().add(4, 'days').format());
});
});

describe('logic in props.onDatesChange affects props.onFocusChange', () => {
let preventFocusChange;
let focusedInput;
Expand Down Expand Up @@ -1652,10 +1779,8 @@ describe('DayPickerRangeController', () => {
focusedInput={START_DATE}
/>
));
// The first day click sets preventFocusChange to true, but it doesn't take effect until the
// second day click because onFocusChange is called before onDatesChange
wrapper.instance().onDayClick(clickDate);
expect(focusedInput).to.equal(END_DATE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does seem like a possible breaking change to me. @majapw?

Copy link
Collaborator

@majapw majapw Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! I'm gonna release it as v19 after I release the previous changes in a minor.

expect(focusedInput).to.equal(START_DATE);
wrapper.instance().onDayClick(clickDate.clone().add(1, 'days'));
expect(focusedInput).to.equal(END_DATE);
});
Expand Down