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';
- Long imports can be broken up to multiple lines.
One line per import. - Spaces between braces of destructured imports.
- App specific imports should be separated by a new line.
- Simple imports should only take up one line.
- 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.
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"
-
Add button classes
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
-
Typography
-
en-, em-dashes for date ranges
-
-
No content inside icon-only buttons (for screenreaders)
-
No type attribute on buttons (button, submit)
-
No roles/aria-roles attributes on landmarks
-
Using block level elements with overridden style
(eg. div with inline-block when could be span) -
Redeclaring global styles already available
-
Not leveraging scss syntax (nesting, variables)
-
Double quotes instead of single in js
Style Guide Review
By Carlos Filoteo
Style Guide Review
- 633