Skip to content

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

Merged
jelbourn merged 1 commit into
angular:masterfrom
crisbeto:5412/template-dialog-elements
Jan 24, 2018
Merged

feat(dialog): support using dialog content directives with template dialogs#9379
jelbourn merged 1 commit into
angular:masterfrom
crisbeto:5412/template-dialog-elements

Conversation

@crisbeto

Copy link
Copy Markdown
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.

@googlebotgooglebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 13, 2018

@jelbournjelbourn left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
crisbetoforce-pushed the 5412/template-dialog-elements branch from 4eb16c2 to d94efbeCompareJanuary 16, 2018 18:47
@crisbetocrisbeto 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
Copy Markdown
Contributor

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

@jelbournjelbourn 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.
Fixesangular#5412.
@crisbeto
crisbetoforce-pushed the 5412/template-dialog-elements branch from d94efbe to 7d7d225CompareJanuary 23, 2018 18:55
@crisbetocrisbeto 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
Copy Markdown
MemberAuthor

Pushed a fix for the null pointer error.

@jelbourn
jelbourn merged commit 99b768e into angular:masterJan 24, 2018
@j2L4ej2L4e mentioned this pull request Mar 23, 2019
@angular-automatic-lock-bot

Copy link
Copy Markdown

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-botangular-automatic-lock-botBot locked and limited conversation to collaborators Sep 8, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: mergeThe PR is ready for merge by the caretakercla: yesPR author has agreed to Google's Contributor License Agreementtarget: minorThis 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

@crisbeto@jelbourn@googlebot