feat(datepicker): Add Custom Header to DatePicker by tobiasschweizer · Pull Request #9639 · angular/components

tobiasschweizer · 2018-01-27T13:05:27Z

This PR provides a new input for DatePicker so that a custom header component can be added to mat-calendar.

I intend to add a custom header from which date formats can be converted (see #2519).

TODO: the custom header should be passed from datepicker

- TODO: solve problem with directive cdkPortalOutlet in calendar.html

…r component

googlebot · 2018-01-27T13:05:32Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

EdricChan03 · 2018-01-27T15:23:00Z

- templateUrl: 'datepicker-demo.html',
- styleUrls: ['datepicker-demo.css'],
- changeDetection: ChangeDetectionStrategy.OnPush,
+moduleId: module.id,


Your formatting should be 2 spaces instead of a tab or four characters.

@Chan4077 Thanks for pointing that out. I changed the indentation to two spaces, undoing the changes introduced previously thereby.

EdricChan03 · 2018-01-29T10:31:16Z

@tobiasschweizer Could you sign the CLA?

tobiasschweizer · 2018-01-29T14:45:42Z

I did on behalf of my organization (https://github.com/dhlab-basel). Probably my boss needs to confirm it.

mmalerba · 2018-01-29T16:34:42Z

@tobiasschweizer you need to sign it with the same account you used to make the git commits

mmalerba

@tobiasschweizer try running the following git command:

git config user.email

and make sure you've signed with that email

mmalerba · 2018-01-30T16:11:33Z

+ * Default header of a [MatCalendar].
+ */
+@Component({
+ selector: 'default-header',


mat-calendar-header

done in 3467c0a

mmalerba · 2018-01-30T16:12:22Z

 export class MatCalendar<D> implements AfterContentInit, OnDestroy, OnChanges {
+
+ /** An input indicating the type of the custom header component, if set. */
+ @Input() customCalendarHeaderComponent: ComponentType<any>;


lets just call the input headerComponent (we know its for the calendar since we're in the calendar component and the "custom" doesn't add much useful info for the user)

done in 3467c0a

mmalerba · 2018-01-30T16:12:59Z

+ @Input() customCalendarHeaderComponent: ComponentType<any>;
+
+ /** A portal containing the header for this calendar. */
+ calendarHeaderPortal: Portal<any>;


name this with a leading _ to indicate internal

done in 3467c0a
I renamed it as you suggested, made it private and added a getter method since the template needs to access it. To my understanding, private members are not accessible from the component's template (although this would make sense, conceptually speaking): https://stackoverflow.com/questions/34574167/angular2-should-private-variables-be-accessible-in-the-template?rq=1

Rather than a getter method just name it with an _ and leave it public. We use this convention throughout Angular Material to mean "we had to leave this public due to technical constraints, but you shouldn't mess with it". We also automatically strip _ properties when generating our docs.

Got it!
done in b6938f2

mmalerba · 2018-01-30T16:15:17Z


 ngAfterContentInit() {
+
+ if (this.customCalendarHeaderComponent !== undefined) {


I think you can just simplify this logic to:
this.clalendarHeaderPortal=newComponentPortal(this.calendarHeaderComponent||MatCalendarHeader);

done in 3467c0a
In general I try to be as specific as possible and not rely on implicit type conversions. JavaScript may behave very unexpectedly in some cases.
This is my understanding of the construct:
If this.calendarHeaderComponent is set, we expect it to be evaluated to true and thus returned. If not (undefined), to false and thus MatCalendarHeader to be returned.

mmalerba · 2018-01-30T16:16:53Z

 export class MatDatepicker<D> implements OnDestroy {
+
+ /** An input indicating the type of the custom header component for the calendar, if set. */
+ @Input() customCalendarHeaderComponent: ComponentType<any>;


calendarHeaderComponent

done in 014777a

…ents

mmalerba · 2018-01-31T19:28:29Z

@@ -1,4 +1,7 @@
 <div class="mat-calendar-header">
+
+ <ng-template [cdkPortalOutlet]="calendarHeaderPortal"></ng-template>


Are you planning to move the .mat-calendar-controls below into the mat-calendar-header? That way we can allow people to swap out the default controls for a custom set

That's a good idea.
In that case it would make sense to add them to mat-calendar-header (component MatCalendarHeader) which is the default.
So the template of MatCalendarHeader should contain:
<divclass="mat-calendar-controls"><buttonmat-buttonclass="mat-calendar-period-button" (click)="_currentPeriodClicked()" [attr.aria-label]="_periodButtonLabel"> {{_periodButtonText}} <divclass="mat-calendar-arrow" [class.mat-calendar-invert]="_currentView != 'month'"></div></button><divclass="mat-calendar-spacer"></div><buttonmat-icon-buttonclass="mat-calendar-previous-button" [disabled]="!_previousEnabled()" (click)="_previousClicked()" [attr.aria-label]="_prevButtonLabel"></button><buttonmat-icon-buttonclass="mat-calendar-next-button" [disabled]="!_nextEnabled()" (click)="_nextClicked()" [attr.aria-label]="_nextButtonLabel"></button></div>
And then we could remove that code from calendar.html since it will be inserted by <ng-template [cdkPortalOutlet]="_calendarHeaderPortal"></ng-template>.
However, if someone provides a custom header, she would have to take care of these controls.
Could we make more sub-components for the buttons so they could be combined in a more modular way?
Could we make components for the buttons (previous, next)? But how would that work with the callback methods that have to be called on MatCalendar?

Oh sorry, I missed this comment somehow. Yeah, that's pretty much what I was thinking. We could package the buttons up into their own components, but it seems a little bit overkill to me since they're not very complex. We will need to make some of the internal properties and methods on calendar public (remove the _) to indicate that its ok for people to inject the calendar and call those methods in their custom header

@mmalerba done in 528361e
One test is failing in src/lib/datepicker/calendar.spec.ts:
it('should re-render when the i18n labels have changed', inject([MatDatepickerIntl], (intl: MatDatepickerIntl) => { const button = fixture.debugElement.nativeElement .querySelector('.mat-calendar-period-button'); intl.switchToMultiYearViewLabel = 'Go to multi-year view?'; intl.changes.next(); fixture.detectChanges(); expect(button.getAttribute('aria-label')).toBe('Go to multi-year view?'); }) );
And another test does not pass in src/lib/select/select.spec.ts:
it('should emit to `optionSelectionChanges` when an option is selected', fakeAsync(() => { trigger.click(); fixture.detectChanges(); flush(); const spy = jasmine.createSpy('option selection spy'); const subscription = fixture.componentInstance.select.optionSelectionChanges.subscribe(spy); const option = overlayContainerElement.querySelector('mat-option') as HTMLElement; option.click(); fixture.detectChanges(); flush(); expect(spy).toHaveBeenCalledWith(jasmine.any(MatOptionSelectionChange)); subscription.unsubscribe(); }));
I assume that this related to the use of querySelector that tries to get an element that was moved into another template (at least in the first case). I do not know why the second test fails. I cannot find the element in the MatCalendarHeader template. But maybe it is the menu that pops up.

mmalerba · 2018-01-31T19:29:24Z

+import {ComponentPortal, ComponentType, Portal} from '@angular/cdk/portal';

+/**
+ * Default header of a [MatCalendar].


/** Default header for MatCalendar */

done in 0faefc4

mmalerba · 2018-01-31T19:30:04Z


 dateFilter =
-(date: Date) => !(date.getFullYear() % 2) && (date.getMonth() % 2) && !(date.getDate() % 2)
+ (date: Date) => !(date.getFullYear() % 2) && (date.getMonth() % 2) && !(date.getDate() % 2)


nit: continuation lines indented 2

done in 0faefc4

tobiasschweizer · 2018-03-21T17:28:13Z

@mmalerba Thanks, that looks better now ;-)

tobiasschweizer · 2018-03-21T17:55:17Z

@mmalerba Ok, everything is green now. Thx.!

mmalerba · 2018-03-21T19:00:37Z

+ private _destroyed = new Subject<void>();
+
+ constructor(private _intl: MatDatepickerIntl,
+ @Host() public calendar: MatCalendar<D>,


@tobiasschweizer I'm seeing a number of failures when trying to presubmit this related to injecting the calendar here. I think you may need to do this:
@Host() @Inject(forwardRef(()=>MatCalendar))public calendar: MatCalendar<D>
Since MatCalendarHeader is declared before MatCalendar

@mmalerba Good point, which test gave you the error?
fixed in 822dd56

Tests in Google's codebase for other project teams. I can't link to them since they're internal, I'll rerun the presubmit with the fix though.
In the mean time, it looks like you need to resolve some conflicts again.

ok, merge done in ba4e839

# Conflicts: # src/lib/datepicker/datepicker-module.ts # src/lib/datepicker/datepicker.ts

rafaelss95 · 2018-03-21T21:53:07Z

+<div class="custom-header">
+ <button mat-icon-button (click)="previousClicked('year')">&lt;&lt;</button>
+ <button mat-icon-button (click)="previousClicked('month')">&lt;</button>
+ <span class="custom-header-label">{{periodLabel}}</span>


Shouldn’t be only 2 spaces?

rafaelss95 · 2018-03-21T21:54:43Z

+
+ get periodLabel() {
+ let year = this._dateAdapter.getYearName(this._calendar.activeDate);
+ let month = (this._dateAdapter.getMonth(this._calendar.activeDate) + 1);


Both variables could be const.

rafaelss95 · 2018-03-21T21:59:51Z

+ flex: 1;
+ text-align: center;
+}
+


Extra new line.

rafaelss95 · 2018-03-21T22:02:06Z

+})
+export class MatCalendarHeader<D> implements OnDestroy {
+ /** Subject that emits when the component has been destroyed. */
+ private _destroyed = new Subject<void>();


Shouldn’t it be readonly, @jelbourn?

I have seen some of the _destroyed subjects in our code marked readonly, but to me it feels unnecessary for private properties.

…ontent This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since angular#9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.

…ontent This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since #9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.

…ontent This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since angular#9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.

…ontent This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since #9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.

…ontent This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since angular#9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.

…ontent This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since #9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.

…ontent This is something I ran into while working on aligning the datepicker with the most-recent Material design spec. Since angular#9639 we use a portal outlet to render the calendar header. The portal outlet directive will detach in `ngOnDestroy` and it won't wait for the parent animation to finish, which ends up shifting the entire calendar up while it's animating away. The only reason that this isn't visible at the moment is because the current animation isn't configured correctly, which causes it to go to `opacity: 0` immediately.

angular-automatic-lock-bot · 2019-09-08T13:02:31Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

Tobias Schweizer added 10 commits January 26, 2018 15:25

feature (add custom header to mat-calendar)
6251e1e
TODO: the custom header should be passed from datepicker

refactor (define custom header component)
8d30140

refactor (remove useless text)
81c89fa

refactor (pass custom header component type as input)
737a563

refactor (add header as component portal)
998be7c

refactor (custom header as component portal)
a14dbd4
- TODO: solve problem with directive cdkPortalOutlet in calendar.html

refactor (switch for custom header component)
585407f

refactor (define entry components)
6668ae6

fix (correct entry point)
f9628a8

refactor (custom header component): add test function to custom heade…
050904b
…r component

tobiasschweizer requested review from jelbourn and mmalerba as code owners January 27, 2018 13:05

googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Jan 27, 2018

refactor (some code formatting)
a2db283

EdricChan03 reviewed Jan 27, 2018
View reviewed changes

refactor (set indentation to two spaces)
7f071ec

mmalerba self-assigned this Jan 29, 2018

mmalerba reviewed Jan 30, 2018
View reviewed changes

Tobias Schweizer added 4 commits January 30, 2018 17:41

refactor (formatting)
5c9f662

refactor (custom header for mat-calendar): renaming and code improvem…
3467c0a
…ents

refactor (calendarHeaderComponent) rename input of MatDatepicker
014777a

refactor (calendarHeaderComponent): make a demo for custom header
179c9af

mmalerba reviewed Jan 31, 2018
View reviewed changes

mmalerba mentioned this pull request Feb 1, 2018
feat(datepicker): @Output for year and month selected in multiyear/year #9678
Merged

Tobias Schweizer added 2 commits February 1, 2018 12:05

refactor (calendar header): make calendar header portal variable public
b6938f2

refactor (calendar header): formatting
0faefc4

mmalerba approved these changes Mar 21, 2018
View reviewed changes

Tobias Schweizer added 2 commits March 21, 2018 21:19

fix (MatCalendarHeader): correct forward reference to MatCalendar
822dd56

Merge branch 'master' into tobiasschweizer/calendar-header-comp
ba4e839
# Conflicts: # src/lib/datepicker/datepicker-module.ts # src/lib/datepicker/datepicker.ts

rafaelss95 reviewed Mar 21, 2018
View reviewed changes

mmalerba added 2 commits March 21, 2018 16:32

Add stateChanges to MatCalendar
1ee0f93

fix nits
aff641d

mmalerba merged commit 4e1bb26 into angular:masterMar 22, 2018

tobiasschweizer mentioned this pull request Mar 24, 2018
Support for Hijri or Jalali or other international calendar in datepicker #2519
Closed

tobiasschweizer deleted the tobiasschweizer/calendar-header-comp branch March 27, 2018 13:59

rafaelss95 mentioned this pull request Apr 8, 2018
chore(calendar): remove explicit preserveWhitespaces setting #10761
Merged

crisbeto mentioned this pull request Jul 30, 2018
fix(datepicker): wait for exit animation to finish before detaching content #12440
Closed

angular-automatic-lock-botBot locked and limited conversation to collaborators Sep 8, 2019


		ngAfterContentInit() {

		if (this.customCalendarHeaderComponent !== undefined) {

		@@ -1,4 +1,7 @@
		<div class="mat-calendar-header">

		<ng-template [cdkPortalOutlet]="calendarHeaderPortal"></ng-template>

Uh oh!

Conversation

tobiasschweizer commented Jan 27, 2018• edited by mmalerba Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Jan 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EdricChan03 commented Jan 29, 2018

Uh oh!

tobiasschweizer commented Jan 29, 2018

Uh oh!

mmalerba commented Jan 29, 2018

Uh oh!

mmalerba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasschweizerMar 4, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasschweizer commented Mar 21, 2018

Uh oh!

tobiasschweizer commented Mar 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobiasschweizer commented Jan 27, 2018•
edited by mmalerba
Loading

tobiasschweizerMar 4, 2018•
edited
Loading