Skip to content

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

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

kseamon
Copy link
Collaborator

@kseamon kseamon commented Aug 29, 2022

No description provided.

@kseamon kseamon added the G This is is related to a Google internal issue label Aug 29, 2022
@@ -70,6 +79,36 @@ export class DefaultMatCalendarRangeStrategy<D> implements MatDateRangeSelection

return new DateRange<D>(start, end);
}

createDrag(dragOrigin: D, originalRange: DateRange<D>, newDate: D) {
Copy link
Member

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

@kseamon kseamon force-pushed the date-drag branch 4 times, most recently from 05c001e to 25fff00 Compare October 14, 2022 18:45
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Oct 14, 2022
@kseamon kseamon marked this pull request as ready for review October 14, 2022 19:59
@kseamon kseamon requested a review from zarend as a code owner October 14, 2022 19:59
@kseamon
Copy link
Collaborator Author

kseamon commented Oct 24, 2022

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;

Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this line, but I noticed that sometimes the highlight was being cleared when I stop dragging. See the cell for 19 below.
image

Copy link
Collaborator Author

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?

Copy link
Member

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.

Copy link
Collaborator Author

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?

Copy link
Member

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.

@kseamon
Copy link
Collaborator Author

kseamon commented Oct 28, 2022

@crisbeto Thanks for the review! Fixed a couple things and have a couple follow-up questions on 2 other comments. PTAL

@kseamon
Copy link
Collaborator Author

kseamon commented Nov 4, 2022

@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?

Copy link
Member

@crisbeto crisbeto left a 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;

Copy link
Member

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 crisbeto added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Nov 16, 2022
@crisbeto crisbeto merged commit 8554e15 into angular:main Nov 16, 2022
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker detected: feature PR contains a feature commit G This is is related to a Google internal issue target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants