feat(datepicker): add ng-content to datepicker header #13236
Conversation
- add ng-content to datepicker header to simplify customization of this component - fix tests using calendarHeaderComponent property to correctly reference the custom component - add test to prove that custom component was rendered Closes angular#13212
googlebot
added
the
cla: yes
| <mat-calendar-header #header> | ||
| <button mat-button type="button" class="mat-calendar-today-button" | ||
| (click)="todayClicked()" [attr.aria-label]="todayButtonLabel" | ||
| cdkAriaLive="polite"> |
There was a problem hiding this comment.
The button shouldn't have cdkAriaLive.
There was a problem hiding this comment.
Should I remove attr.aria-label as well? To be honest I was just following suit based on the buttons in mat-calendar-header
(https://github.com/angular/material2/blob/master/src/lib/datepicker/calendar-header.html#L4-L5)
There was a problem hiding this comment.
I've gone ahead and removed it for brevity
| })); | ||
| }); | ||
|
|
||
| describe('datepicker with custom header using ng-content', () => { |
There was a problem hiding this comment.
I'm not sure whether we need a test for this. It's basically testing if ng-content works which isn't something Material is responsible for.
There was a problem hiding this comment.
I agree with this comment for the datepicker with custom header using ng-content suite I added and will remove.
However I think the additional test that checks that the custom element is added is useful for the datepicker with custom header suite. Prior to my change to DatepickerWithCustomHeader a custom header component was not in fact being used by that test suite.
- remove unnecessary tests - simplify demo code
crisbeto
added
pr: lgtm
action: merge
|
I'd like @mmalerba to weigh in on this I'm a little iffy about projecting all of the content. |
|
I think this should be fine, we won't project anything in the default case, but if people are using the |
* feat(datepicker): add ng-content to datepicker header - add ng-content to datepicker header to simplify customization of this component - fix tests using calendarHeaderComponent property to correctly reference the custom component - add test to prove that custom component was rendered Closes angular#13212 * test(datepicker): code review fixes - remove unnecessary tests - simplify demo code
|
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. |
customization of this component
to correctly reference the custom component
rendered
Closes #13212