Skip to content

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

Closed
cyrilletuzi wants to merge 5 commits into
angular:masterfrom
cyrilletuzi:patch-3
Closed

fix(@angular/cli): fix tests to match app component template#6538
cyrilletuzi wants to merge 5 commits into
angular:masterfrom
cyrilletuzi:patch-3

Conversation

@cyrilletuzi

Copy link
Copy Markdown
Contributor

Fixes#6537

@clydin

Copy link
Copy Markdown
Member

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

@filipesilvafilipesilva self-assigned this Jun 1, 2017

@filipesilvafilipesilva 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.

This doesn't quite seem to be the whole story, somethings missing. See #6537 (comment).

@cyrilletuzi

cyrilletuzi commented Jun 1, 2017

Copy link
Copy Markdown
ContributorAuthor

@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

@gioraguttgioraguttJun 3, 2017

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.

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

@sumitarorasumitarora 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.

@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
Copy Markdown
ContributorAuthor

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
@filipesilvafilipesilva assigned Brocco and unassigned sumitaroraJul 3, 2017
@cyrilletuzi

Copy link
Copy Markdown
ContributorAuthor

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
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 12, 2019
Sign up for freeto 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

@cyrilletuzi@clydin@sumitarora@filipesilva@gioragutt@Brocco@googlebot