-
Notifications
You must be signed in to change notification settings - Fork 665
Calendar: improve Views and Selection strategies types #30688
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
Conversation
| const localizedWidgetName = this._getLocalizedWidgetName(); | ||
| // @ts-expect-error ts-error | ||
| const [startDate, endDate] = value; | ||
| const [startDate, endDate] = value as [Date | undefined, Date | undefined]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dateLike[]?
Maybe we can even extract a new type like DateRange? Because I see you use this custom type in several places.
I remember I've tried to do it in .d.ts some time ago but didin't manage to do this because IT does not support ts tuples. But maybe we can use it for the code at least
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's more like local hack here, tbh i think we can even remove undefined, updated the pr. I don't think we should extract a separate type, since it's only actual in couple place
dateLike[]?
no, in views we don't use dateLike type, since value is already converted from string or number when it passed to Views
packages/devextreme/js/__internal/ui/calendar/m_calendar.range.selection.strategy.ts
Show resolved
Hide resolved
| } | ||
|
|
||
| _getDaysInRange(startDate, endDate) { | ||
| _getDaysInRange(startDate: Date | null, endDate: Date | null): Date[] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get return type from calendar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nah, don't think we should, it's more like local for range selection
packages/devextreme/js/__internal/ui/calendar/m_calendar.range.selection.strategy.ts
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/ui/calendar/m_calendar.single.selection.strategy.ts
Outdated
Show resolved
Hide resolved
packages/devextreme/js/__internal/ui/calendar/m_calendar.base_view.ts
Outdated
Show resolved
Hide resolved
…proveTsCalendarViews__25_2
No description provided.