diff --git a/ghdocs/BESTPRACTICES.md b/ghdocs/BESTPRACTICES.md index bcf01c605c34f..203dd70c0aed6 100644 --- a/ghdocs/BESTPRACTICES.md +++ b/ghdocs/BESTPRACTICES.md @@ -187,10 +187,6 @@ Note that it's very critical you do NOT inline the functions in render! This cau is recreated. If you do this, react will call your function with null to clear it, and then render, and then once complete, will call it again with the element. This creates timing issues where your ref may be null when you didn't expect. -See example here that illustates refs being re-evaluated: - -http://codepen.io/dzearing/pen/WGZOaR - ## Use unique keys for items in a mapped array, avoid indexes for keys ```typescript diff --git a/packages/office-ui-fabric-react/src/components/Breadcrumb/Breadcrumb.tsx b/packages/office-ui-fabric-react/src/components/Breadcrumb/Breadcrumb.tsx index 6c6409e6733dc..fa3dc54b318b7 100644 --- a/packages/office-ui-fabric-react/src/components/Breadcrumb/Breadcrumb.tsx +++ b/packages/office-ui-fabric-react/src/components/Breadcrumb/Breadcrumb.tsx @@ -83,14 +83,13 @@ export class Breadcrumb extends BaseComponent { return (
    { renderedOverflowItems && renderedOverflowItems.length !== 0 && ( -
  1. +
  2. { ) } { renderedItems.map( (item, index) => ( -
  3. +
  4. { onRenderItem(item, this._onRenderItem) } impl workWeekDays: defaultWorkWeekDays }; - public refs: { - [key: string]: React.ReactInstance; - root: HTMLElement; - dayPicker: CalendarDay; - monthPicker: CalendarMonth; - }; + public root: HTMLElement; + public dayPicker: CalendarDay; + public monthPicker: CalendarMonth; private _focusOnUpdate: boolean; @@ -116,10 +113,10 @@ export class Calendar extends BaseComponent impl public componentDidUpdate() { if (this._focusOnUpdate) { // if the day picker is shown, focus on it - if (this.refs.dayPicker) { - this.refs.dayPicker.focus(); - } else if (this.refs.monthPicker) { - this.refs.monthPicker.focus(); + if (this.dayPicker) { + this.dayPicker.focus(); + } else if (this.monthPicker) { + this.monthPicker.focus(); } this._focusOnUpdate = false; } @@ -134,7 +131,7 @@ export class Calendar extends BaseComponent impl const overlayedWithButton = showMonthPickerAsOverlay && showGoToToday; return ( -
    +
    impl minDate={ minDate } maxDate={ maxDate } workWeekDays={ this.props.workWeekDays } - ref='dayPicker' + ref={ this._resolveRef('_dayPicker') } /> } @@ -185,7 +182,7 @@ export class Calendar extends BaseComponent impl dateTimeFormatter={ this.props.dateTimeFormatter! } minDate={ minDate } maxDate={ maxDate } - ref='monthPicker' + ref={ this._resolveRef('monthPicker') } /> } { showGoToToday && @@ -208,8 +205,8 @@ export class Calendar extends BaseComponent impl } public focus() { - if (this.refs.dayPicker) { - this.refs.dayPicker.focus(); + if (this.dayPicker) { + this.dayPicker.focus(); } } diff --git a/packages/office-ui-fabric-react/src/components/ColorPicker/ColorRectangle.tsx b/packages/office-ui-fabric-react/src/components/ColorPicker/ColorRectangle.tsx index 49b88e4b3d5fa..292182370cd44 100644 --- a/packages/office-ui-fabric-react/src/components/ColorPicker/ColorRectangle.tsx +++ b/packages/office-ui-fabric-react/src/components/ColorPicker/ColorRectangle.tsx @@ -35,10 +35,7 @@ export class ColorRectangle extends BaseComponent +
    @@ -90,7 +87,7 @@ export class ColorRectangle extends BaseComponent) { const { color, onSVChanged } = this.props; - const rectSize = this.refs.root.getBoundingClientRect(); + const rectSize = this.root.getBoundingClientRect(); const sPercentage = (ev.clientX - rectSize.left) / rectSize.width; const vPercentage = (ev.clientY - rectSize.top) / rectSize.height; diff --git a/packages/office-ui-fabric-react/src/components/ColorPicker/ColorSlider.tsx b/packages/office-ui-fabric-react/src/components/ColorPicker/ColorSlider.tsx index 4f453776680be..e2a96508f88d4 100644 --- a/packages/office-ui-fabric-react/src/components/ColorPicker/ColorSlider.tsx +++ b/packages/office-ui-fabric-react/src/components/ColorPicker/ColorSlider.tsx @@ -34,10 +34,7 @@ export class ColorSlider extends BaseComponent) { const { onChanged, minValue, maxValue } = this.props; - const rectSize = this.refs.root.getBoundingClientRect(); + const rectSize = this._root.getBoundingClientRect(); const currentPercentage = (ev.clientX - rectSize.left) / rectSize.width; const newValue = Math.min(maxValue!, Math.max(minValue!, currentPercentage * maxValue!)); diff --git a/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx b/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx index fb211aefa3c85..9501383589374 100644 --- a/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx +++ b/packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx @@ -89,10 +89,7 @@ export class ComboBox extends BaseComponent { buttonIconProps: { iconName: 'ChevronDown' } }; - public refs: { - [key: string]: React.ReactInstance, - root: HTMLElement - }; + private _root: HTMLElement; // The input aspect of the comboBox private _comboBox: Autofill; @@ -306,7 +303,7 @@ export class ComboBox extends BaseComponent { ); return ( -
    +
    { label && ( ) } @@ -784,7 +781,7 @@ export class ComboBox extends BaseComponent { // inside the comboBox root or the comboBox menu since // it we are not really bluring from the whole comboBox if (event.relatedTarget && - (this.refs.root && this.refs.root.contains(event.relatedTarget as HTMLElement) || + (this._root && this._root.contains(event.relatedTarget as HTMLElement) || this._comboBoxMenu && this._comboBoxMenu.contains(event.relatedTarget as HTMLElement))) { event.preventDefault(); event.stopPropagation(); diff --git a/packages/office-ui-fabric-react/src/components/CommandBar/CommandBar.tsx b/packages/office-ui-fabric-react/src/components/CommandBar/CommandBar.tsx index 537a81c7fdb40..19f7244a8808d 100644 --- a/packages/office-ui-fabric-react/src/components/CommandBar/CommandBar.tsx +++ b/packages/office-ui-fabric-react/src/components/CommandBar/CommandBar.tsx @@ -44,13 +44,15 @@ export class CommandBar extends BaseComponent +
    +
    { searchBox } - -
    + +
    { renderedItems!.map(item => ( this._renderItemInCommandBar(item, posInSet++, setSize, expandedMenuItemKey!) )).concat((renderedOverflowItems && renderedOverflowItems.length) ? [ -
    +
    -
    +
    { renderedFarItems!.map(item => ( this._renderItemInCommandBar(item, posInSet++, setSize, expandedMenuItemKey!, true) )) } @@ -165,7 +167,7 @@ export class CommandBar extends BaseComponent { - public refs: { - [key: string]: React.ReactInstance; - menuButton: HTMLElement; - gapSize: TextField; - }; - public constructor(props: {}) { super(props); @@ -95,7 +89,7 @@ export class ContextualMenuDirectionalExample extends React.Component<{}, IConte /> }
    -
    +
    { - public refs: { - [key: string]: React.ReactInstance, - cellMeasurer: HTMLElement - }; - private _root: HTMLElement | undefined; + private _cellMeasurer: HTMLElement; private _focusZone: IFocusZone; private _hasSetFocus: boolean; private _droppingClassNames: string; @@ -139,7 +135,7 @@ export class DetailsRow extends BaseComponent= 0) { - const newWidth = this.refs.cellMeasurer.getBoundingClientRect().width; + const newWidth = this._cellMeasurer.getBoundingClientRect().width; columnMeasureInfo.onMeasureDone(newWidth); @@ -205,7 +201,7 @@ export class DetailsRow extends BaseComponent { - public refs: { - [key: string]: React.ReactInstance; - list: DetailsList - }; - private _isFetchingItems: boolean; private _selection: Selection; @@ -124,7 +119,6 @@ export class DetailsListAdvancedExample extends React.Component<{}, IDetailsList } implem private static _focusStack: FocusTrapZone[] = []; private static _clickStack: FocusTrapZone[] = []; - public refs: { - [key: string]: React.ReactInstance, - root: HTMLElement - }; - + private _root: HTMLElement; private _previouslyFocusedElement: HTMLElement; private _isInFocusStack: boolean = false; private _isInClickStack: boolean = false; @@ -42,7 +38,7 @@ export class FocusTrapZone extends BaseComponent implem const { isClickableOutsideFocusTrap = false, forceFocusInsideTrap = true, elementToFocusOnDismiss, disableFirstFocus = false } = this.props; this._previouslyFocusedElement = elementToFocusOnDismiss ? elementToFocusOnDismiss : document.activeElement as HTMLElement; - if (!elementContains(this.refs.root, this._previouslyFocusedElement) && !disableFirstFocus) { + if (!elementContains(this._root, this._previouslyFocusedElement) && !disableFirstFocus) { this.focus(); } @@ -91,7 +87,7 @@ export class FocusTrapZone extends BaseComponent implem
    @@ -110,12 +106,11 @@ export class FocusTrapZone extends BaseComponent implem : firstFocusableSelector && firstFocusableSelector(); let _firstFocusableChild; - const root = this.refs.root; if (focusSelector) { - _firstFocusableChild = root.querySelector('.' + focusSelector); + _firstFocusableChild = this._root.querySelector('.' + focusSelector); } else { - _firstFocusableChild = getNextElement(root, root.firstChild as HTMLElement, true, false, false, true); + _firstFocusableChild = getNextElement(this._root, this._root.firstChild as HTMLElement, true, false, false, true); } if (_firstFocusableChild) { (_firstFocusableChild as any).focus(); @@ -128,10 +123,8 @@ export class FocusTrapZone extends BaseComponent implem return; } - const { root } = this.refs; - - const _firstFocusableChild = getFirstFocusable(root, root.firstChild as HTMLElement, true); - const _lastFocusableChild = getLastTabbable(root, root.lastChild as HTMLElement, true); + const _firstFocusableChild = getFirstFocusable(this._root, this._root.firstChild as HTMLElement, true); + const _lastFocusableChild = getLastTabbable(this._root, this._root.lastChild as HTMLElement, true); if (ev.shiftKey && _firstFocusableChild === ev.target) { _lastFocusableChild!.focus(); @@ -148,7 +141,7 @@ export class FocusTrapZone extends BaseComponent implem if (FocusTrapZone._focusStack.length && this === FocusTrapZone._focusStack[FocusTrapZone._focusStack.length - 1]) { const focusedElement = document.activeElement as HTMLElement; - if (!elementContains(this.refs.root, focusedElement)) { + if (!elementContains(this._root, focusedElement)) { this.focus(); ev.preventDefault(); ev.stopPropagation(); @@ -160,7 +153,7 @@ export class FocusTrapZone extends BaseComponent implem if (FocusTrapZone._clickStack.length && this === FocusTrapZone._clickStack[FocusTrapZone._clickStack.length - 1]) { const clickedElement = ev.target as HTMLElement; - if (clickedElement && !elementContains(this.refs.root, clickedElement)) { + if (clickedElement && !elementContains(this._root, clickedElement)) { this.focus(); ev.preventDefault(); ev.stopPropagation(); diff --git a/packages/office-ui-fabric-react/src/components/GroupedList/GroupedList.tsx b/packages/office-ui-fabric-react/src/components/GroupedList/GroupedList.tsx index a3db445bee95a..26f914eb1d8b4 100644 --- a/packages/office-ui-fabric-react/src/components/GroupedList/GroupedList.tsx +++ b/packages/office-ui-fabric-react/src/components/GroupedList/GroupedList.tsx @@ -38,10 +38,10 @@ export class GroupedList extends BaseComponent number): void { - this.refs.list && this.refs.list.scrollToIndex(index, measureItem); + this._list && this._list.scrollToIndex(index, measureItem); } public componentWillReceiveProps(newProps: IGroupedListProps) { @@ -92,7 +92,6 @@ export class GroupedList extends BaseComponent @@ -249,11 +248,11 @@ export class GroupedList extends BaseComponent { - public refs: { - [key: string]: React.ReactInstance, - root: HTMLElement, - list: List - }; + private _root: HTMLElement; + private _list: List; private _subGroups: { [key: string]: GroupedListSection; @@ -151,7 +148,7 @@ export class GroupedListSection extends BaseComponent @@ -227,7 +224,7 @@ export class GroupedListSection extends BaseComponent 0) { const subGroupCount = group.children.length; for (let i = 0; i < subGroupCount; i++) { - const subGroup = this.refs.list.refs['subGroup_' + String(i)] as GroupedListSection; + const subGroup = this._list.refs['subGroup_' + String(i)] as GroupedListSection; if (subGroup) { subGroup.forceListUpdate(); @@ -325,7 +322,7 @@ export class GroupedListSection extends BaseComponent diff --git a/packages/office-ui-fabric-react/src/components/List/List.tsx b/packages/office-ui-fabric-react/src/components/List/List.tsx index 9ab99cc74aed8..e09da24fb1883 100644 --- a/packages/office-ui-fabric-react/src/components/List/List.tsx +++ b/packages/office-ui-fabric-react/src/components/List/List.tsx @@ -88,10 +88,11 @@ export class List extends BaseComponent implements IList public refs: { [key: string]: React.ReactInstance, - root: HTMLElement, - surface: HTMLElement }; + private _root: HTMLElement; + private _surface: HTMLElement; + private _estimatedPageHeight: number; private _totalEstimates: number; private _cachedPageHeights: { @@ -262,10 +263,10 @@ export class List extends BaseComponent implements IList this._updatePages(); this._measureVersion++; - this._scrollElement = findScrollableParent(this.refs.root) as HTMLElement; + this._scrollElement = findScrollableParent(this._root) as HTMLElement; this._events.on(window, 'resize', this._onAsyncResize); - this._events.on(this.refs.root, 'focus', this._onFocus, true); + this._events.on(this._root, 'focus', this._onFocus, true); if (this._scrollElement) { this._events.on(this._scrollElement, 'scroll', this._onScroll); this._events.on(this._scrollElement, 'scroll', this._onAsyncScroll); @@ -342,16 +343,14 @@ export class List extends BaseComponent implements IList pageElements.push(this._renderPage(page)); } - // console.log(`Page elements ${pageElements.length}`); - return (
    -
    +
    { pageElements }
    @@ -478,7 +477,7 @@ export class List extends BaseComponent implements IList private _onFocus(ev: any) { let target = ev.target as HTMLElement; - while (target !== this.refs.surface) { + while (target !== this._surface) { const indexString = target.getAttribute('data-list-index'); if (indexString) { @@ -939,7 +938,7 @@ export class List extends BaseComponent implements IList !scrollHeight || scrollHeight !== this._scrollHeight || Math.abs(this._scrollTop - scrollTop) > this._estimatedPageHeight / 3) { - surfaceRect = this._surfaceRect = _measureSurfaceRect(this.refs.surface); + surfaceRect = this._surfaceRect = _measureSurfaceRect(this._surface); this._scrollTop = scrollTop; } diff --git a/packages/office-ui-fabric-react/src/components/MarqueeSelection/MarqueeSelection.tsx b/packages/office-ui-fabric-react/src/components/MarqueeSelection/MarqueeSelection.tsx index 72a441851f777..4ba53eb325c61 100644 --- a/packages/office-ui-fabric-react/src/components/MarqueeSelection/MarqueeSelection.tsx +++ b/packages/office-ui-fabric-react/src/components/MarqueeSelection/MarqueeSelection.tsx @@ -38,11 +38,7 @@ export class MarqueeSelection extends BaseComponent { children } { dragRect && (
    ) } @@ -177,9 +173,9 @@ export class MarqueeSelection extends BaseComponent { shouldRestoreFocus: true }; - public refs: { - [key: string]: React.ReactInstance; - root: HTMLElement; - }; + public _root: HTMLDivElement; private _originalFocusedElement: HTMLElement; private _containsFocus: boolean; @@ -32,9 +29,9 @@ export class Popup extends BaseComponent { } public componentDidMount(): void { - this._events.on(this.refs.root, 'focus', this._onFocus, true); - this._events.on(this.refs.root, 'blur', this._onBlur, true); - if (doesElementContainFocus(this.refs.root)) { + this._events.on(this._root, 'focus', this._onFocus, true); + this._events.on(this._root, 'blur', this._onBlur, true); + if (doesElementContainFocus(this._root)) { this._containsFocus = true; } } @@ -58,13 +55,13 @@ export class Popup extends BaseComponent { const { role, className, ariaLabel, ariaLabelledBy, ariaDescribedBy, style } = this.props; let needsVerticalScrollBar = false; - if (this.refs.root && this.refs.root.firstElementChild) { - needsVerticalScrollBar = this.refs.root.firstElementChild.clientHeight > this.refs.root.clientHeight; + if (this._root && this._root.firstElementChild) { + needsVerticalScrollBar = this._root.firstElementChild.clientHeight > this._root.clientHeight; } return (
    scrollablePane: PropTypes.object }; - public refs: { - root: HTMLElement; - stickyContainer: HTMLElement; - stickyAbove: HTMLElement; - stickyBelow: HTMLElement; - }; - + public root: HTMLElement; + public stickyContainer: HTMLElement; + public stickyAbove: HTMLElement; + public stickyBelow: HTMLElement; private _subscribers: Set; private _stickyAbove: Set; private _stickyBelow: Set; @@ -62,26 +59,23 @@ export class ScrollablePaneBase extends BaseComponent } public componentDidMount() { - const { root, stickyContainer } = this.refs; - - this._events.on(root, 'scroll', this.notifySubscribers); + this._events.on(this.root, 'scroll', this.notifySubscribers); this._events.on(window, 'resize', this._onWindowResize); setTimeout(() => { this._resizeContainer(); - if (stickyContainer.parentElement && root.parentElement) { - stickyContainer.parentElement.removeChild(stickyContainer); - root.parentElement.insertBefore(stickyContainer, root.nextSibling); + if (this.stickyContainer.parentElement && this.root.parentElement) { + this.stickyContainer.parentElement.removeChild(this.stickyContainer); + this.root.parentElement.insertBefore(this.stickyContainer, this.root.nextSibling); this.notifySubscribers(); } }, 500); } public componentWillUnmount() { - const { stickyContainer } = this.refs; - this._events.off(this.refs.root); + this._events.off(this.root); this._events.off(window); - if (stickyContainer.parentElement) { - stickyContainer.parentElement.removeChild(stickyContainer); + if (this.stickyContainer.parentElement) { + this.stickyContainer.parentElement.removeChild(this.stickyContainer); } } @@ -96,13 +90,13 @@ export class ScrollablePaneBase extends BaseComponent return (
    -
    -
    -
    +
    +
    +
    { this.props.children }
    @@ -125,41 +119,38 @@ export class ScrollablePaneBase extends BaseComponent @autobind public addStickyHeader(sticky: Sticky) { - const { stickyAbove } = this.refs; - this._addSticky(sticky, this._stickyAbove, stickyAbove, () => { - stickyAbove.appendChild(sticky.content); + this._addSticky(sticky, this._stickyAbove, this.stickyAbove, () => { + this.stickyAbove.appendChild(sticky.content); }); } @autobind public addStickyFooter(sticky: Sticky) { - const { stickyBelow } = this.refs; - this._addSticky(sticky, this._stickyBelow, stickyBelow, () => { - stickyBelow.insertBefore(sticky.content, stickyBelow.firstChild); + this._addSticky(sticky, this._stickyBelow, this.stickyBelow, () => { + this.stickyBelow.insertBefore(sticky.content, this.stickyBelow.firstChild); }); } @autobind public removeStickyHeader(sticky: Sticky) { - this._removeSticky(sticky, this._stickyAbove, this.refs.stickyAbove); + this._removeSticky(sticky, this._stickyAbove, this.stickyAbove); } @autobind public removeStickyFooter(sticky: Sticky) { - this._removeSticky(sticky, this._stickyBelow, this.refs.stickyBelow); + this._removeSticky(sticky, this._stickyBelow, this.stickyBelow); } @autobind public notifySubscribers(sort?: boolean): void { - const { stickyAbove, stickyBelow } = this.refs; this._subscribers.forEach((handle) => { - handle(stickyAbove.getBoundingClientRect(), stickyBelow.getBoundingClientRect()); + handle(this.stickyAbove.getBoundingClientRect(), this.stickyBelow.getBoundingClientRect()); }); if (this._stickyAbove.size > 1) { - this._sortStickies(this._stickyAbove, stickyAbove); + this._sortStickies(this._stickyAbove, this.stickyAbove); } if (this._stickyBelow.size > 1) { - this._sortStickies(this._stickyBelow, stickyBelow); + this._sortStickies(this._stickyBelow, this.stickyBelow); } } @@ -200,7 +191,7 @@ export class ScrollablePaneBase extends BaseComponent } private _resizeContainer() { - const { stickyContainer, root } = this.refs; + const { stickyContainer, root } = this; const { borderTopWidth, borderLeftWidth } = getComputedStyle(root); stickyContainer.style.height = root.clientHeight + 'px'; stickyContainer.style.width = root.clientWidth + 'px'; @@ -222,8 +213,8 @@ export class ScrollablePaneBase extends BaseComponent private _sortStickies(stickyList: Set, container: HTMLElement): void { let stickyArr = Array.from(stickyList); stickyArr = stickyArr.sort((a, b) => { - const aOffset = this._calculateOffsetParent(a.refs.root); - const bOffset = this._calculateOffsetParent(b.refs.root); + const aOffset = this._calculateOffsetParent(a.root); + const bOffset = this._calculateOffsetParent(b.root); return aOffset - bOffset; }); while (container.lastChild) { @@ -236,7 +227,7 @@ export class ScrollablePaneBase extends BaseComponent private _calculateOffsetParent(ele: HTMLElement): number { let offset = 0; - while (ele.offsetParent !== this.refs.root.offsetParent) { + while (ele.offsetParent !== this.root.offsetParent) { offset += ele.offsetTop; if (ele.parentElement) { ele = ele.parentElement; diff --git a/packages/office-ui-fabric-react/src/components/Slider/Slider.tsx b/packages/office-ui-fabric-react/src/components/Slider/Slider.tsx index 784b057a35214..b4303db6412e4 100644 --- a/packages/office-ui-fabric-react/src/components/Slider/Slider.tsx +++ b/packages/office-ui-fabric-react/src/components/Slider/Slider.tsx @@ -34,13 +34,8 @@ export class Slider extends BaseComponent implements buttonProps: {} }; - public refs: { - [key: string]: React.ReactInstance, - root: HTMLElement, - sliderLine: HTMLElement, - thumb: HTMLElement - }; - + private _sliderLine: HTMLDivElement; + private _thumb: HTMLSpanElement; private _id: string; constructor(props: ISliderProps) { @@ -102,7 +97,6 @@ export class Slider extends BaseComponent implements ['ms-Slider-row ' + styles.rootIsHorizontal]: !vertical, ['ms-Slider-column ' + styles.rootIsVertical]: vertical }) } - ref='root' > { label && (