feat(date-picker): add date range picker #606 #823
Conversation
There was a problem hiding this comment.
Thanks very much! Great work! I took an initial look over the code - didn't get much time, but I spotted a few small things, but functional seems to work well.
I know we chatted quite a bit about API and functionality, so it would perhaps be good to get few others sets of eyes on this in case they have other thoughts or suggestions that maybe we did not think of 😊
Thanks again, really nice addition!
|
Hey @ashley-hunter you are not wrong at all, but when you use it like this
it is working, because the linkedSignal is having the reference to the signal itself and not to the value of the signal this is wrong you also get a compiler issue this is working this is also working you need to pass a computation and not a value :) 
|
apps/app/src/app/pages/(components)/components/(calendar)/calendar.page.ts
Outdated
Show resolved
Hide resolved
| selectDate(date: T): void { | ||
| const start = this.startDate(); | ||
| const end = this.endDate(); | ||
|
|
||
| if (!start && !end) { | ||
| this.startDate.set(date); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (start && !end) { | ||
| if (this._dateAdapter.isAfter(date, start)) { | ||
| this.endDate.set(date); | ||
| } else if (this._dateAdapter.isBefore(date, start)) { | ||
| this.startDate.set(date); | ||
| this.endDate.set(start); | ||
| } else if (this._dateAdapter.isSameDay(date, start)) { | ||
| this.endDate.set(date); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // If both start and end are selected, reset selection | ||
| this.startDate.set(date); | ||
|
|
||
| this.endDate.set(undefined); | ||
| } |
There was a problem hiding this comment.
What if dateDisabled input is passed and some dates are disabled. Should we allow range selection over a disabled date?
For example the 6th is disabled and I select the 4th and than the 8th.
Should the range be 4.8 - 8.8 or should the start date reset to 8th because the 6th is disabled? If we want this type of selection, should it maybe be a config option?
There was a problem hiding this comment.
Oh this is a great catch, hmm... at least angular material is just allowing to set min and max date. Maybe we should not allow to disable single dates in a date range picker? I don't find a use case for that maybe you?
There was a problem hiding this comment.
A use case for a date range picker could be to select a date range for some kind of booking like accommodation. Then it would make sense to allow disabling single or multiple dates and thus the range should not allow selection with disabled dates in between.
I guess that use case and feature is very specific. Not sure if we need it in the first implementation or add it later when needed.
There was a problem hiding this comment.
This is very special, then we just should not allow it and implement multiple date ranges. This we can implement later in my point of view
| public readonly date = input<T>(); | ||
|
|
||
| protected readonly _mutableDate = linkedSignal(this.date); |
There was a problem hiding this comment.
I am just curious, why not use model but instead use an input and linkedSignal? Isn't model exactly that but just as one value?
There was a problem hiding this comment.
sadly not... for e.g. the date is touched in the writeValue. In that case the model is emiting a change event which is actually not true. This is the reason why i splitted that up.
MerlinMoos
force-pushed
the
date-range-picker
branch
from
647fb83 to
5f8a7c0
Compare
There was a problem hiding this comment.
Thanks very much for the updates! Looking great! I think there is a conflict that needs resolved, and just one issue I spotted is if I change month and select a range, when the popover closes and open again it reverts back to the current month, whereas I believe we should restore it to the month with the selection.
|
hey @ashley-hunter I restore now the calendar to the startday |
PR Checklist
Please check if your PR fulfills the following requirements:
guidelines: https://github.com/spartan-ng/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
PR Type
What kind of change does this PR introduce?
Which package are you modifying?
Primitives
Others
What is the current behavior?
Closes #606
What is the new behavior?
Does this PR introduce a breaking change?
I removed the changed output from the date-picker and date-multi picker. The model two way binding is changed to a normal input and linked signal. For ngModel and forms the date model is emitting to many events.
Other information