feat(dateRangePicker): Adding DateRangePicker feature #590

mustafapsd · 2022-02-15T07:38:35Z

Hi! I've started to work on date range picker feature #54. I'm a bit confused, I hope I'm doing correct. Features to be added:

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

vlio20 · 2022-02-15T09:04:50Z

projects/ng2-date-picker/src/lib/date-picker/date-picker.component.ts

-import {IDatePickerConfig, IDatePickerConfigInternal} from './date-picker-config.model';
-import {IDpDayPickerApi} from './date-picker.api';
-import {DatePickerService} from './date-picker.service';
+import { IDate, IDateRange } from '../common/models/date.model';


please revet those changes as they are not relevant

vlio20 · 2022-02-15T09:05:41Z

projects/ng2-date-picker/.eslintrc.json

@@ -10,8 +10,8 @@
      ],
      "parserOptions": {
        "project": [
-          "projects/ng2-datepicker/tsconfig.lib.json",
-          "projects/ng2-datepicker/tsconfig.spec.json"
+          "projects/ng2-date-picker/tsconfig.lib.json",


how is that relevant to your changes? I think that you pulled from older version and not from the one in the master

Imports are wrong, so the editor gives errors. You can see these wrong imports.

https://github.com/vlio20/angular-datepicker/blob/master/projects/ng2-date-picker/.eslintrc.json

vlio20 · 2022-02-15T09:07:32Z

projects/ng2-date-picker/src/lib/date-picker/date-picker.component.ts

-                                            ControlValueAccessor,
-                                            Validator,
-                                            OnDestroy {
+  OnInit,


Please revert all linting changes.

Opps, sorry

vlio20 · 2022-02-15T09:08:47Z

projects/ng2-date-picker/src/lib/date-picker/date-picker.component.ts

@@ -93,13 +93,16 @@ export class DatePickerComponent implements OnChanges,
  @Input() maxDate: SingleCalendarValue;
  @Input() minTime: SingleCalendarValue;
  @Input() maxTime: SingleCalendarValue;
+  @Input() dateRangePicker: boolean = false;
+  @Input() dateRange: IDateRange;


The range value should be set through the controller input - not as an additional Input of the component.

I thought that user can set default values. This is not work now. Should I remove this?

vlio20 · 2022-02-15T09:10:06Z

projects/ng2-date-picker/src/lib/date-picker/date-picker.component.ts

+    }
+
+    this.inputElementValue = (this.utilsService.convertToString(range.from, this.componentConfig.format))
+      + ' <> ' + (this.utilsService.convertToString(range.to, this.componentConfig.format));


Please use template strings

Let's make the delimiter to be configured

Ok

I'll work for limiters tomorrow.

vlio20 · 2022-02-15T09:10:41Z

projects/ng2-date-picker/src/lib/day-calendar/day-calendar.component.less

@@ -43,12 +43,32 @@
      color: @c-white;
    }

-    .dp-prev-month, .dp-next-month {
+    .dp-start-date {
+      background: @c-primary !important;


please avoid using important. add additional classes if needed.

I've removed, thanks

vlio20 · 2022-02-15T09:10:56Z

projects/ng2-date-picker/src/lib/day-calendar/day-calendar.component.ts

+
+      this.dateRange.to = day.date;
+      this.onDateRangeSelect.emit(this.dateRange);
+      // eslint-disable-next-line no-console


please remove

What's wrong here? Thank you for review.

Oh okey, you want to remove eslint-disable line. I currently using this line for testing.

mustafapsd · 2022-02-16T01:03:00Z

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)?

vlio20 · 2022-02-16T06:28:47Z

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

mustafapsd · 2022-02-16T23:07:12Z

I added hover effect and inline mode.

Video

vlio20 · 2022-02-17T06:47:50Z

@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:

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

We would also need to add e2e tests.

mustafapsd · 2022-02-17T08:00:42Z

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.

vlio20 · 2022-02-17T08:41:50Z

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?

mustafapsd · 2022-02-17T09:14:23Z

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.

vlio20 · 2022-02-18T06:16:59Z

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

mustafapsd · 2022-02-18T08:17:22Z

Thank you!

mustafapsd added 3 commits February 15, 2022 02:12

Fix wrong import

efcf474

Add dateRangePicker to library

79f093c

Add dateRangePicker to demo

7c3fdbb

vlio20 requested changes Feb 15, 2022

View reviewed changes

mustafapsd added 4 commits February 16, 2022 03:51

Add range to string function, remove lint

78e3212

Remove important usages

3b3b7b2

Remove console log

a97a931

Remove lint changes

4f341c2

mustafapsd added 3 commits February 17, 2022 01:37

Add hover effect to daterangepicker

eac17d0

Add set date range when write a range to input

b932fe5

Fix hover effect bug

239b6d3

Remove linting changes

1767e3a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dateRangePicker): Adding DateRangePicker feature #590

feat(dateRangePicker): Adding DateRangePicker feature #590

mustafapsd commented Feb 15, 2022

vlio20 Feb 15, 2022

vlio20 Feb 15, 2022

mustafapsd Feb 15, 2022

vlio20 Feb 15, 2022

mustafapsd Feb 15, 2022

vlio20 Feb 15, 2022

mustafapsd Feb 15, 2022 •

edited

vlio20 Feb 15, 2022

mustafapsd Feb 16, 2022

vlio20 Feb 15, 2022

mustafapsd Feb 16, 2022

vlio20 Feb 15, 2022

mustafapsd Feb 15, 2022

mustafapsd Feb 15, 2022

mustafapsd commented Feb 16, 2022

vlio20 commented Feb 16, 2022

mustafapsd commented Feb 16, 2022

vlio20 commented Feb 17, 2022

mustafapsd commented Feb 17, 2022 •

edited

vlio20 commented Feb 17, 2022

mustafapsd commented Feb 17, 2022

vlio20 commented Feb 18, 2022

mustafapsd commented Feb 18, 2022

feat(dateRangePicker): Adding DateRangePicker feature #590

Are you sure you want to change the base?

feat(dateRangePicker): Adding DateRangePicker feature #590

Conversation

mustafapsd commented Feb 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mustafapsd Feb 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mustafapsd commented Feb 16, 2022

vlio20 commented Feb 16, 2022

mustafapsd commented Feb 16, 2022

vlio20 commented Feb 17, 2022

mustafapsd commented Feb 17, 2022 • edited

vlio20 commented Feb 17, 2022

mustafapsd commented Feb 17, 2022

vlio20 commented Feb 18, 2022

mustafapsd commented Feb 18, 2022

mustafapsd Feb 15, 2022 •

edited

mustafapsd commented Feb 17, 2022 •

edited