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.

Sign up for GitHub

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

Jump to bottom

feat(sidenav): emit event when backdrop is clicked #1638

Merged
merged 2 commits into from
Dec 1, 2016
Merged

feat(sidenav): emit event when backdrop is clicked #1638

merged 2 commits into from
Dec 1, 2016

Conversation

ianjoneill
Copy link
Contributor

This allows clients to distinguish between close events caused by calling close() and those caused by the backdrop being clicked.

Closes #1427

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). 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.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). 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.

@googlebot googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Oct 28, 2016
@ianjoneill
Copy link
Contributor Author

I resigned the CLA for my additional email address.

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@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 Oct 28, 2016
@jelbourn jelbourn added the needs: discussion Further discussion with the team is needed before proceeding label Nov 9, 2016
@ianjoneill
Copy link
Contributor Author

Rebased onto current master branch to get alpha-10 goodness and resolve a merge conflict.

mmalerba
mmalerba reviewed Nov 29, 2016
View reviewed changes
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.

Sorry this took so long, wasn't sure if we wanted to do this or go with a larger rewrite of the sidenav.

src/lib/sidenav/sidenav.ts
@@ -398,6 +401,8 @@ export class MdSidenavLayout implements AfterContentInit {
}

_closeModalSidenav() {
this.onBackdropClicked.emit(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to pass null, can just call emit()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, looks like that parameter was made optional in Angular 2 RC5.

Done

src/lib/sidenav/sidenav.ts
@@ -398,6 +401,8 @@ export class MdSidenavLayout implements AfterContentInit {
}

_closeModalSidenav() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would create a new method _onBackdropClicked that emits and then calls this, we will probably add other bindings that call this function later (e.g. esc key).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ianjoneill
feat(sidenav): emit event when backdrop is clicked
d6290d0 
This allows clients to distinguish between close events caused by
calling close() and those caused by the backdrop being clicked.

Closes angular#1427
@ianjoneill
Address review comments
8353609
mmalerba
mmalerba approved these changes Nov 29, 2016
View reviewed changes
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 pr: lgtm action: merge The PR is ready for merge by the caretaker and removed needs: discussion Further discussion with the team is needed before proceeding labels Nov 29, 2016
@tinayuangao tinayuangao merged commit 93807ed into angular:master Dec 1, 2016
@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 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mmalerba mmalerba mmalerba approved these changes

Assignees

@mmalerba mmalerba

Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Freature Request: callback when backdrop is clicked for md-sidenav
5 participants
@ianjoneill @googlebot @mmalerba @jelbourn @tinayuangao