feat(TG-318): add empty home page #158

Hliebov · 2016-11-16T22:32:18Z

No description provided.

review-ninja · 2016-11-16T22:32:19Z

borys-ihnatyev · 2016-11-17T10:37:55Z

src/app/home/home.template.html

+        </button>
+        <div class="home-nav-menu">
+            <ul class="home-nav-menu-list">
+                <span class="home-nav-menu-list__title">


It is semantically wrong to put span into ul, there is only li can be children of ul

borys-ihnatyev · 2016-11-17T10:40:23Z

src/app/home/home.style.scss

@@ -0,0 +1,66 @@
+.home {
+
+    &-nav {


I would rather split this menu list and button, into aib blocks

borys-ihnatyev · 2016-11-17T10:41:56Z

src/app/home/home.style.scss

+            float: right;
+            margin: 30px 80px 0 0;
+
+            &-list {


could you please make home block structure flatten by moving list to aib block,
you should avoid using

.ablock { &-block {

single stylesheet file should describe single block

.ablock { &__element { // good &_mod { } // good } &_mod { } // good &-not-another-block {} // bad: you should declare it in another file }

for example:

.menu { // independent styles } .home { &__menu { // styling of menu (placement ....) } }

and usage

<div class="home"> <ul class="home__menu menu"> </ul> </div>

borys-ihnatyev · 2016-11-17T10:53:13Z

src/app/home/home.style.scss

+            }
+        }
+
+        &__create {


I think this is not the element, this is block button block
you can leave this element only for context-styling like color, placing, floating ....

borys-ihnatyev · 2016-11-18T11:40:33Z

src/app/home/home.style.scss

+    .nav {
+        height: 70px;
+
+        .create {


please avoid cascades as possible, it is highly not recommended to use cascades with bem

borys-ihnatyev · 2016-11-18T11:41:54Z

src/app/home/home.style.scss

+
+        .create {
+            float: right;
+            &__button {


in this case button is block not an element

codecov-io · 2016-11-18T13:11:42Z

Current coverage is 96.94% (diff: 100%)

Merging #158 into master will increase coverage by 0.01%

@@             master       #158   diff @@
==========================================
  Files            51         53     +2   
  Lines           881        885     +4   
  Methods           8          8          
  Messages          0          0          
  Branches         34         33     -1   
==========================================
+ Hits            854        858     +4   
  Misses           27         27          
  Partials          0          0

Powered by Codecov. Last update 2f3f2d0...b765427

velmyk · 2016-11-20T22:00:59Z

src/app/core/header/header.component.ts

@@ -22,7 +11,7 @@ export class HeaderComponent implements OnInit {
    private mainRoute: Object;


You created NavigationRoute interface. It can be used to declare type of mainRoute

disist · 2016-11-21T09:12:29Z

src/app/home/home.component.ts

+})
+export class HomeComponent {
+
+    public onCreateViewClick(): void {


If you create this for future, will be better to leave comment about this, because it looks like not finished solution.

disist · 2016-11-21T09:16:31Z

src/app/core/header/navigation-route.ts

@@ -0,0 +1,6 @@
+export interface NavigationRoute {


Interface's approach it is not good way to describe models in TypeScript.
It will better to use model approach.

For example, please look into angular 2 code style (https://angular.io/styleguide) and find hero.model.ts

yakimko · 2016-11-21T12:58:11Z

src/app/core/header/header.component.ts


    ngOnInit() {
-        const routes = NAVIGATION_ROUTES.filter(route => route['main']);
+        const routes = NAVIGATION_ROUTES.filter(route => route.main);


we have only one main route

so

this.mainRoute = NAVIGATION_ROUTES.find(route => route.main);

yakimko · 2016-11-21T13:00:03Z

src/app/core/header/header.component.test.ts

-    { path: `/${ROUTING.LOGOUT}`, headerName: 'Logout', iconImage: './assets/logout.svg' },
-    { path: `/${ROUTING.REGISTRATION}`, headerName: 'Registration', iconImage: './assets/registration.svg' },
-];
+const MAIN_ROUTE = NAVIGATION_ROUTES[0];


why not NAVIGATION_ROUTES.find(route => route.main) ?

borys-ihnatyev · 2016-11-21T13:11:56Z

src/app/core/header/header.component.test.ts

-    { path: `/${ROUTING.LOGOUT}`, headerName: 'Logout', iconImage: './assets/logout.svg' },
-    { path: `/${ROUTING.REGISTRATION}`, headerName: 'Registration', iconImage: './assets/registration.svg' },
-];
+const MAIN_ROUTE = NAVIGATION_ROUTES[0];


borys-ihnatyev · 2016-11-21T13:12:05Z

src/app/core/header/header.component.ts


    ngOnInit() {
-        const routes = NAVIGATION_ROUTES.filter(route => route['main']);
+        const routes = NAVIGATION_ROUTES.filter(route => route.main);


borys-ihnatyev · 2016-11-21T13:12:08Z

src/app/home/home.component.ts

+})
+export class HomeComponent {
+
+    public onCreateViewClick(): void {


borys-ihnatyev reviewed Nov 17, 2016

View reviewed changes

borys-ihnatyev changed the title ~~Story/tg 287~~ feat(TG-287): add empty dashboard view Nov 17, 2016

borys-ihnatyev reviewed Nov 17, 2016

View reviewed changes

borys-ihnatyev changed the title ~~feat(TG-287): add empty dashboard view~~ feat(TG-318): add empty dashboard view Nov 17, 2016

borys-ihnatyev reviewed Nov 18, 2016

View reviewed changes

src/app/home/home.style.scss

.create {

float: right;

&__button {

Copy link

Contributor

borys-ihnatyev Nov 18, 2016

Choose a reason for hiding this comment

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

in this case button is block not an element

Oleksandr Hliebov and others added 6 commits November 18, 2016 14:48

[TG-318][init] structure created

266df43

[TG-318][init] styles added

9edf82e

[TG-318][refactor] styling changed

efbd181

fix selectors and markup semantics

2cd007f

fix header component tests

f5af9b1

extract shared blocks

545f8c5

borys-ihnatyev force-pushed the story/TG-287 branch from 4579159 to 451c669 Compare November 18, 2016 12:49

fix lint errors

3eb8e31

borys-ihnatyev force-pushed the story/TG-287 branch from 451c669 to 3eb8e31 Compare November 18, 2016 13:04

Borys Ihnatyev added 2 commits November 18, 2016 15:07

fix lint errors

f251098

use url approach of style and template declaration

98640bd

move shared styles import into app.style.scss

e16e2cd

borys-ihnatyev changed the title ~~feat(TG-318): add empty dashboard view~~ feat(TG-318): add empty home page Nov 18, 2016

velmyk reviewed Nov 20, 2016

View reviewed changes

disist reviewed Nov 21, 2016

View reviewed changes

fix review issues

6d14a82

yakimko reviewed Nov 21, 2016

View reviewed changes

fix review issues

b137b67

fix review issues

793f68e

borys-ihnatyev reviewed Nov 21, 2016

View reviewed changes

src/app/home/home.component.ts

})

export class HomeComponent {

public onCreateViewClick(): void {

Copy link

Contributor

borys-ihnatyev Nov 21, 2016

Choose a reason for hiding this comment

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

!fixed

fix lint issues

b765427

borys-ihnatyev merged commit 4bb0bc5 into master Nov 21, 2016

borys-ihnatyev deleted the story/TG-287 branch November 21, 2016 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(TG-318): add empty home page #158

feat(TG-318): add empty home page #158

Hliebov commented Nov 16, 2016

review-ninja commented Nov 16, 2016

borys-ihnatyev Nov 17, 2016

borys-ihnatyev Nov 17, 2016

borys-ihnatyev Nov 17, 2016 •

edited

Loading

borys-ihnatyev Nov 17, 2016

borys-ihnatyev Nov 18, 2016

borys-ihnatyev Nov 18, 2016

codecov-io commented Nov 18, 2016 •

edited

Loading

velmyk Nov 20, 2016

borys-ihnatyev Nov 21, 2016

disist Nov 21, 2016

borys-ihnatyev Nov 21, 2016

disist Nov 21, 2016

borys-ihnatyev Nov 21, 2016

yakimko Nov 21, 2016

borys-ihnatyev Nov 21, 2016

yakimko Nov 21, 2016

borys-ihnatyev Nov 21, 2016

borys-ihnatyev Nov 21, 2016

borys-ihnatyev Nov 21, 2016

borys-ihnatyev Nov 21, 2016

		@@ -22,7 +11,7 @@ export class HeaderComponent implements OnInit {
		private mainRoute: Object;

feat(TG-318): add empty home page #158

feat(TG-318): add empty home page #158

Conversation

Hliebov commented Nov 16, 2016

review-ninja commented Nov 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

borys-ihnatyev Nov 17, 2016 • edited Loading

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

codecov-io commented Nov 18, 2016 • edited Loading

Current coverage is 96.94% (diff: 100%)

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

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

borys-ihnatyev Nov 17, 2016 •

edited

Loading

codecov-io commented Nov 18, 2016 •

edited

Loading