-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(material/datepicker): Support drag and drop to adjust date ranges #25548
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
Conversation
@@ -70,6 +79,36 @@ export class DefaultMatCalendarRangeStrategy<D> implements MatDateRangeSelection | |||
|
|||
return new DateRange<D>(start, end); | |||
} | |||
|
|||
createDrag(dragOrigin: D, originalRange: DateRange<D>, newDate: D) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite get why the range selection strategy needs to know about dragging. I was imagining that we'd treat it as a preview that is updated on mousemove
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to know the additional information, and the behavior is pretty different than the click-click behavior in regular preview generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As well, there could be differing opinions on what this behavior should be, or whether dragging should be supported at all, so this gives folks that flexibility.
05c001e
to
25fff00
Compare
Note to reviewers: the test failure appears to be an unrelated flake |
@@ -159,18 +165,31 @@ export class MatCalendarBody implements OnChanges, OnDestroy, AfterViewChecked { | |||
/** Width of an individual cell. */ | |||
_cellWidth: string; | |||
|
|||
private _didDragSinceMouseDown = false; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean - the 19 appears to be in the currently hovered state. Is the mouse not currently over 19?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't hovering the 19, I started dragging from it but my mouse was completely outside of the calendar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't really track this one down. One thing I noticed is that the hover effect does not show up in chrome's simulated touch mode. It seems to work as expected in mouse mode the vast majority of the time. Are we willing to live with this for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can fix it in a follow-up.
@crisbeto Thanks for the review! Fixed a couple things and have a couple follow-up questions on 2 other comments. PTAL |
@crisbeto Got the drag behavior fixed. Not sure what to do about the hover effect, but IMO it's not that big an issue. Ok to proceed at this point? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -159,18 +165,31 @@ export class MatCalendarBody implements OnChanges, OnDestroy, AfterViewChecked { | |||
/** Width of an individual cell. */ | |||
_cellWidth: string; | |||
|
|||
private _didDragSinceMouseDown = false; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can fix it in a follow-up.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.