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

Extend DatePicker to support ranges #5232

Merged

Conversation

ivarnakken
Copy link
Member

@ivarnakken ivarnakken commented Dec 13, 2024

Description

Better than having to use two separate date pickers to simulate it. The plan is to use this to filter on events on /events.

Result

  • Changes look good on both light and dark theme.
  • Changes look good with different viewports (mobile, tablet, etc.).
  • Changes look good with slower Internet connections.

Notice how the regular dropdown does not update which month is shown as default according to the value it is given (unlike the new range version 😎):

Before After
image image
image image

Testing

  • I have thoroughly tested my changes.

  • Tested when start date and end date are the same days (but different times of the day)

  • Tested as both fields and regular components (see images above)

  • Tested that changes to global table styles does not affect the Table component

  • Tested that it looks good on smaller viewports:
    image

  • Tested that the regular date picker still works :)

Also make small improvements to the look of the regular date picker
@ivarnakken ivarnakken added enhancement Pull requests that make enhancements, instead of just purely new features review-needed Pull requests that need review bug-fix Pull requests that fix a bug new-feature Pull requests that introduce a new feature technical-debt Pull requests that reduces technical debt labels Dec 13, 2024
@ivarnakken ivarnakken requested a review from a team December 13, 2024 11:22
@ivarnakken ivarnakken self-assigned this Dec 13, 2024
Copy link

linear bot commented Dec 13, 2024

Copy link

vercel bot commented Dec 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
lego-bricks-storybook ⬜️ Ignored (Inspect) Visit Preview Dec 13, 2024 9:03pm

@ivarnakken ivarnakken force-pushed the ivarnakken/aba-642-extend-datepicker-to-support-ranges branch 2 times, most recently from 216223b to 81a00cf Compare December 13, 2024 14:23
And update e2e specs accordingly ..
When selecting a start date in range mode, automatically adjust the end date
if the selected start date comes after the current end date. This prevents
invalid date ranges where the end date precedes the start date.
@ivarnakken ivarnakken force-pushed the ivarnakken/aba-642-extend-datepicker-to-support-ranges branch from 81a00cf to 26e09c3 Compare December 13, 2024 15:02
Copy link
Member

@norbye norbye left a comment

Choose a reason for hiding this comment

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

Absolutely love it!

Came onto some situations while testing where the choice of date felt a little buggy (regarding which of the two dates should be moved to the new location) - but didn't get any clear cases to reproduce just yet so might be simpler to try it out and change it after a little bit.

One thing that I think should be changed before merging this is closing the dialog after selecting your second date. As times are below dates I would naturally select a date first and that makes it annoying that it decides to close on its own before I've gotten to do that

@ivarnakken
Copy link
Member Author

Came onto some situations while testing where the choice of date felt a little buggy (regarding which of the two dates should be moved to the new location) - but didn't get any clear cases to reproduce just yet so might be simpler to try it out and change it after a little bit.

I made some changes to this like 30 min ago - did you use the up-to-date code?

One thing that I think should be changed before merging this is closing the dialog after selecting your second date. As times are below dates I would naturally select a date first and that makes it annoying that it decides to close on its own before I've gotten to do that

Very good point! I'll fix that asap

@ivarnakken ivarnakken added do-not-merge/WIP Pull requests that are "work in progress", and should not be merged approved Pull requests that have been approved labels Dec 13, 2024
@norbye
Copy link
Member

norbye commented Dec 13, 2024

I made some changes to this like 30 min ago - did you use the up-to-date code?

At first I didn't, then I did - might be why I struggled to reproduce it properly. Looked smoother now

@ivarnakken ivarnakken removed the do-not-merge/WIP Pull requests that are "work in progress", and should not be merged label Dec 13, 2024
@ivarnakken ivarnakken merged commit 1b63f49 into master Dec 18, 2024
8 checks passed
@ivarnakken ivarnakken deleted the ivarnakken/aba-642-extend-datepicker-to-support-ranges branch December 18, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug enhancement Pull requests that make enhancements, instead of just purely new features new-feature Pull requests that introduce a new feature review-needed Pull requests that need review technical-debt Pull requests that reduces technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants