-
Notifications
You must be signed in to change notification settings - Fork 12k
fix(@angular/cli): fix tests to match app component template #6538
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
Conversation
I think the actual problem is that the inline template is different. It only includes the prefix and not the welcome text. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't quite seem to be the whole story, somethings missing. See #6537 (comment).
Avoid mismatch when app component template blueprint are modified.
@clydin is right, I updated the PR. By the way, as I'm modifying the template :
|
<h1> | ||
Welcome to {{title}}!! | ||
</h1> | ||
<img width="300" src="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4N |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it better to save the image as a file in the assets
folder and reference it here? Would make the file a lot cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The content is going to be almost immediately deleted so there's little point to adding an additional asset that must be deleted as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clydin notice below that I referenced your PR. I had to do a similar fix in my PR so that the tests will pass, so I've just fixed the Welcome to {{title}}!
so that it would work and wouldn't have merge-conflicts with your PR if mine happens to go in master before yours.
* note: this error already came up in angular#6538, so I'm just fixing the title so that the PR would not cause any merge-conflicts
…g --inline-template * note: this error already came up in angular#6538, so I'm just fixing the title so that the PR would not cause any merge-conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can you please squash all commits and make it one commit?
- Add
e2e
tests for the use case? - Rebase with master
…g --inline-template * note: this error already came up in angular#6538, so I'm just fixing the title so that the PR would not cause any merge-conflicts
Unfortunately I won't be available the next 2 weeks, so feel free to edit my PR or to create a new one. |
…g --inline-template * note: this error already came up in angular#6538, so I'm just fixing the title so that the PR would not cause any merge-conflicts
Closing as I think it's outdated. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #6537