Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

bug(plunker): fix plunkers not working with dialogs #5899 #228

Closed
wants to merge 2 commits into from

Conversation

amcdnl
Copy link
Contributor

@amcdnl amcdnl commented Jul 23, 2017

@julianobrasil
Copy link

@amcdnl, I haven't seen any mention in the code to the entryComponents array. It should be present in the case of MdDialog.

@jelbourn
Copy link
Member

@tinayuangao is better to review since she authored the plunker generation

// Replace `declarations: [MaterialDocsExample]`
// will be replaced as `declarations: [ButtonDemo]`
fileContent = fileContent.
replace(/declarations: \[MaterialDocsExample\]/g,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also replace entryComponents?

Add entryComponents: [MaterialDocsExample] to main.ts.

I tested on plunker and found out it doesn't work without entryComponents.

@amcdnl
Copy link
Contributor Author

amcdnl commented Sep 9, 2017

@tinayuangao - Is this PR still relevant. Looks like they are working today.

@julianobrasil
Copy link

julianobrasil commented Sep 9, 2017

@amcdnl, @tinayuangao, dialogs are still throwing exceptions (lack of entryComponents).

@amcdnl
Copy link
Contributor Author

amcdnl commented Sep 11, 2017

@julianobrasil where are you seeing it throwing the errors?

@julianobrasil
Copy link

@amcdnl, if you click on the link to the plunk example for the dialogs (at https://material.angular.io/components/dialog/overview), you'll see the errors:

image

Notice that the entryComponents attribute is missing and there are two components in bootstrap attribute.

@amcdnl
Copy link
Contributor Author

amcdnl commented Dec 16, 2017

Closing this since it outdated, will update in a new PR.

@amcdnl amcdnl closed this Dec 16, 2017
@amcdnl amcdnl deleted the dialog-plunker-bug branch December 16, 2017 22:31
amcdnl added a commit to amcdnl/material.angular.io that referenced this pull request Dec 16, 2017
@amcdnl
Copy link
Contributor Author

amcdnl commented Dec 16, 2017

Resolved here - #356

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants