feat(datepicker): Add Custom Header to DatePicker #9639

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

src/demo-app/datepicker/datepicker-demo.ts

-  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

src/lib/datepicker/calendar.ts

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


mat-calendar-header

done in 3467c0a

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

src/lib/datepicker/calendar.ts

@@ -64,6 +77,13 @@ import {Directionality} from '@angular/cdk/bidi';
  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 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

src/lib/datepicker/calendar.ts

+  @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

src/lib/datepicker/calendar.ts

@@ -200,6 +220,17 @@ export class MatCalendar<D> implements AfterContentInit, OnDestroy, OnChanges {
  }

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


I think you can just simplify this logic to:

this.clalendarHeaderPortal = new ComponentPortal(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

src/lib/datepicker/datepicker.ts

@@ -115,6 +115,10 @@ export class MatDatepickerContent<D> implements AfterContentInit {
  preserveWhitespaces: false,
 })
 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

src/lib/datepicker/calendar.html

@@ -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:

<div class="mat-calendar-controls"> <button mat-button class="mat-calendar-period-button" (click)="_currentPeriodClicked()" [attr.aria-label]="_periodButtonLabel"> {{_periodButtonText}} <div class="mat-calendar-arrow" [class.mat-calendar-invert]="_currentView != 'month'"></div> </button> <div class="mat-calendar-spacer"></div> <button mat-icon-button class="mat-calendar-previous-button" [disabled]="!_previousEnabled()" (click)="_previousClicked()" [attr.aria-label]="_prevButtonLabel"> </button> <button mat-icon-button class="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

src/lib/datepicker/calendar.ts


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


/** Default header for MatCalendar */

done in 0faefc4

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

src/demo-app/datepicker/datepicker-demo.ts

@@ -32,10 +34,24 @@ export class DatepickerDemo {
  lastDateChange: Date | null;

  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

src/lib/datepicker/calendar.ts

+  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

src/demo-app/datepicker/custom-header.html

+<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

src/demo-app/datepicker/datepicker-demo.ts

+
+  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

src/demo-app/datepicker/custom-header.scss

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


Extra new line.

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

src/lib/datepicker/calendar.ts

+})
+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:master Mar 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-bot bot locked and limited conversation to collaborators Sep 8, 2019

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

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

feat(datepicker): Add Custom Header to DatePicker #9639

feat(datepicker): Add Custom Header to DatePicker #9639

Conversation

tobiasschweizer commented Jan 27, 2018 • edited by mmalerba Loading

googlebot commented Jan 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdricChan03 commented Jan 29, 2018

tobiasschweizer commented Jan 29, 2018

mmalerba commented Jan 29, 2018

mmalerba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasschweizer Mar 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasschweizer commented Mar 21, 2018

tobiasschweizer commented Mar 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 8, 2019

tobiasschweizer commented Jan 27, 2018 •

edited by mmalerba

Loading

tobiasschweizer Mar 4, 2018 •

edited

Loading