angular-datepicker icon indicating copy to clipboard operation
angular-datepicker copied to clipboard

feat(dateRangePicker): Adding DateRangePicker feature

Open mustafapsd opened this issue 3 years ago • 9 comments

Hi! I've started to work on date range picker feature https://github.com/vlio20/angular-datepicker/issues/54. I'm a bit confused, I hope I'm doing correct. Features to be added:

  1. Hover indication of the range.
  2. Max & MIn range validations.
  3. Preview format.
  4. inline mode.
  5. Multiple ranges (advanced feature - not mandatory).

mustafapsd avatar Feb 15 '22 07:02 mustafapsd

I will work tomorrow on the features that you wrote. I did the changes that you mentioned in the review. Thank you for your review, this is really important for me! Could you share a formatting config file (like .prettierrc)?

mustafapsd avatar Feb 16 '22 01:02 mustafapsd

@mustafapsd I don't have one. I am ok with adding a prettier but it should be on a different PR.

vlio20 avatar Feb 16 '22 06:02 vlio20

I added hover effect and inline mode.

Video

mustafapsd avatar Feb 16 '22 23:02 mustafapsd

@mustafapsd it's very hard to follow the changes because of the linters. Can you please undo them so I can review the actual code that is related to the range feature?

Few more notes:

  1. Maybe it is better to have a dedicated component for the range picker?
  2. Other calendars should also be supported - dateTime/month.

We would also need to add e2e tests.

vlio20 avatar Feb 17 '22 06:02 vlio20

Sorry for linters, writing without formatting is really hard :S

Actually I think the same thing. This will be getting bigger day to day, so will be hard to change.

I never saw time or month pickers in date range pickers. Is it really necessary?

Yes I'll add tests.

mustafapsd avatar Feb 17 '22 08:02 mustafapsd

It is not mandatory but should somehow be reflected there.

For liniting, I suggest that you create a separate PR with the your prettier config file then create PR for the rangepicker. WDYT?

vlio20 avatar Feb 17 '22 08:02 vlio20

I removed linting changes for now. Is this logic true for date range pickers? If I'm doing it correctly then I'll create a new component for the range picker.

mustafapsd avatar Feb 17 '22 09:02 mustafapsd

I will try to pull your fork and play with it and give you my feedback

vlio20 avatar Feb 18 '22 06:02 vlio20

Thank you!

mustafapsd avatar Feb 18 '22 08:02 mustafapsd