Skip to content

Commit ebc97da

Browse files
committed
refactor(drp): address comments
1 parent 8353984 commit ebc97da

File tree

4 files changed

+102
-101
lines changed

4 files changed

+102
-101
lines changed

src/components/date-range-picker/date-range-picker.spec.ts

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import { DateTimeUtil } from '../date-time-input/date-util.js';
3333
import type IgcDialogComponent from '../dialog/dialog.js';
3434
import IgcInputComponent from '../input/input.js';
3535
import IgcDateRangePickerComponent, {
36-
type CustomDateRange,
3736
type DateRangeValue,
3837
} from './date-range-picker.js';
3938

@@ -675,7 +674,7 @@ describe('Date range picker', () => {
675674
await elementUpdated(picker);
676675

677676
expect(eventSpy).not.called;
678-
expect(picker.value).to.deep.equal({ start: null, end: null });
677+
expect(picker.value).to.deep.equal(null);
679678
expect(dateTimeInputs[0].value).to.be.null;
680679
expect(dateTimeInputs[1].value).to.be.null;
681680
});
@@ -952,25 +951,25 @@ describe('Date range picker', () => {
952951
});
953952
});
954953
describe('Selection via the range selection chips', () => {
955-
const previousThreeDaysStart = CalendarDay.today.add('day', -3).native;
956-
const nextThreeDaysEnd = CalendarDay.today.add('day', 3).native;
957-
958-
const customRanges: CustomDateRange[] = [
959-
{
960-
label: 'Previous Three Days',
961-
dateRange: {
962-
start: previousThreeDaysStart,
963-
end: today.native,
964-
},
965-
},
966-
{
967-
label: 'Next Three Days',
968-
dateRange: {
969-
start: today.native,
970-
end: nextThreeDaysEnd,
971-
},
972-
},
973-
];
954+
//const previousThreeDaysStart = CalendarDay.today.add('day', -3).native;
955+
//const nextThreeDaysEnd = CalendarDay.today.add('day', 3).native;
956+
957+
// const customRanges: CustomDateRange[] = [
958+
// {
959+
// label: 'Previous Three Days',
960+
// dateRange: {
961+
// start: previousThreeDaysStart,
962+
// end: today.native,
963+
// },
964+
// },
965+
// {
966+
// label: 'Next Three Days',
967+
// dateRange: {
968+
// start: today.native,
969+
// end: nextThreeDaysEnd,
970+
// },
971+
// },
972+
// ];
974973

975974
it('should not render any chips when usePredefinedRanges is false and there are no custom ranges added', async () => {
976975
picker.open = true;
@@ -988,7 +987,7 @@ describe('Date range picker', () => {
988987
);
989988

990989
picker.open = true;
991-
const predefinedRanges = (picker as any).predefinedRanges;
990+
const predefinedRanges = (picker as any)._predefinedRanges;
992991
await elementUpdated(picker);
993992

994993
const chips = picker.renderRoot.querySelectorAll('igc-chip');
@@ -1252,7 +1251,7 @@ describe('Date range picker', () => {
12521251
await elementUpdated(picker);
12531252

12541253
expect(eventSpy).not.to.be.called;
1255-
expect(picker.value).to.deep.equal({ start: null, end: null });
1254+
expect(picker.value).to.deep.equal(null);
12561255
});
12571256

12581257
it('should open the picker on calendar show icon click in dropdown mode', async () => {
@@ -1330,10 +1329,10 @@ describe('Date range picker', () => {
13301329

13311330
expect(isFocused(input)).to.be.false;
13321331
expect(eventSpy).to.be.calledWith('igcChange', {
1333-
detail: { start: null, end: null },
1332+
detail: null,
13341333
});
13351334
expect(picker.open).to.be.false;
1336-
expect(picker.value).to.deep.equal({ start: null, end: null });
1335+
expect(picker.value).to.deep.equal(null);
13371336
expect(input.value).to.equal('');
13381337
});
13391338

src/components/date-range-picker/date-range-picker.ts

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import {
5353
createCounter,
5454
equal,
5555
findElementFromEventPath,
56+
isEmpty,
5657
last,
5758
} from '../common/util.js';
5859
import IgcDateTimeInputComponent from '../date-time-input/date-time-input.js';
@@ -339,11 +340,7 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
339340
}
340341

341342
public get value(): DateRangeValue | null {
342-
// TODO what should the default value be? null?
343-
return {
344-
start: this._formValue?.value?.start ?? null,
345-
end: this._formValue?.value?.end ?? null,
346-
};
343+
return this._formValue.value;
347344
}
348345
/**
349346
* Renders chips with custom ranges based on the elements of the array.
@@ -627,7 +624,10 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
627624
constructor() {
628625
super();
629626
this._formValue = createFormValueState<DateRangeValue | null>(this, {
630-
initialValue: null,
627+
initialValue: {
628+
start: null,
629+
end: null,
630+
},
631631
transformers: defaultDateRangeTransformers,
632632
});
633633

@@ -651,11 +651,6 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
651651

652652
protected override formResetCallback() {
653653
super.formResetCallback();
654-
this._formValue.setValueAndFormState({
655-
start: this._formValue.defaultValue?.start ?? null,
656-
end: this._formValue.defaultValue?.end ?? null,
657-
});
658-
659654
this._setCalendarRangeValues();
660655
this._updateMaskedRangeValue();
661656
}
@@ -858,13 +853,18 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
858853
: { start: currentStart, end: newValue };
859854
}
860855

861-
// delegate the inputs validity state to the date-range-picker
856+
// Delegates the validity methods of internal input elements
857+
// to the component's own validation logic specific to date-range values.
858+
// Checks for dirty state to avoid unnecessary validation on form reset,
859+
// caused by the inputs value being set.
862860
private _delegateInputsValidity() {
863861
const inputs = this.useTwoInputs ? this._inputs : [this._input];
864862

865863
inputs.forEach((input) => {
866-
input.checkValidity = () => this.checkValidity();
867-
input.reportValidity = () => this.reportValidity();
864+
input.checkValidity = () =>
865+
this._dirty || !this._pristine ? this.checkValidity() : true;
866+
input.reportValidity = () =>
867+
this._dirty || !this._pristine ? this.reportValidity() : true;
868868
});
869869
}
870870

@@ -890,11 +890,11 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
890890
this._inputs[1].max = this._max;
891891
}
892892
}
893-
if (this._disabledDates?.length > 0) {
893+
if (!isEmpty(this.disabledDates)) {
894894
dates.push(...this.disabledDates);
895895
}
896896

897-
this._dateConstraints = dates.length > 0 ? dates : [];
897+
this._dateConstraints = isEmpty(dates) ? [] : dates;
898898
}
899899

900900
private _updateMaskedRangeValue() {
@@ -1066,8 +1066,9 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
10661066
return nothing;
10671067
}
10681068

1069-
const hasHeaderDate =
1070-
this._headerDateSlotItems.length > 0 ? 'header-date' : '';
1069+
const hasHeaderDate = isEmpty(this._headerDateSlotItems)
1070+
? ''
1071+
: 'header-date';
10711072

10721073
return html`
10731074
<slot name="title" slot="title">
@@ -1110,13 +1111,13 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
11101111

11111112
protected renderActions() {
11121113
const slot =
1113-
this._isDropDown || this._actions.length === 0 ? undefined : 'footer';
1114+
this._isDropDown || isEmpty(this._actions) ? undefined : 'footer';
11141115

11151116
// If in dialog mode use the dialog footer slot
11161117
return html`
11171118
<div
11181119
part="actions"
1119-
?hidden=${this._actions.length === 0}
1120+
?hidden=${isEmpty(this._actions)}
11201121
slot=${ifDefined(slot)}
11211122
>
11221123
<slot name="actions"></slot>
@@ -1262,12 +1263,12 @@ export default class IgcDateRangePickerComponent extends FormAssociatedRequiredM
12621263
${this.renderCalendarIcon()}
12631264
<slot
12641265
name="prefix"
1265-
slot=${ifDefined(this._prefixes.length > 0 ? 'prefix' : undefined)}
1266+
slot=${ifDefined(isEmpty(this._prefixes) ? undefined : 'prefix')}
12661267
></slot>
12671268
${this._renderClearIcon()}
12681269
<slot
12691270
name="suffix"
1270-
slot=${ifDefined(this._suffixes.length > 0 ? 'suffix' : undefined)}
1271+
slot=${ifDefined(isEmpty(this._suffixes) ? undefined : 'suffix')}
12711272
></slot>
12721273
</igc-input>
12731274
${this.renderHelperText()} ${this.renderPicker(id)}`;

src/components/date-range-picker/validators.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { calendarRange, isDateInRanges } from '../calendar/helpers.js';
2+
import { CalendarDay } from '../calendar/model.js';
23
import type { DateRangeDescriptor } from '../calendar/types.js';
34
import messages from '../common/localization/validation-en.js';
45
import { formatString, last } from '../common/util.js';
@@ -68,13 +69,13 @@ export const badInputDateRangeValidator: Validator<{
6869
key: 'badInput',
6970
message: ({ value }) => formatString(messages.disabledDate, value),
7071
isValid: ({ value, disabledDates }) => {
71-
if (!value?.start || !value?.end || !disabledDates) {
72+
if (!(value?.start && value?.end && disabledDates)) {
7273
return true;
7374
}
7475

75-
let range = [];
76+
let range: CalendarDay[] = [];
7677
if (value.start.getTime() === value.end.getTime()) {
77-
range = [value.start];
78+
range = [CalendarDay.from(value.start)];
7879
} else {
7980
range = Array.from(calendarRange({ start: value.start, end: value.end }));
8081
const lastDay = last(range);
@@ -83,8 +84,8 @@ export const badInputDateRangeValidator: Validator<{
8384
}
8485
}
8586

86-
for (let i = 0; i < range.length; i++) {
87-
if (isDateInRanges(range[i], disabledDates)) {
87+
for (const dateRange of range) {
88+
if (isDateInRanges(dateRange, disabledDates)) {
8889
return false;
8990
}
9091
}

0 commit comments

Comments
 (0)