Style Guide Review

declare var $:any, dna:any;

@Component({
    ...
})
export class EthDiversityComponent {
    private regionId: string;
    private routeSub:any;
    private ethnicitySub:any;
    ...
}
declare var $: any, dna: any;
              ^         ^
@Component({
    ...
})
export class EthDiversityComponent {
    private regionId: string;
    private routeSub: any;
                     ^
    private ethnicitySub: any;
                         ^
    ...
}

Missing spaces between type definition.

Prefs > Editor > Code Style > TypeScript > check "After type reference colon ':'"

import { Component, OnInit, Input, ViewChild, ElementRef } from '@angular/core';
import {ActivatedRoute} from '@angular/router';
import {Branch} from '../branch/branch.model';
import {
    I18nService
} from '../shared';
import { UnusedComponent } from '../somewhere';
import { 
    Component, 
    OnInit, 
    Input, 
    ViewChild, 
    ElementRef 
} from '@angular/core';
import { ActivatedRoute } from '@angular/router';

import { Branch } from '../branch/branch.model';
import { I18nService } from '../shared';
  1. Long imports can be broken up to multiple lines.
    One line per import.
  2. Spaces between braces of destructured imports.
  3. App specific imports should be separated by a new line.
  4. Simple imports should only take up one line.
  5. Remove unused imports. They may have performance implications.

Prefs > Editor > Code Style > TypeScript

        > Spaces > check "ES6 import/export braces"

        > Wrapping and Braces > Select "Chop down if long" for ES6 import/export

import { OnInit } from '@angular/core';

export class SomeComponent {
    ...
    ngOnInit() {
        ...
    }
}

export class SomeOtherComponent implements OnInit {
    ...
    ngOnInit() {
        ...
    }

    ngOnDestroy() {
        ...
    }
}
import { OnInit, OnDestroy } from '@angular/core';
               ^^^^^^^^^^^
export class SomeComponent implements OnInit {
    ...                    ^^^^^^^^^^^^^^^^^
    ngOnInit() {
        ...
    }
}

export class SomeOtherComponent implements OnInit, OnDestroy {
    ...                                          ^^^^^^^^^^^
    ngOnInit() {
        ...
    }

    ngOnDestroy() {
        ...
    }
}

Implement the lifecycle hook interfaces.

https://angular.io/styleguide#!#09-01

ethnicity.regions.filter( region => {
    return region.regionKey === this.regionId;
}).map( region => {
    ...
});

this.appStateService.hoveredBranchId.subscribe((branchId) => {
    this.hovered = (this.branch.id.toString() === branchId);
}).unsubscribe();
ethnicity.regions
    .filter(region => {
        return region.regionKey === this.regionId;
    })
    .map(region => {
        ...
    });

this.appStateService.hoveredBranchId
    .subscribe(branchId => {
        this.hovered = (this.branch.id.toString() === branchId);
    })
    .unsubscribe();

Chained method calls should be on their own lines and indented by 1 additional tab.

http://www.ancestry.com/cs/standards/js-standards#chainedMethodCalls 

Prefs > Editor > Code Style > TypeScript

        > Tabs and Indents > check  "Indent all chained calls in a group."

<div (click)="doSomething()" 
    (keypress)="detectSpacebarPress($evt)" 
    aria-role="button" tab-index=0></div>
<button (click)="doSomething()" type="button"></button>
  • Semantic HTML
  • A div with a click event cannot be detected easily with screen readers, even with the correct aria-role
  • Using a button automatically adds it to the tab flow as well as enable click on spacebar/ENTER press (keyboard users)
  • If you HAVE to, make sure to:
    • Add button classes
      • Don't forget hover/focus/active styles!
    • Add aria-role="button"
    • Detect spacebar/ENTER keypress
    • Add to tab flow with tabindex="0"
import { Component, OnInit } from '@angular/core';

@Component({
  moduleId: module.id,
  selector: 'dna-test',
  templateUrl: 'test.component.html',
  styleUrls: ['test.component.css']
})
export class TestComponent implements OnInit {

  constructor() {}

  ngOnInit() {
  }

}

* Autogenerated by CLI

import { Component, OnInit } from '@angular/core';

@Component({
    moduleId: module.id,
    selector: 'dna-test',
    templateUrl: 'test.component.html',
    styleUrls: ['test.component.scss']
                               ^^^^^ 
})
export class TestComponent implements OnInit {
    constructor() {}

    ngOnInit() {
    }
}

Code generated by the CLI doesn't use our editorconfig. Make sure to fix formatting before committing autogenerated code.

  • Remove extra spaces
  • Rename .css to .scss
  • Remove unused lifecycle implements if needed

Other missteps

  1. Typography

    1. en-, em-dashes for date ranges

  2. No content inside icon-only buttons (for screenreaders)

  3. No type attribute on buttons (button, submit)

  4. No roles/aria-roles attributes on landmarks

  5. Using block level elements with overridden style
    (eg. div with inline-block when could be span)

  6. Redeclaring global styles already available

  7. Not leveraging scss syntax (nesting, variables)

  8. Double quotes instead of single in js

Style Guide Review

By Carlos Filoteo

Style Guide Review

  • 633