feat(datepicker): Add initial view setting #221
Conversation
trevordaniels
force-pushed
the
datepicker-initial-view
branch
from
381f7cc
to
27674ee
Compare
|
Did you check what happens if initialView is not in the keys of config.mappings.changed? Does it behave well under those conditions? Otherwise a check must be added. |
trevordaniels
force-pushed
the
datepicker-initial-view
branch
from
27674ee
to
33531f3
Compare
|
Added a check to ensure the view exists in mappings.changed |
|
Nice, I mentioned it because I had already been thinking about implementing this. Keep it up! |
|
I appreciate your feedback, hope you hadn't started on it! |
| @@ -112,6 +119,13 @@ export class DatepickerPage { | |||
| defaultValue: "0" | |||
| }, | |||
| { | |||
| name: "pickerInitialView", | |||
| type: "CalendarViewType", | |||
| description: "Specifies the initial view for the datepicker. Options are: <code>CalendarViewType.Year</code>, " + | |||
There was a problem hiding this comment.
For the options list, use their string values rather than the enum references
There was a problem hiding this comment.
From my understanding, because CalendarViewType is an enum
export enum CalendarViewType {
Year = 0, ...
}there are no string equivalent values to use. For eg. this is not allowed
<suiDatepicker pickerInitialView="year">To use string values instead of enums, a new type would need to be created similar to DatepickerMode
export type DatepickerMode = "year" | "month" | "date" | "datetime" | "time";
export const DatepickerMode = {
Year: "year" as DatepickerMode, ...
}What is your preferred option?
There was a problem hiding this comment.
Ahh I see, sorry completely forgot the fact that it was just an enum! Turning it into a string enum is definitely the way foward, as then it can be easily used in HTML
There was a problem hiding this comment.
alas TS 2.4.1 support still isn't in Angular when using strict null checks so the "enum" solution is still the best way
| public set initialView(view:CalendarViewType) { | ||
| if (this.changed && this.changed.has(view)) { | ||
| this._initialView = view; | ||
| } |
| @@ -32,6 +33,19 @@ export class SuiDatepickerDirective | |||
| this.onSelectedDateChange.emit(date); | |||
| } | |||
|
|
|||
| private _initialView:CalendarViewType; | |||
| @@ -60,6 +74,9 @@ export class SuiDatepickerDirective | |||
| this.config = new TimeConfig(); | |||
| break; | |||
| } | |||
| if (this._initialView != undefined) { | |||
There was a problem hiding this comment.
Thoughts on making initialView?:CalendarViewType a constructor parameter of CalendarMapping?
There was a problem hiding this comment.
Do you mean a parameter on CalendarMappings? so it can be used like
export class DateMappings extends CalendarMappings {
constructor(initialView?:CalendarViewType) {
super(initialView || CalendarViewType.Date);
There was a problem hiding this comment.
Exactly, though do this instead:
export class DateMappings extends CalendarMappings {
constructor(initialView:CalendarViewType = CalendarViewType.Date) {
super(initialView);| @@ -1,4 +1,5 @@ | |||
| export { | |||
| SuiDatepickerModule, | |||
| DatepickerMode | |||
| DatepickerMode, | |||
| CalendarViewType | |||
There was a problem hiding this comment.
Export as DatepickerViewType for consistency
| @@ -1,6 +1,6 @@ | |||
| import { Component } from "@angular/core"; | |||
| import { ApiDefinition } from "../../../components/api/api.component"; | |||
| import { DatepickerMode } from "../../../../../../src/public"; | |||
| import { DatepickerMode, CalendarViewType } from "../../../../../../src/public"; | |||
There was a problem hiding this comment.
DatepickerViewType (see bottom)
| @@ -112,6 +119,13 @@ export class DatepickerPage { | |||
| defaultValue: "0" | |||
| }, | |||
| { | |||
| name: "pickerInitialView", | |||
| type: "CalendarViewType", | |||
There was a problem hiding this comment.
DatepickerViewType (see bottom)
Partially addresses #165
Not sure if you want to expose CalendarViewType, or create a new export type similar to DatepickerMode