Skip to content

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

Closed
wants to merge 5 commits into from
Closed

fix(@angular/cli): fix tests to match app component template #6538

wants to merge 5 commits into from

Conversation

cyrilletuzi
Copy link
Contributor

Fixes #6537

@clydin
Copy link
Member

clydin commented Jun 1, 2017

I think the actual problem is that the inline template is different. It only includes the prefix and not the welcome text.

@filipesilva filipesilva self-assigned this Jun 1, 2017
Copy link
Contributor

@filipesilva filipesilva left a 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).

@cyrilletuzi
Copy link
Contributor Author

cyrilletuzi commented Jun 1, 2017

@clydin is right, I updated the PR.

By the way, as I'm modifying the template :

  • I added a comment about what can be deleted in the template, it was confusing if routing,
  • do we really want inline styles, even if it's a boilerplate ? especially when it's not really relevant visually, as on big screens it's really unaligned with the links below.

<h1>
Welcome to {{title}}!!
</h1>
<img width="300" src="data:image/svg+xml;base64,PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTgiPz4N
Copy link
Contributor

@gioragutt gioragutt Jun 3, 2017

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.

Copy link
Member

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.

Copy link
Contributor

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.

gioragutt added a commit to gioragutt/angular-cli that referenced this pull request Jun 3, 2017
* 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
gioragutt added a commit to gioragutt/angular-cli that referenced this pull request Jun 4, 2017
…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
@filipesilva filipesilva requested a review from sumitarora June 6, 2017 14:01
@filipesilva filipesilva assigned sumitarora and unassigned filipesilva Jun 6, 2017
Copy link
Contributor

@sumitarora sumitarora left a comment

Choose a reason for hiding this comment

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

@cyrilletuzi

  • Can you please squash all commits and make it one commit?
  • Add e2e tests for the use case?
  • Rebase with master

gioragutt added a commit to gioragutt/angular-cli that referenced this pull request Jun 7, 2017
…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
@cyrilletuzi
Copy link
Contributor Author

Unfortunately I won't be available the next 2 weeks, so feel free to edit my PR or to create a new one.

gioragutt added a commit to gioragutt/angular-cli that referenced this pull request Jun 12, 2017
…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
@filipesilva filipesilva assigned Brocco and unassigned sumitarora Jul 3, 2017
@cyrilletuzi
Copy link
Contributor Author

Closing as I think it's outdated.

@cyrilletuzi cyrilletuzi deleted the patch-3 branch August 19, 2017 12:50
@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 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ng test / e2e fail in 1.1.0 due to mismatch between test and template - inline templates
7 participants