Skip to content

Conversation

fhuschle
Copy link
Contributor

@fhuschle fhuschle commented Sep 1, 2020

Adds viewChanged output to calendar so you can react if the view changed between 'month' | 'year' | 'multi-year'.
Fixes #13759.

@fhuschle fhuschle requested a review from mmalerba as a code owner September 1, 2020 10:46
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). 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 (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Sep 1, 2020
@fhuschle
Copy link
Contributor Author

fhuschle commented Sep 1, 2020

@googlebot I signed it!

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@fhuschle fhuschle force-pushed the datepicker-view-changed branch from ee907d6 to 7b1c1a0 Compare September 1, 2020 11:25
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Sep 1, 2020
@fhuschle fhuschle force-pushed the datepicker-view-changed branch from 7b1c1a0 to 06baa01 Compare September 1, 2020 13:08
@@ -299,6 +305,9 @@ export class MatCalendar<D> implements AfterContentInit, AfterViewChecked, OnDes
/** Whether the calendar is in month view. */
get currentView(): MatCalendarView { return this._currentView; }
set currentView(value: MatCalendarView) {
if (this._currentView !== value) {
this.viewChanged.emit(value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we emit here, the event will come before the UI has actually updated. I think people would expect this event to come after the view has updated. I think a Promise.resolve().then() should make this emit after, if not maybe a setTimeout(). Please also add a test that verifies this

Copy link
Member

@crisbeto crisbeto Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Event emitters have an isAsync flag which we could use here? E.g. new EventEmitter(true).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fhuschle can you try removing the setTimeout and passing true when constructing the EventEmitter? If that works we should use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to work the same way.

@fhuschle fhuschle force-pushed the datepicker-view-changed branch from 06baa01 to 1547aee Compare September 2, 2020 19:49
@fhuschle fhuschle force-pushed the datepicker-view-changed branch from 1547aee to 26d6ef0 Compare September 13, 2020 14:08
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mmalerba mmalerba added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Sep 14, 2020
@wagnermaciel wagnermaciel merged commit dfdf5cf into angular:master Sep 16, 2020
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fr(datepicker): add viewChanged output to calendar
5 participants