Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions apps/fabric-website/src/components/App/App.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import './App.scss';
import { AppState } from './AppState';
import { autobind } from 'office-ui-fabric-react/lib/Utilities';
import AttachedScrollUtility from '../../utilities/AttachedScrollUtility';
import { Fabric } from 'office-ui-fabric-react/lib/Fabric';
import { Nav } from '../Nav/Nav';
Expand Down Expand Up @@ -75,8 +74,7 @@ export class App extends React.Component<IAppProps, any> {
);
}

@autobind
private _handleNavPositioning() {
private _handleNavPositioning = () => {
let { isAttached, navHeight } = this.state;
this._appContentRect = this._appContent && this._appContent.getBoundingClientRect();
const viewPortHeight = window.innerHeight;
Expand Down
5 changes: 2 additions & 3 deletions apps/todo-app/src/components/TodoForm.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { autobind, BaseComponent, IBaseProps, createRef } from 'office-ui-fabric-react/lib/Utilities';
import { BaseComponent, IBaseProps, createRef } from 'office-ui-fabric-react/lib/Utilities';
import { PrimaryButton } from 'office-ui-fabric-react/lib/Button';
import { TextField, ITextField } from 'office-ui-fabric-react/lib/TextField';
import * as stylesImport from './Todo.scss';
Expand Down Expand Up @@ -78,8 +78,7 @@ export default class TodoForm extends BaseComponent<ITodoFormProps, ITodoFormSta
);
}

@autobind
private _onSubmit(event: React.FormEvent<HTMLElement>): void {
private _onSubmit = (event: React.FormEvent<HTMLElement>): void => {
event.preventDefault();

const { value: textField } = this._textField;
Expand Down
5 changes: 2 additions & 3 deletions apps/todo-app/src/components/TodoTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from 'office-ui-fabric-react/lib/Pivot';
import { FocusZone, FocusZoneDirection } from 'office-ui-fabric-react/lib/FocusZone';
import { List } from 'office-ui-fabric-react/lib/List';
import { KeyCodes, autobind } from 'office-ui-fabric-react/lib/Utilities';
import { KeyCodes } from 'office-ui-fabric-react/lib/Utilities';
import TodoItem from './TodoItem';
import { ITodoItem, ITodoItemProps, ITodoTabsProps } from '../types/index';

Expand Down Expand Up @@ -73,8 +73,7 @@ export default class TodoTabs extends React.Component<ITodoTabsProps, {}> {
);
}

@autobind
private _isInnerZoneKeystroke(ev: React.KeyboardEvent<HTMLElement>): boolean {
private _isInnerZoneKeystroke = (ev: React.KeyboardEvent<HTMLElement>): boolean => {
return ev.which === KeyCodes.right;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@uifabric/example-app-base",
"comment": "Use arrow function properties instead of @autobind",
"type": "patch"
}
],
"packageName": "@uifabric/example-app-base",
"email": "mark@thedutchies.com"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@uifabric/experiments",
"comment": "Use arrow function properties instead of @autobind",
"type": "patch"
}
],
"packageName": "@uifabric/experiments",
"email": "mark@thedutchies.com"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@uifabric/fabric-website",
"comment": "Use arrow function properties instead of @autobind",
"type": "patch"
}
],
"packageName": "@uifabric/fabric-website",
"email": "mark@thedutchies.com"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@uifabric/utilities",
"comment": "Use arrow function properties instead of @autobind",
"type": "patch"
}
],
"packageName": "@uifabric/utilities",
"email": "mark@thedutchies.com"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "Use arrow function properties instead of @autobind",
"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "mark@thedutchies.com"
}
11 changes: 11 additions & 0 deletions common/changes/todo-app/retire-autobind_2018-03-17-15-50.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "todo-app",
"comment": "Use arrow function properties instead of @autobind",
"type": "patch"
}
],
"packageName": "todo-app",
"email": "mark@thedutchies.com"
}
30 changes: 23 additions & 7 deletions ghdocs/BESTPRACTICES.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,38 @@ BAD:
GOOD:
`onRenderItem: IRenderFunction<IItemProps>`

## Use @autobind. Avoid ALL binds in templates
## Use arrow function properties to avoid ALL binds in templates

The autobind decorator simplifies making event callbacks "bound" to the instance. It's easy to use:
When we use bind in a template, it means that the function is recreated every time, which is an anti-pattern.

BAD:
```typescript
class Foo {

@autobind
public _onClick(ev: React.MouseEvent) {
}

render() {
<button onClick={this._onClick.bind(this)}>Click me</button
}

}
```

When we use bind in a template, it means that the function is recreated every time, which is an anti-pattern.
Instead we can use an arrow function property as it will always be bound to the component.

GOOD:
```typescript
class Foo {

public _onClick = (ev: React.MouseEvent) => {
}

render() {
<button onClick={this._onClick}>Click me</button
}
}
```

## For gathering refs, avoid string refs, use resolve functions

Expand Down Expand Up @@ -189,7 +206,7 @@ class Foo extends BaseComponent<...> {
// Set the reference by passing the reference object as the ref prop
return <button ref={ _root } onClick={this._onClick} />;
}

private _onClick() {
// Access the reference through the .value property on the reference object
this._root.value.focus();
Expand Down Expand Up @@ -270,8 +287,7 @@ class ItemComponent extends React.Component<...> {
);
}

@autobind
_onClick(ev: MouseEvent) {
_onClick = (ev: MouseEvent) => {

}
}
Expand Down
10 changes: 3 additions & 7 deletions packages/example-app-base/src/components/App/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react';
import {
css
} from 'office-ui-fabric-react/lib/Utilities';
import { autobind } from 'office-ui-fabric-react/lib/Utilities';
import { Fabric } from 'office-ui-fabric-react/lib/Fabric';
import { Panel, PanelType } from 'office-ui-fabric-react/lib/Panel';
import { Nav } from 'office-ui-fabric-react/lib/Nav';
Expand Down Expand Up @@ -108,19 +107,16 @@ export class App extends React.Component<IAppProps, IAppState> {
);
}

@autobind
private _onIsMenuVisibleChanged(isMenuVisible: boolean): void {
private _onIsMenuVisibleChanged = (isMenuVisible: boolean): void => {
this.setState({ isMenuVisible });
}

@autobind
private _onLinkClick(): void {
private _onLinkClick = (): void => {
this.setState({ isMenuVisible: false });
}

@autobind
// tslint:disable-next-line:no-any
private _onRenderLink(link: INavLink): any {
private _onRenderLink = (link: INavLink): any => {
return (
[
<span key={ 1 } className='Nav-linkText'>{ link.name }</span>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react';
import {
BaseComponent,
css,
autobind,
customizable
} from 'office-ui-fabric-react/lib/Utilities';
import {
Expand Down Expand Up @@ -157,8 +156,7 @@ export class CommandBarBase extends BaseComponent<ICommandBarProps, {}> implemen
return [primaryKey, farKey, overflowKey].join(' ');
}

@autobind
private _onReduceData(data: ICommandBarData): ICommandBarData | undefined {
private _onReduceData = (data: ICommandBarData): ICommandBarData | undefined => {
const { endAligned, onDataReduced } = this.props;
let { primaryItems, overflowItems, cacheKey, farItems } = data;

Expand All @@ -182,8 +180,7 @@ export class CommandBarBase extends BaseComponent<ICommandBarProps, {}> implemen
return undefined;
}

@autobind
private _onGrowData(data: ICommandBarData): ICommandBarData | undefined {
private _onGrowData = (data: ICommandBarData): ICommandBarData | undefined => {
const { endAligned, onDataGrown } = this.props;
let { primaryItems, overflowItems, cacheKey, minimumOverflowItems, farItems } = data;
const movedItem = overflowItems[0];
Expand All @@ -207,8 +204,7 @@ export class CommandBarBase extends BaseComponent<ICommandBarProps, {}> implemen
return undefined;
}

@autobind
private _onRenderItems(item: ICommandBarItemProps): JSX.Element | React.ReactNode {
private _onRenderItems = (item: ICommandBarItemProps): JSX.Element | React.ReactNode => {
let { buttonStyles } = this.props;

if (item.onRender) {
Expand All @@ -235,8 +231,7 @@ export class CommandBarBase extends BaseComponent<ICommandBarProps, {}> implemen
return this._onRenderButton(commandButtonProps);
}

@autobind
private _onRenderButton(props: ICommandBarItemProps): JSX.Element {
private _onRenderButton = (props: ICommandBarItemProps): JSX.Element => {
// tslint:disable-next-line:no-any
return <CommandBarButton { ...props as any } />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react';
import {
BaseComponent,
KeyCodes,
autobind,
css,
} from '../../Utilities';
import {
Expand Down Expand Up @@ -133,8 +132,7 @@ export class BaseExtendedPicker<T, P extends IBaseExtendedPickerProps<T>> extend
);
}

@autobind
protected onSelectionChange(): void {
protected onSelectionChange = (): void => {
this.forceUpdate();
}

Expand Down Expand Up @@ -179,13 +177,11 @@ export class BaseExtendedPicker<T, P extends IBaseExtendedPickerProps<T>> extend
}
}

@autobind
protected onInputChange(value: string): void {
protected onInputChange = (value: string): void => {
this.floatingPicker.onQueryStringChanged(value);
}

@autobind
protected onInputFocus(ev: React.FocusEvent<HTMLInputElement | Autofill>): void {
protected onInputFocus = (ev: React.FocusEvent<HTMLInputElement | Autofill>): void => {
this.selectedItemsList.unselectAll();
this.floatingPicker.showPicker();

Expand All @@ -196,8 +192,7 @@ export class BaseExtendedPicker<T, P extends IBaseExtendedPickerProps<T>> extend

// This is protected because we may expect the backspace key to work differently in a different kind of picker.
// This lets the subclass override it and provide it's own onBackspace. For an example see the BasePickerListBelow
@autobind
protected onBackspace(ev: React.KeyboardEvent<HTMLElement>): void {
protected onBackspace = (ev: React.KeyboardEvent<HTMLElement>): void => {
if (ev.which !== KeyCodes.backspace) {
return;
}
Expand All @@ -209,23 +204,20 @@ export class BaseExtendedPicker<T, P extends IBaseExtendedPickerProps<T>> extend
}
}

@autobind
protected onCopy(ev: React.ClipboardEvent<HTMLElement>): void {
protected onCopy = (ev: React.ClipboardEvent<HTMLElement>): void => {
// Pass it down into the selected items list
this.selectedItemsList.onCopy(ev);
}

@autobind
protected onPaste(ev: React.ClipboardEvent<Autofill | HTMLInputElement>): void {
protected onPaste = (ev: React.ClipboardEvent<Autofill | HTMLInputElement>): void => {
if (this.props.onPaste) {
let inputText = ev.clipboardData.getData('Text');
ev.preventDefault();
this.props.onPaste(inputText);
}
}

@autobind
protected _isFocusZoneInnerKeystroke(ev: React.KeyboardEvent<HTMLElement>): boolean {
protected _isFocusZoneInnerKeystroke = (ev: React.KeyboardEvent<HTMLElement>): boolean => {
// If suggestions are shown let up/down keys control them, otherwise allow them through to control the focusZone.
if (this.floatingPicker.isSuggestionsShown) {
switch (ev.which) {
Expand All @@ -243,8 +235,7 @@ export class BaseExtendedPicker<T, P extends IBaseExtendedPickerProps<T>> extend
return false;
}

@autobind
protected _onSuggestionSelected(item: T): void {
protected _onSuggestionSelected = (item: T): void => {
this.selectedItemsList.addItems([item]);
if (this.props.onItemSelected) {
this.props.onItemSelected(item);
Expand All @@ -255,8 +246,7 @@ export class BaseExtendedPicker<T, P extends IBaseExtendedPickerProps<T>> extend
this.focus();
}

@autobind
protected _onSelectedItemsChanged(): void {
protected _onSelectedItemsChanged = (): void => {
this.focus();
}
}
Loading