Skip to content
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

refactor(dialog): normalize event names #11549

Merged
merged 1 commit into from
Aug 7, 2018

Conversation

crisbeto
Copy link
Member

A while ago we made the output names of components consistent (e.g. #8053 and #8052), however that didn't cover the methods on the dialog. These changes align the dialog event names with the ones on the various components.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 29, 2018
* @deprecated Use `afterOpened` instead.
* @deletion-target 7.0.0
*/
afterOpen(): Observable<void> {
Copy link
Member

Choose a reason for hiding this comment

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

Since this is in cdk-experimental we do not have to go through the deprecation process, we can just change the name.

/**
* Gets an observable that emits when dialog is finished opening.
* @deprecated Use `afterOpened` instead.
* @deletion-target 7.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be 8.0 if we're deprecating it now

@crisbeto
Copy link
Member Author

Addressed the feedback @jelbourn @josephperrott.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels May 31, 2018
@josephperrott josephperrott added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Jun 29, 2018
@josephperrott
Copy link
Member

@crisbeto Can you rebase this? There have been changes to the dialog that introduced new usages of the properties being renamed in this PR.

A while ago we made the output names of components consistent (e.g. angular#8053 and angular#8052), however that didn't cover the methods on the dialog. These changes align the dialog event names with the ones on the various components.
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Jun 30, 2018
@crisbeto
Copy link
Member Author

Rebased and sorted out the compilation error.

@josephperrott josephperrott merged commit 6c3442a into angular:master Aug 7, 2018
@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 Sep 9, 2019
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.

4 participants