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

DatePicker: fix incorrect interaction on range change when defaultTime provided #9310

Merged
merged 1 commit into from
Jan 18, 2018
Merged

DatePicker: fix incorrect interaction on range change when defaultTime provided #9310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions packages/date-picker/src/panel/date-range.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,9 +426,8 @@
},

handleChangeRange(val) {
const defaultTime = this.defaultTime || [];
this.minDate = modifyWithGivenTime(val.minDate, defaultTime[0]);
this.maxDate = modifyWithGivenTime(val.maxDate, defaultTime[1]);
this.minDate = val.minDate;
Copy link
Contributor

@wacky6 wacky6 Jan 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's better to "trim" time before passing {min, max}Date to date-table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cancelled my previous changes here to fix this issue (now it's just the same behavior as what it used to be). If we want to add trim behavior before passing minDate / maxDate to date-table, maybe that should be another issue?

Copy link
Contributor

@wacky6 wacky6 Jan 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I mean resetting the time to 00:00:00 before passing minDate/maxDate to date-table.

date-table checks date equality in a naive way, so it can not handle non 00:00:00 time. But the rest of date-range panel should know about the selected time.

Like this:

<date-table :min-date="resetTime(minDate)">

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. Thx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems it is a little complex to realize this.

  1. I have to reset to 12:00:00 to make it work (I'm still looking into why), which will change the default behavior on pick.

  2. There is some "reference equal" issue, which means, when resetTime returns a new Date object (even with the same time value), it breaks.

I'll spend time on it later. If you have any ideas about the above problems, plz hint me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. Might have something to do with legacy codes. I suggest you keep current solution, it works. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree that.

this.maxDate = val.maxDate;
this.rangeState = val.rangeState;
},

Expand Down
58 changes: 32 additions & 26 deletions test/unit/specs/date-picker.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,20 +1272,23 @@ describe('DatePicker', () => {
triggerEvent(leftCell, 'click', true);
setTimeout(_ => {
triggerEvent(rightCell, 'mousemove', true);
triggerEvent(rightCell, 'click', true);

setTimeout(_ => {
const {
minDate,
maxDate
} = vmWithDefaultTime.picker;
expect(minDate.getHours()).to.be.equal(11);
expect(minDate.getMinutes()).to.be.equal(59);
expect(minDate.getSeconds()).to.be.equal(59);
expect(maxDate.getHours()).to.be.equal(0);
expect(maxDate.getMinutes()).to.be.equal(0);
expect(maxDate.getSeconds()).to.be.equal(0);
done();
expect(rightCell.classList.contains('in-range')).to.be.true;

triggerEvent(rightCell, 'click', true);
setTimeout(_ => {
const {
minDate,
maxDate
} = vmWithDefaultTime.picker;
expect(minDate.getHours()).to.be.equal(11);
expect(minDate.getMinutes()).to.be.equal(59);
expect(minDate.getSeconds()).to.be.equal(59);
expect(maxDate.getHours()).to.be.equal(0);
expect(maxDate.getMinutes()).to.be.equal(0);
expect(maxDate.getSeconds()).to.be.equal(0);
done();
}, DELAY);
}, DELAY);
}, DELAY);
}, DELAY);
Expand Down Expand Up @@ -1317,20 +1320,23 @@ describe('DatePicker', () => {
triggerEvent(leftCell, 'click', true);
setTimeout(_ => {
triggerEvent(rightCell, 'mousemove', true);
triggerEvent(rightCell, 'click', true);

setTimeout(_ => {
const {
minDate,
maxDate
} = vmWithDefaultTime.picker;
expect(minDate.getHours()).to.be.equal(11);
expect(minDate.getMinutes()).to.be.equal(59);
expect(minDate.getSeconds()).to.be.equal(59);
expect(maxDate.getHours()).to.be.equal(18);
expect(maxDate.getMinutes()).to.be.equal(0);
expect(maxDate.getSeconds()).to.be.equal(0);
done();
expect(rightCell.classList.contains('in-range')).to.be.true;

triggerEvent(rightCell, 'click', true);
setTimeout(_ => {
const {
minDate,
maxDate
} = vmWithDefaultTime.picker;
expect(minDate.getHours()).to.be.equal(11);
expect(minDate.getMinutes()).to.be.equal(59);
expect(minDate.getSeconds()).to.be.equal(59);
expect(maxDate.getHours()).to.be.equal(18);
expect(maxDate.getMinutes()).to.be.equal(0);
expect(maxDate.getSeconds()).to.be.equal(0);
done();
}, DELAY);
}, DELAY);
}, DELAY);
}, DELAY);
Expand Down