-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
feat(datepicker): add month & year view #2904
Conversation
}); | ||
|
||
it('handles month and date underflow', () => { | ||
expect(new SimpleDate(2017, -1, 0)).toEqual(new SimpleDate(2016, 10, 30)); |
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.
Is it (2016, 10, 31)?
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.
The months are 0-indexed to match native JS Dates, so 10 = November. I considered using 1-indexed because it make more sense, but I thought it was better to just stay consistent with what people already know. I'll add a note about the 0-indexing to the SimpleDate docs.
}); | ||
|
||
it('highlights selected', () => { | ||
let todayCell = calendarTableNativeElement.querySelector('.mat-calendar-table-selected'); |
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.
very nit: selectedCell?
TestBed.compileComponents(); | ||
})); | ||
|
||
describe('standard year view', () => { |
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.
nit: standard month view
src/lib/datepicker/month-view.ts
Outdated
* The date of the month that the currently selected Date falls on. | ||
* Null if the currently selected Date is in another month. | ||
*/ | ||
_selectedDate = 0; |
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.
Do we want to set default value to null
?
Add type for this variable?
|
||
constructor(private _locale: CalendarLocale) { | ||
// First row of months only contains 5 elements so we can fit the year label on the same row. | ||
this._months = [[0, 1, 2, 3, 4], [5, 6, 7, 8, 9, 10, 11]].map(row => row.map( |
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.
months value should start from 0 or 1? could you please add some docs?
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.
LGTM
src/lib/core/datetime/simple-date.ts
Outdated
return this._date.getMonth(); | ||
} | ||
|
||
/** The date component of this 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.
nit: Also add doc about date index?
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.
Nits
static fromNativeDate(nativeDate: Date) { | ||
return new SimpleDate(nativeDate.getFullYear(), nativeDate.getMonth(), nativeDate.getDate()); | ||
} | ||
|
||
constructor(public year: number, public month: number, public date: number) {} | ||
/** Creates a SimpleDate object representing today. */ | ||
static today() { |
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.
Return type? Same for methods below.
|
||
.mat-calendar-table-label { | ||
height: $mat-calendar-table-cell-content-size; | ||
padding: 0 0 0 10px; |
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.
Nit: Sass variable for 10px? Seems like the only one missing.
expect(labelEls.length).toBe(1); | ||
expect(cellEls.length).toBe(11); | ||
expect(rowEls[0].firstElementChild.classList.contains('mat-calendar-table-label')).toBe( | ||
true, 'first cell should be the label'); |
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.
This might be a bit more readable if you use toContain()
assertion
expect(rowEls[0].firstElementChild.classList)
.toContain('mat-calendar-table-label', 'First cell should be the label.');
this._date = this._locale.parseDate(value) || SimpleDate.today(); | ||
this._init(); | ||
} | ||
private _date = SimpleDate.today(); |
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 you stack all the private properties at the top of the class? That's typically the format we use in other components so it's easy to find them.
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.
When it's acting as backing for a public property with a getter/setter isn't it nicer to have it next to its getter/setter?
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
* date picker initial commit * month view * added month view functionality * more month view tweaking * started extracting common stuff to calendar-table. * base month view on calendar table * added year view * add disclaimers * addressed comments * fix lint * fixed aot and added comment * started on tests * calendar table tests * tests for month and year view * rebase on top of CalendarLocale & SimpleDate * add some additional functionality to SimpleDate * fix lint * addressed comments * add comment
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.