feat(@schematics/angular): cli app redesign #14403

sjtrimble · 2019-05-11T03:30:56Z

Updated ng new template design
Update favicon to be black as default

CC: @IgorMinar

sjtrimble · 2019-05-11T03:36:47Z

Initial Mockups

Desktop

Mobile

filipesilva · 2019-05-20T07:46:56Z

@alexeagle do you know if this should be part of CLI version 8?

filipesilva · 2019-05-20T10:29:01Z

packages/schematics/angular/application/files/src/index.html.template

+    />
+
+  <style>
+    body {


These styles should be in src/styles.css. But they might be unusable there if the user chooses a CSS extension whose syntax isn't a superset of basic css. like sass. Perhaps we should make an extra src/demo-styles.css (or some other name).

The idea was to keep them in the index file (even though not really the way things are normally done) so that the user can easily delete all of the template code in one full swoop.

packages/schematics/angular/application/other-files/app.component.html.template

filipesilva · 2019-05-20T12:41:29Z

packages/schematics/angular/application/files/src/index.html.template

@@ -7,6 +7,311 @@

  <meta name="viewport" content="width=device-width, initial-scale=1">
  <link rel="icon" type="image/x-icon" href="favicon.ico">
+  <link


I feel users might just forget these here when removing the demo code.

I tried moving them into src/styles.css like this:

@import "https://fonts.googleapis.com/css?family=Roboto"; @import "https://fonts.googleapis.com/css?family=Roboto+Mono"; @import "https://fonts.googleapis.com/icon?family=Material+Icons";

And it does work, in Chrome at least. But it's not converted to anything else. It relies on native browser support for @import in CSS. This it wouldn't be a great choice either. Might just be better to keep it here.

I see your point about it not being obvious for users to delete. Like with the rest of the CSS I like having it all in one place to just focus on one file when removing template code.

What if we add comments for now that say "remove this template code"? I'll implement that for now until we come up with something better.

Added the comments  and  in both template and in index files.

That SGTM, let's roll with it.

alexeagle · 2019-05-20T19:38:21Z

Looks great to get this into 8, but needs to land by tomorrow

sjtrimble · 2019-05-20T22:53:58Z

@alexeagle let me know if there is anything you need from me! I just made the updates and rebased.

filipesilva · 2019-05-21T10:28:42Z

@sjtrimble I'll pick it up from here and fix the failing tests. They are mostly related to the unit tests and e2e expecting the old structure.

filipesilva · 2019-05-21T11:00:43Z

Followup PR in #14484

alexeagle · 2019-05-21T17:18:51Z

@sjtrimble could we remove the changes to index.html? We think developers will accidentally leave these behind in their application.

do we really need the custom font? can we avoid using material icons and fall back to browser built-in font?
can we inline as many styles as possible?
if styles are needed on the body, could they be injected by the component constructor

we want it to be sufficient to delete the component and then you have a clean starting point for an app without any of this boilerplate left behind

alan-agius4 · 2019-05-21T17:24:06Z

For fonts maybe we can use @import in the components style file?
See support: https://developer.mozilla.org/en-US/docs/Web/CSS/@import

IgorMinar

let's not make any changes to the index.html.

IgorMinar · 2019-05-21T17:28:56Z

packages/schematics/angular/application/files/src/index.html.template

+      -moz-osx-font-smoothing: grayscale;
+    }
+
+    body,


could we move all of these stylesheets into the component itself? We can't rely on people to correctly remove the style sheet from their app.

You can use https://next.angular.io/guide/component-styles#host-context to get the sheets into the top level document stylesheet.

And for the fonts or anything else that can't be added via that method, we could just inject the https://next.angular.io/api/common/DOCUMENT into the component constructor and do manual dom manipulation from there to create and append fonts to the document.head.

sjtrimble · 2019-05-21T18:01:07Z

Copied over #14484 test fixes per @IgorMinar

sjtrimble · 2019-05-21T18:48:51Z

Moved all code to the template file for easy removal, except for body zero padding/margin styles.

IgorMinar · 2019-05-23T13:20:10Z

Let's get this done in 8.1. I'm sorry for deferring it but there are still questions to resolve and as much as I love this change, we shouldn't rush it and pollute new projects with code that we'll regret in the future.

vthinkxie · 2019-07-04T09:18:13Z

any update on this, it seems that 8.1 has not included this feat

mgechev · 2019-07-19T00:00:46Z

Let's see if we can release it as part of version 8.2.

vikerman · 2019-07-25T17:45:40Z

Hi @sjtrimble - We discussed how to take this forward.

We would like to include this as part of the 8.2 release.

We would like the starter to have the following properties:

No changes to index.html
All changes the user need to do while changing it to their are restricted to app.component.html, app.component.css
Sort of simple CSS for the app component. The current one is very long
Use a system font without needing to link to an external font file

sjtrimble · 2019-08-02T20:35:19Z

✅I rebased on the latest master so everything is fresh.
✅I removed the external font references/links and replaced with system fonts.
🔸 Let's chat about the CSS - I think there is value in having a presentable and visually appealing first impression but perhaps we can simplify the code a bit.
✅I removed everything additional from the index.html file.

sjtrimble · 2019-08-05T21:10:23Z

@vikerman FYI ⬆️

IgorMinar · 2019-08-08T18:30:13Z

packages/schematics/angular/application/files/src/index.html.template

@@ -7,6 +7,7 @@

  <meta name="viewport" content="width=device-width, initial-scale=1">
  <link rel="icon" type="image/x-icon" href="favicon.ico">
+


I assume you mean the extra line space here?

IgorMinar

Considering all the complications with the other approaches I've proposed before, I think having everything in this single file is fine, as long as we leave clear comments about what code belongs to the welcome placeholder and can be removed.

I've added a few comments.

IgorMinar · 2019-08-08T18:58:52Z

packages/schematics/angular/application/other-files/app.component.html.template

@@ -1,21 +1,506 @@
 <!--The content below is only a placeholder and can be replaced.-->


could you make this a multiline comment full of stars and emojis with a clear text in the middle that the content below should be removed?

something like this, but you can make it even more fancy: https://github.com/angular/angular.js/blob/060bcdeeb9f5181fb885417f68429c86a8e59eb2/src/ngSanitize/sanitize.js#L3-L12

and we should have a similar "banner" at the bottom as well.

IgorMinar · 2019-08-08T19:00:10Z

packages/schematics/angular/application/other-files/app.component.html.template

+        <path id="Path_19" data-name="Path 19" d="M4137.529,1027.283l-7.7-3.723,4.417-2.721,7.753,3.723Z" transform="translate(-4125.003 -1015.199)" fill="#fff"/>
+      </svg>
+
+      <!-- <img src="../assets/animations.png" height="20px" alt="Angular Animations Logo"/> -->


commented out?

IgorMinar · 2019-08-08T19:01:47Z

packages/schematics/angular/application/other-files/app.component.html.template

+
+</div>
+
+<!-- *** Template Code Ends *** -->


as mentioned above, this should be a highly visible multiline comment/banner as well so that it's easy to identify. Also at the top you used "placeholder" terminology and down here you are using "Template", can you please make it consistent? thanks!

vikerman · 2019-08-09T18:31:53Z

packages/schematics/angular/application/other-files/app.component.html.template

+<!-- * * * * * * * * * * is only a placeholder * * * * * * * * * * -->
+<!-- * * * * * * * * * * and can be replaced. * * * * * * * * * * * -->
+<!-- * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * -->
+<!-- * * * * * * * * * Delete the template below * * * * * * * * * * -->


It's not below :) We can probably leave this sentence out in this part.

Or maybe just say - End of placeholder

Ahhh, I said above on the first line and forgot the last sentence :) I like your idea of end of placeholder better! Thanks :)

mgechev

LGTM. The failures are caused by:

Few commit messages without a scope
Size has increased a bit because of the template & style updates
Failures caused by framework's release as @alan-agius4 pointed out

@vikerman, as discussed, I think we're ready to merge.

angular-automatic-lock-bot · 2019-09-13T07:40:56Z

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.}

googlebot added the cla: yes label May 11, 2019

sjtrimble force-pushed the ng-new-redesign branch from 3898182 to 2b18d6f Compare May 11, 2019 03:31

sjtrimble changed the title ~~CLI app redesign~~ feat(@angular/cli): cli app redesign May 11, 2019

filipesilva reviewed May 20, 2019

View reviewed changes

alexeagle added this to the 8.0 milestone May 20, 2019

sjtrimble force-pushed the ng-new-redesign branch 2 times, most recently from 32b115d to e1fcdcf Compare May 20, 2019 22:51

filipesilva self-assigned this May 21, 2019

filipesilva mentioned this pull request May 21, 2019

feat(@schematics/angular): cli app redesign #14484

Closed

IgorMinar suggested changes May 21, 2019

View reviewed changes

sjtrimble force-pushed the ng-new-redesign branch from 46d3a35 to c8445f4 Compare May 21, 2019 18:42

IgorMinar modified the milestones: 8.0, Backlog May 23, 2019

alexeagle modified the milestones: Backlog, 8.1.x Jun 18, 2019

alan-agius4 mentioned this pull request Jul 1, 2019

better looking default template when ng new #14946

Closed

mgechev modified the milestones: 8.1.x, 8.2.x Jul 19, 2019

mgechev added the needs: discussion On the agenda for team meeting to determine next steps label Jul 19, 2019

vikerman assigned vikerman and unassigned filipesilva Jul 25, 2019

filipesilva removed the needs: discussion On the agenda for team meeting to determine next steps label Jul 25, 2019

sjtrimble force-pushed the ng-new-redesign branch from c8445f4 to 6f38341 Compare August 2, 2019 20:19

sjtrimble force-pushed the ng-new-redesign branch from 70aa987 to 07852d2 Compare August 5, 2019 21:05

IgorMinar reviewed Aug 8, 2019

View reviewed changes

sjtrimble added 9 commits August 9, 2019 10:23

feat(@angular/cli): cli app redesign

29aeb86

add template comment code and fix rocket ship smoke image placement

c19602a

correct tests verbiage

c9736ba

fix tests copy of PR 14484

0e00408

move styles to template and remove unused icons font ref

ff2b715

move font references to template

0292887

remove external font references

bf34161

remove body styles and simplify css

1591fb5

add comment block and remove old comments

3853f88

sjtrimble force-pushed the ng-new-redesign branch from 07852d2 to 3853f88 Compare August 9, 2019 17:35

vikerman reviewed Aug 9, 2019

View reviewed changes

fix end of placeholder comment

ea8e796

mgechev approved these changes Aug 12, 2019

View reviewed changes

mgechev requested a review from IgorMinar August 12, 2019 20:52

mgechev changed the title ~~feat(@angular/cli): cli app redesign~~ feat(@schematics/angular): cli app redesign Aug 12, 2019

mgechev merged commit 40601c4 into angular:master Aug 12, 2019

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

+                  />
+                <style>
+                  body {

		@@ -7,6 +7,7 @@

		<meta name="viewport" content="width=device-width, initial-scale=1">
		<link rel="icon" type="image/x-icon" href="favicon.ico">

		@@ -1,21 +1,506 @@
		<!--The content below is only a placeholder and can be replaced.-->

feat(@schematics/angular): cli app redesign #14403

feat(@schematics/angular): cli app redesign #14403

Conversation

sjtrimble commented May 11, 2019

sjtrimble commented May 11, 2019

Initial Mockups

filipesilva commented May 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexeagle commented May 20, 2019

sjtrimble commented May 20, 2019

filipesilva commented May 21, 2019

filipesilva commented May 21, 2019

alexeagle commented May 21, 2019

alan-agius4 commented May 21, 2019

IgorMinar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjtrimble commented May 21, 2019

sjtrimble commented May 21, 2019

IgorMinar commented May 23, 2019

vthinkxie commented Jul 4, 2019

mgechev commented Jul 19, 2019

vikerman commented Jul 25, 2019

sjtrimble commented Aug 2, 2019 • edited

sjtrimble commented Aug 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IgorMinar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgechev left a comment • edited

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 13, 2019

sjtrimble commented Aug 2, 2019 •

edited

mgechev left a comment •

edited