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

Refactor DatePicker- and TimePicker-component #3941

Merged
merged 2 commits into from Aug 29, 2023
Merged

Conversation

eikhr
Copy link
Member

@eikhr eikhr commented May 27, 2023

Description

Small refactor of the DatePicker thingy. Shouldn't change too much, but rewrote to functional components and simplified the logic a bit. ~100 fewer lines of code:)

Result

Some minor changes in appearance:

Before After
Screenshot 2023-05-27 at 18 54 41 Screenshot 2023-06-10 at 10 25 07

I have changed the behaviour slightly to simplify the logic. In the meeting editor we would previously automatically move the end time forward if it was set before the start time, but this didn't play well with the simplified component. Now it will simply show a form validation error (which is how it works in f.ex. the event-editor).
Screenshot 2023-06-10 at 09 54 58

The end-time auto-moving is kept intact! It now moved the end time to ~two hours after the start time when blurring (deselecting) the start time field.

Testing

  • I have thoroughly tested my changes.

I've gone through every usage of the component in the webapp, and checked manually that it still works as expected.

@github-actions github-actions bot added the review-needed Pull requests that need review label May 27, 2023
@eikhr eikhr force-pushed the function-components branch 3 times, most recently from 01a13d0 to df9a298 Compare June 2, 2023 09:57
@eikhr eikhr force-pushed the function-components branch 4 times, most recently from b59599d to aee0d4f Compare June 10, 2023 08:26
@eikhr eikhr added technical-debt Pull requests that reduces technical debt chore Pull requests that does something "boring", yet important, e.g. cleaning up code labels Jun 10, 2023
@eikhr eikhr marked this pull request as ready for review June 10, 2023 08:33
@eikhr eikhr force-pushed the function-components branch 2 times, most recently from 7b1b5ab to 992cc72 Compare June 10, 2023 09:16
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

Code looks solid, and the component looks nicer!

But I do like when the end time automatically updates after changing the start time.

Copy link
Contributor

@erlingfn erlingfn left a comment

Choose a reason for hiding this comment

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

Excellent rewrite, yet the feature moving the end time forward will be sorely missed. Is it not possible to make this work with functional components?

@ivarnakken
Copy link
Member

Excellent rewrite, yet the feature moving the end time forward will be sorely missed. Is it not possible to make this work with functional components?

I'd like to add that we should extend the date picker to have a range functionality. Like this: https://ui.shadcn.com/docs/components/date-picker#date-range-picker

@ivarnakken ivarnakken added the approved Pull requests that have been approved label Jun 28, 2023
@ollfkaih ollfkaih added the do-not-merge/WIP Pull requests that are "work in progress", and should not be merged label Jul 11, 2023
@ivarnakken ivarnakken added the question Pull requests with an unresolved question label Jul 12, 2023
@ivarnakken
Copy link
Member

ivarnakken commented Jul 12, 2023

@eikhr What's the status on this? I see there are some unresolved questions.

@eikhr
Copy link
Member Author

eikhr commented Jul 12, 2023

@eikhr What's the status on this? Is see there are some unresolved questions.

Yep, I am going to add back the functionality for automatically moving end date forwards. Just haven't gotten around to it yet.

@eikhr eikhr removed the do-not-merge/WIP Pull requests that are "work in progress", and should not be merged label Aug 22, 2023
@eikhr eikhr force-pushed the function-components branch 2 times, most recently from f6997cf to f18222d Compare August 28, 2023 15:07
End-time is moved to the whole hour between 1 and 2 hours after the start-time, after the startTime field is blurred (deselected).
@eikhr eikhr merged commit 4e95168 into master Aug 29, 2023
4 checks passed
@eikhr eikhr deleted the function-components branch August 29, 2023 08:29
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 chore Pull requests that does something "boring", yet important, e.g. cleaning up code question Pull requests with an unresolved question review-needed Pull requests that need review technical-debt Pull requests that reduces technical debt
Projects
None yet
4 participants