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

feat(dialog): support using dialog content directives with template dialogs #9379

Merged
merged 1 commit into from
Jan 24, 2018

Conversation

crisbeto
Copy link
Member

Previously the matDialogClose, matDialogTitle etc. directives would only work correctly inside component dialogs, because using DI to get the dialog ref doesn't work inside template dialogs. These changes add a fallback that finds the dialog ref based on the id of the closest dialog container.

Fixes #5412.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 13, 2018
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, one small comment

if (!this.dialogRef) {
// If we couldn't get a dialog ref through DI, try to find one by looking at the DOM.
// This can happen if the close button is used inside a template dialog. Note that
// we have to do the lookup in OnInit, because the constructor might be too early.
Copy link
Member

Choose a reason for hiding this comment

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

Small comment suggestion:

// When this directive is included in a dialog via TemplateRef (rather than being
// in a Component), the DialogRef isn't available via injection because embedded
// views cannot be given a custom injector. Instead, we look up the DialogRef by
// ID. This must occur in `onInit`, as the ID binding for the dialog container won't
// be resolved at constructor time.

@crisbeto crisbeto force-pushed the 5412/template-dialog-elements branch from 4eb16c2 to d94efbe Compare January 16, 2018 18:47
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Jan 16, 2018
@jelbourn
Copy link
Member

@crisbeto many tests failing with Cannot read property '_ariaLabelledBy' of undefined at MatDialogTitle.ngOnInit

@jelbourn jelbourn added the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jan 23, 2018
…ialogs

Previously the `matDialogClose`, `matDialogTitle` etc. directives would only work correctly inside component dialogs, because using DI to get the dialog ref doesn't work inside template dialogs. These changes add a fallback that finds the dialog ref based on the id of the closest dialog container.

Fixes angular#5412.
@crisbeto crisbeto force-pushed the 5412/template-dialog-elements branch from d94efbe to 7d7d225 Compare January 23, 2018 18:55
@crisbeto crisbeto removed the presubmit failures This PR has failures in Google's internal presubmit process and cannot be immediately merged label Jan 23, 2018
@crisbeto
Copy link
Member Author

Pushed a fix for the null pointer error.

@jelbourn jelbourn merged commit 99b768e into angular:master Jan 24, 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 8, 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.

Opening an MdDialog with a TemplateRef that uses md-dialog-close throws "no provider for MdDialogRef"
3 participants