The Google JavaScript Style Guide is the basis for our coding style, with additional guidance here where that style guide is not aligned with ES6 or TypeScript.
Comments that explain what some block of code does are nice; they can tell you something in less time than it would take to follow through the code itself.
Comments that explain why some block of code exists at all, or does something the way it does, are invaluable. The "why" is difficult, or sometimes impossible, to track down without seeking out the original author. When collaborators are in the same room, this hurts productivity. When collaborators are in different timezones, this can be devastating to productivity.
For example, this is a not-very-useful comment:
// Set default tabindex.
if (!$attrs['tabindex']) {
$element.attr('tabindex', '-1');
}
While this is much more useful:
// Unless the user specifies so, the calendar should not be a tab stop.
// This is necessary because ngAria might add a tabindex to anything with an ng-model
// (based on whether or not the user has turned that particular feature on/off).
if (!$attrs['tabindex']) {
$element.attr('tabindex', '-1');
}
For example, rather than doing this:
<span tooltip="Basic tooltip!">test tooltip</span>
do this:
<md2-tooltip>Basic tooltip!</md2-tooltip>
Keeping modules to a single responsibility makes the code easier to test, consume, and maintain. ES6 modules offer a straightforward way to organize code into logical, granular units. Ideally, individual files are 200 - 300 lines of code.
As a rule of thumb, once a file draws near 400 lines (barring abnormally long constants / comments), start considering how to refactor into smaller pieces.
Once a feature is released, it never goes away. We should avoid adding features that don't offer high user value for price we pay both in maintenance, complexity, and payload size. When in doubt, leave it out.
This applies especially so to providing two different APIs to accomplish the same thing. Always prefer sticking to a single API for accomplishing something.
All code and docs in the repo should be 100 columns or fewer. This applies to TypeScript, SCSS, HTML, bash scripts, and markdown files.
Avoid any
where possible. If you find yourself using any
, consider whether a generic may be
appropriate in your case.
For methods and properties that are part of a component's public API, all types must be explicitly specified because our documentation tooling cannot currently infer types in places where TypeScript can.
When creating a fluent or builder-pattern style API, use the this
return type for methods:
class ConfigBuilder {
withName(name: string): this {
this.config.name = name;
return this;
}
}
- Omit the
public
keyword as it is the default behavior. - Use
private
when appropriate and possible, prefixing the name with an underscore. - Use
protected
when appropriate and possible with no prefix. - Prefix library-internal properties and methods with an underscore without using the
private
keyword. This is necessary for anything that must be public (to be used by Angular), but should not be part of the user-facing API. This typically applies to symbols used in template expressions,@ViewChildren
/@ContentChildren
properties, host bindings, and@Input
/@Output
properties (when using an alias).
Additionally, the @docs-private
JsDoc annotation can be used to hide any symbol from the public
API docs.
All public APIs must have user-facing comments. These are extracted and shown in the documation on https://github.com/Promact/md2.
Private and internal APIs should have JsDoc when they are not obvious. Ultimately it is the purview of the code reviwer as to what is "obvious", but the rule of thumb is that most classes, properties, and methods should have a JsDoc description.
Properties should have a concise description of what the property means:
/** The label position relative to the checkbox. Defaults to 'after' */
@Input() labelPosition: 'before' | 'after' = 'after';
Methods blocks should describe what the function does and provide a description for each parameter and the return value:
/**
* Opens a modal dialog containing the given component.
* @param component Type of the component to load into the dialog.
* @param config Dialog configuration options.
* @returns Reference to the newly-opened dialog.
*/
open<T>(component: ComponentType<T>, config?: Md2DialogConfig): Md2DialogRef<T> { ... }
Boolean properties and return values should use "Whether..." as opposed to "True if...":
/** Whether the button is disabled. */
disabled: boolean = false;
- Prefer writing out words instead of using abbreviations.
- Prefer exact names over short names (within reason). E.g.,
labelPosition
is better thanalign
because the former much more exactly communicates what the property means. - Except for
@Input
properties, useis
andhas
prefixes for boolean properties / methods.
Classes should be named based on what they're responsible for. Names should capture what the code does, not how it is used:
/** NO: */
class RadioService { }
/** YES: */
class UniqueSelectionDispatcher { }
Avoid suffixing a class with "Service", as it communicates nothing about what the class does. Try to think of the class name as a person's job title.
The name of a method should capture the action that is performed by that method.
Prefer using the host
object in the directive configuration instead of @HostBinding
and
@HostListener
. We do this because TypeScript preserves the type information of methods with
decorators, and when one of the arguments for the method is a native Event
type, this preserved
type information can lead to runtime errors in non-browser environments (e.g., server-side
pre-rendering).
- The baseline calculation for flex elements is different than other display values, making it difficult to align flex elements with standard elements like input and button.
- Component outermost elements are never flex (block or inline-block)
- Don't use
display: flex
on elements that will contain projected content.
Always prioritize lower specificity over other factors. Most style definitions should consist of a single element or css selector plus necessary state modifiers. Avoid SCSS nesting for the sake of code organization. This will allow users to much more easily override styles.
For example, rather than doing this:
.md2-calendar {
display: block;
.md2-month {
display: inline-block;
.md2-date.md2-selected {
font-weight: bold;
}
}
}
do this:
.md2-calendar {
display: block;
}
.md2-calendar-month {
display: inline-block;
}
.md2-calendar-date.md2-selected {
font-weight: bold;
}
The end-user of a component should be the one to decide how much margin a component has around it.
This makes it easier to override styles when necessary. For example, rather than
the-host-element {
// ...
.some-child-element {
color: red;
}
}
you can write
the-host-element {
// ...
color: red;
}
The latter is equivalent for the component, but makes it easier override when necessary.
This is a low-effort task that makes a big difference for low-vision users. Example:
@media screen and (-ms-high-contrast: active) {
.unicorn-motocycle {
border: 1px solid #fff !important;
}
}
When it is not super obvious, include a brief description of what a class represents. For example:
// The calendar icon button used to open the calendar pane.
.md2-datepicker-button { ... }
// Floating pane that contains the calendar at the bottom of the input.
.md2-datepicker-calendar-pane { ... }
// Portion of the floating panel that sits, invisibly, on top of the input.
.md2-datepicker-input-mask { }