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.

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

feat(datepicker): implement comparison and overlap ranges #18753

Merged
merged 1 commit into from Mar 12, 2020

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 7, 2020

Adds the ability to render a comparison range in the date range picker. When the comparison overlaps with the primary range, the overlapping dates are shown in a separate "overlap" range.

Some examples for reference:

demo
Angular_Material_-_Google_Chrome_2020-03-07_19-24-57
Angular_Material_-_Google_Chrome_2020-03-07_19-25-13
Angular_Material_-_Google_Chrome_2020-03-07_19-25-26
Angular_Material_-_Google_Chrome_2020-03-07_19-25-38
Angular_Material_-_Google_Chrome_2020-03-07_19-26-12

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent merge safe labels Mar 7, 2020
@crisbeto crisbeto requested a review from mmalerba as a code owner March 7, 2020 18:38
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 7, 2020
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

I really don't like the yellow circles, it's very confusing to look at and try to understand. Instead of a straightforward mental mapping like:

  • blue = only selected range occupies this cell
  • yellow = only comparison range occupies this cell
  • green = both ranges occupy this cell

We get:

  • blue = only selected range occupies this cell
  • yellow = ¯\(ツ)
  • green = both ranges occupy this cell

I propose just making the dark yellow circle a dark green circle, and if people really want their users to be confused they can always override it

src/dev-app/datepicker/datepicker-demo.html Show resolved Hide resolved
src/dev-app/datepicker/datepicker-demo.ts Show resolved Hide resolved
@crisbeto
Copy link
Member Author

crisbeto commented Mar 9, 2020

I've reworked it based on the feedback @mmalerba

@mmalerba
Copy link
Contributor

mmalerba commented Mar 9, 2020

I noticed a couple visual bugs:

  1. Notice the extra green glow on the right side of the solid green circle: 1
  2. This is when I have selected 13 and was hovering on 6. Notice the extra pale blue selection, and the fact that the circle is blue instead of green:
    2

@crisbeto
Copy link
Member Author

crisbeto commented Mar 9, 2020

  1. Yeah that's annoying, but it's a side effect of the changes which add the gap between the calendar rows. The problem is that the element isn't perfectly circular anymore so in some cases it can end up sticking out. We might have to re-evaluate how we do the row gap.
  2. That one's a little tricky to resolve with the current approach. I'll see if I can address it when I rework it to use gradients.

@mmalerba
Copy link
Contributor

mmalerba commented Mar 9, 2020

Did some hacking in chrome devtools and this seemed to fix the first issue for LTR:

.mat-calendar-body-cell::before, .mat-calendar-body-cell::after {
  left: 0;
  right: 0;
  // width: unspecified;
}

.mat-calendar-body-range-start:not(.mat-calendar-body-in-comparison-range)::before, .mat-calendar-body-range-start.mat-calendar-body-comparison-start::before, .mat-calendar-body-range-start::after, .mat-calendar-body-comparison-start:not(.mat-calendar-body-in-range)::before, .mat-calendar-body-comparison-start::after {
  left: 5%;
  border-top-left-radius: 999px;
  border-bottom-left-radius: 999px;
  // width: unspecified;
}

.mat-calendar-body-range-end:not(.mat-calendar-body-in-comparison-range)::before, .mat-calendar-body-range-end.mat-calendar-body-comparison-end::before, .mat-calendar-body-range-end::after, .mat-calendar-body-range-start.mat-calendar-body-comparison-end::before, .mat-calendar-body-comparison-end:not(.mat-calendar-body-in-range)::before, .mat-calendar-body-comparison-end:not(.mat-calendar-body-range-start)::after {
  border-top-right-radius: 999px;
  border-bottom-right-radius: 999px;
  right: 5%;
  // width: unspecified;
}

Would still need to be updated for RTL (which is currently broken with and without these adjustments).

It might just be worth revisiting this whole approach though, as it seems very complicated and brittle

@crisbeto crisbeto force-pushed the date-range-comparison branch 3 times, most recently from 117afc7 to ff6461e Compare March 11, 2020 06:29
Adds the ability to render a comparison range in the date range picker. When the comparison overlaps with the primary range, the overlapping dates are shown in a separate "overlap" range.
@crisbeto crisbeto merged commit 6328ef5 into angular:date-range Mar 12, 2020
crisbeto added a commit that referenced this pull request Mar 12, 2020
Adds the ability to render a comparison range in the date range picker. When the comparison overlaps with the primary range, the overlapping dates are shown in a separate "overlap" range.
crisbeto added a commit that referenced this pull request Mar 17, 2020
Adds the ability to render a comparison range in the date range picker. When the comparison overlaps with the primary range, the overlapping dates are shown in a separate "overlap" range.
crisbeto added a commit that referenced this pull request Mar 21, 2020
Adds the ability to render a comparison range in the date range picker. When the comparison overlaps with the primary range, the overlapping dates are shown in a separate "overlap" range.
crisbeto added a commit that referenced this pull request Mar 31, 2020
Adds the ability to render a comparison range in the date range picker. When the comparison overlaps with the primary range, the overlapping dates are shown in a separate "overlap" range.
crisbeto added a commit that referenced this pull request Apr 9, 2020
Adds the ability to render a comparison range in the date range picker. When the comparison overlaps with the primary range, the overlapping dates are shown in a separate "overlap" range.
@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 Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants