From 08fd1c88b7ea0cfa59ec72ce169ad952b48afffc Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Tue, 11 Aug 2020 18:32:57 -0700 Subject: [PATCH 1/6] Enable eslint-plugin-react-hooks in react-next and fix a few bugs --- packages/eslint-plugin/src/configs/react.js | 8 +- packages/react-next/.eslintrc.json | 5 +- packages/react-next/RELEASE_NOTES.md | 31 ++- .../Callout/CalloutContent.base.tsx | 170 +++++++++------ .../src/components/Checkbox/useCheckbox.ts | 3 +- .../components/Coachmark/Coachmark.base.tsx | 203 ++++++++---------- .../PositioningContainer.tsx | 25 +-- .../src/components/Fabric/Fabric.base.tsx | 3 +- .../src/components/Image/Image.base.tsx | 27 +-- .../react-next/src/components/Link/useLink.ts | 2 +- .../src/components/Persona/Persona.base.tsx | 1 + .../Persona/PersonaCoin/PersonaCoin.base.tsx | 1 + .../react-next/src/components/Popup/Popup.tsx | 11 +- .../ResizeGroup/ResizeGroup.base.tsx | 1 + .../components/SearchBox/SearchBox.base.tsx | 96 +++++---- .../src/components/Slider/Slider.base.tsx | 1 + .../src/components/Slider/useSlider.ts | 2 +- .../components/SpinButton/SpinButton.base.tsx | 2 + .../src/components/Toggle/useToggle.ts | 6 +- packages/test-utilities/src/safeMount.ts | 5 +- 20 files changed, 312 insertions(+), 291 deletions(-) diff --git a/packages/eslint-plugin/src/configs/react.js b/packages/eslint-plugin/src/configs/react.js index e81d00500233bd..1307e638158c1c 100644 --- a/packages/eslint-plugin/src/configs/react.js +++ b/packages/eslint-plugin/src/configs/react.js @@ -87,6 +87,13 @@ const config = { 'no-empty': 'error', 'no-eval': 'error', 'no-new-wrappers': 'error', + 'no-restricted-globals': [ + 'error', + ...['blur', 'close', 'focus', 'length', 'name', 'parent', 'self', 'stop'].map(name => ({ + name, + message: `"${name}" refers to a DOM global. Did you mean to reference a local value instead?`, + })), + ], 'no-restricted-properties': [ 'error', { object: 'describe', property: 'only', message: 'describe.only should only be used during test development' }, @@ -204,7 +211,6 @@ const config = { // permanently disable because we disagree with these rules 'import/prefer-default-export': 'off', 'no-await-in-loop': 'off', // contrary to rule docs, awaited things often are NOT parallelizable - 'no-restricted-globals': 'off', // airbnb bans isNaN in favor of Number.isNaN which is unavailable in IE 11 'react/jsx-props-no-spreading': 'off', 'react/prop-types': 'off', diff --git a/packages/react-next/.eslintrc.json b/packages/react-next/.eslintrc.json index f43801139eedbd..21e6cf0d004786 100644 --- a/packages/react-next/.eslintrc.json +++ b/packages/react-next/.eslintrc.json @@ -3,9 +3,6 @@ "extends": ["../office-ui-fabric-react/.eslintrc"], "root": true, "rules": { - "@typescript-eslint/no-explicit-any": "error", - // Disable until issues are dealt with - "react-hooks/exhaustive-deps": "off", - "react-hooks/rules-of-hooks": "off" + "@typescript-eslint/no-explicit-any": "error" } } diff --git a/packages/react-next/RELEASE_NOTES.md b/packages/react-next/RELEASE_NOTES.md index c4cb80bb3f94b9..11121a9df515ee 100644 --- a/packages/react-next/RELEASE_NOTES.md +++ b/packages/react-next/RELEASE_NOTES.md @@ -2,17 +2,9 @@ ## Breaking changes -### Beak +### Calendar -- Removed empty `IBeak` interface -- Removed `componentRef` prop - -### SpinButton - -- Simplified props to `ISpinButtonStyles` to include only the parts of the component to bring inline with - other components. -- Replaced `getClassNames` legacy prop with `styles` prop to bring component consistent to other components - and improve cachability of internal styles. +TODO: Diff of OUFR vs date-time Calendar ### Checkbox @@ -24,10 +16,27 @@ ### Coachmark - Removed `isBeaconAnimating` and `isMeasured` style props +- Beak: + - Removed empty `IBeak` interface + - Removed `componentRef` prop + +### DatePicker + +TODO: Diff of OUFR vs date-time DatePicker ### Pivot - Removed deprecated and redundant props from v7, including: `initialSelectedKey` and `defaultSelectedIndex`. Use `selectedKey` or `defaultSelectedKey` to define the selected tab, and provide `itemKey` on pivot item children. + - TODO: enumerate all removed props + +### Slider + +TODO: document any API or functionality changes + +### SpinButton + +- Simplified props to `ISpinButtonStyles` to include only the parts of the component to bring inline with other components. +- Replaced `getClassNames` legacy prop with `styles` prop to bring component consistent to other components and improve cachability of internal styles. ### Others @@ -45,7 +54,7 @@ - Updated enums to string union type: `PivotLinkFormat`, `PivotLinkSize`. (#13370) -## Changes worth callout +## Other notable changes - `styles` prop backward compat solution. - css variables and IE 11 solution. diff --git a/packages/react-next/src/components/Callout/CalloutContent.base.tsx b/packages/react-next/src/components/Callout/CalloutContent.base.tsx index 0c0721b590b928..cf77916979afcd 100644 --- a/packages/react-next/src/components/Callout/CalloutContent.base.tsx +++ b/packages/react-next/src/components/Callout/CalloutContent.base.tsx @@ -28,7 +28,7 @@ import { import { Popup } from '../../Popup'; import { classNamesFunction } from '../../Utilities'; import { AnimationClassNames } from '../../Styling'; -import { useMergedRefs, useAsync } from '@uifabric/react-hooks'; +import { useMergedRefs, useAsync, useConst } from '@uifabric/react-hooks'; const ANIMATIONS: { [key: number]: string | undefined } = { [RectangleEdge.top]: AnimationClassNames.slideUpIn10, @@ -142,7 +142,7 @@ function useBounds( cachedBounds.current = currentBounds; } return cachedBounds.current; - }, [bounds, minPagePadding, target]); + }, [bounds, minPagePadding, target, targetRef, targetWindowRef]); return getBounds; } @@ -174,6 +174,12 @@ function useMaxHeight( } else if (hidden) { setMaxHeight(undefined); } + // TODO: check with Michael + // - Missing dependencies: coverTarget, directionalHintFixed, isBeakVisible, maxHeight + // (and immutable values async, targetRef) + // - "Mutable values like 'targetRef.current' aren't valid dependencies because mutating them + // doesn't re-render the component" + // eslint-disable-next-line react-hooks/exhaustive-deps }, [targetRef.current, gapSpace, beakWidth, directionalHint, getBounds, hidden]); return maxHeight; @@ -215,6 +221,9 @@ function useHeightOffset({ finalHeight, hidden }: ICalloutProps, calloutElement: if (!hidden) { setHeightOffsetEveryFrame(); } + // TODO: check with Michael + // (missing dep on setHeightOffsetEveryFrame) + // eslint-disable-next-line react-hooks/exhaustive-deps }, [finalHeight, hidden]); return heightOffset; @@ -275,6 +284,10 @@ function usePositions( return () => async.cancelAnimationFrame(timerId); } + // TODO: check with Michael + // missing deps finalHeight, getBounds, onPositioned, positions, props, target + // (and immutable values async, calloutElement, hostElement, targetRef) + // eslint-disable-next-line react-hooks/exhaustive-deps }, [hidden, directionalHint]); return positions; @@ -289,8 +302,9 @@ function useAutoFocus( calloutElement: React.RefObject, ) { const async = useAsync(); + const hasPositions = !!positions; React.useEffect(() => { - if (!hidden && setInitialFocus && positions && calloutElement.current) { + if (!hidden && setInitialFocus && hasPositions && calloutElement.current) { const timerId = async.requestAnimationFrame( () => focusFirstChild(calloutElement.current!), calloutElement.current, @@ -298,7 +312,11 @@ function useAutoFocus( return () => async.cancelAnimationFrame(timerId); } - }, [hidden, !!positions]); + // TODO: check with Michael + // missing dep on setInitialFocus + // (and immutable values async, calloutElement) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [hidden, hasPositions]); } /** @@ -314,55 +332,56 @@ function useDismissHandlers( const isMouseDownOnPopup = React.useRef(false); const async = useAsync(); - const mouseDownOnPopup = () => { - isMouseDownOnPopup.current = true; - }; - - const mouseUpOnPopup = () => { - isMouseDownOnPopup.current = false; - }; + const mouseDownHandlers = useConst([ + () => { + isMouseDownOnPopup.current = true; + }, + () => { + isMouseDownOnPopup.current = false; + }, + ] as const); - const dismissOnScroll = (ev: Event) => { - if (positions && !preventDismissOnScroll) { - dismissOnClickOrScroll(ev); - } - }; + React.useEffect(() => { + const dismissOnScroll = (ev: Event) => { + if (positions && !preventDismissOnScroll) { + dismissOnClickOrScroll(ev); + } + }; - const dismissOnResize = (ev: Event) => { - if (!preventDismissOnResize) { - onDismiss?.(ev); - } - }; + const dismissOnResize = (ev: Event) => { + if (!preventDismissOnResize) { + onDismiss?.(ev); + } + }; - const dismissOnLostFocus = (ev: Event) => { - if (!preventDismissOnLostFocus) { - dismissOnClickOrScroll(ev); - } - }; + const dismissOnLostFocus = (ev: Event) => { + if (!preventDismissOnLostFocus) { + dismissOnClickOrScroll(ev); + } + }; - const dismissOnClickOrScroll = (ev: Event) => { - const target = ev.target as HTMLElement; - const isEventTargetOutsideCallout = hostElement.current && !elementContains(hostElement.current, target); + const dismissOnClickOrScroll = (ev: Event) => { + const target = ev.target as HTMLElement; + const isEventTargetOutsideCallout = hostElement.current && !elementContains(hostElement.current, target); - // If mouse is pressed down on callout but moved outside then released, don't dismiss the callout. - if (isEventTargetOutsideCallout && isMouseDownOnPopup.current) { - isMouseDownOnPopup.current = false; - return; - } + // If mouse is pressed down on callout but moved outside then released, don't dismiss the callout. + if (isEventTargetOutsideCallout && isMouseDownOnPopup.current) { + isMouseDownOnPopup.current = false; + return; + } - if ( - (!targetRef.current && isEventTargetOutsideCallout) || - (ev.target !== targetWindowRef.current && - isEventTargetOutsideCallout && - (!targetRef.current || - 'stopPropagation' in targetRef.current || - (target !== targetRef.current && !elementContains(targetRef.current as HTMLElement, target)))) - ) { - onDismiss?.(ev); - } - }; + if ( + (!targetRef.current && isEventTargetOutsideCallout) || + (ev.target !== targetWindowRef.current && + isEventTargetOutsideCallout && + (!targetRef.current || + 'stopPropagation' in targetRef.current || + (target !== targetRef.current && !elementContains(targetRef.current as HTMLElement, target)))) + ) { + onDismiss?.(ev); + } + }; - React.useEffect(() => { // This is added so the callout will dismiss when the window is scrolled // but not when something inside the callout is scrolled. The delay seems // to be required to avoid React firing an async focus event in IE from @@ -381,15 +400,43 @@ function useDismissHandlers( }; } }, 0); + // TODO: check with Michael + // missing deps on onDismiss, positions, preventDismissOnLostFocus, preventDismissOnResize, preventDismissOnScroll + // (and immutable values async, hostElement, targetRef, targetWindowRef + // eslint-disable-next-line react-hooks/exhaustive-deps }, [hidden]); - return [mouseDownOnPopup, mouseUpOnPopup] as const; + return mouseDownHandlers; } export const CalloutContentBase = React.memo( React.forwardRef((propsWithoutDefaults: ICalloutProps, forwardedRef: React.Ref) => { const props = getPropsWithDefaults(DEFAULT_PROPS, propsWithoutDefaults); + const { + styles, + style, + ariaLabel, + ariaDescribedBy, + ariaLabelledBy, + className, + isBeakVisible, + children, + beakWidth, + calloutWidth, + calloutMaxWidth, + finalHeight, + hideOverflow = !!finalHeight, + backgroundColor, + calloutMaxHeight, + onScroll, + // eslint-disable-next-line deprecation/deprecation + shouldRestoreFocus = true, + target, + hidden, + onLayerMounted, + } = props; + const hostElement = React.useRef(null); const calloutElement = React.useRef(null); const rootRef = useMergedRefs(hostElement, forwardedRef); @@ -410,35 +457,16 @@ export const CalloutContentBase = React.memo( useAutoFocus(props, positions, calloutElement); React.useEffect(() => { - if (!props.hidden) { - props.onLayerMounted?.(); + if (!hidden) { + onLayerMounted?.(); } - }, [props.hidden]); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run if hidden changes + }, [hidden]); + // If there is no target window then we are likely in server side rendering and we should not render anything. if (!targetWindowRef.current) { return null; } - const { - styles, - style, - ariaLabel, - ariaDescribedBy, - ariaLabelledBy, - className, - isBeakVisible, - children, - beakWidth, - calloutWidth, - calloutMaxWidth, - finalHeight, - hideOverflow = !!finalHeight, - backgroundColor, - calloutMaxHeight, - onScroll, - // eslint-disable-next-line deprecation/deprecation - shouldRestoreFocus = true, - target, - } = props; const getContentMaxHeight: number | undefined = maxHeight ? maxHeight + heightOffset : undefined; const contentMaxHeight: number | undefined = diff --git a/packages/react-next/src/components/Checkbox/useCheckbox.ts b/packages/react-next/src/components/Checkbox/useCheckbox.ts index 08d8b048e93cbe..a681c92a172c5f 100644 --- a/packages/react-next/src/components/Checkbox/useCheckbox.ts +++ b/packages/react-next/src/components/Checkbox/useCheckbox.ts @@ -84,6 +84,7 @@ export const useCheckbox = (props: ICheckboxProps, forwardedRef: React.Ref(); @@ -37,6 +37,8 @@ export interface IEntityRect { height?: number; } +type BeakPosition = Pick; + const DEFAULT_PROPS: Partial = { isCollapsed: true, mouseProximityOffset: 10, @@ -49,43 +51,43 @@ const DEFAULT_PROPS: Partial = { }; function useCollapsedState(props: ICoachmarkProps, entityInnerHostElementRef: React.RefObject) { - /** - * Is the Coachmark currently collapsed into - * a tear drop shape - */ - const [isCollapsed, setIsCollapsed] = React.useState(!!props.isCollapsed); - const async = useAsync(); + const { isCollapsed: propsIsCollapsed, onAnimationOpenStart, onAnimationOpenEnd } = props; + + /** Is the Coachmark currently collapsed into a tear drop shape */ + const [isCollapsed, setIsCollapsed] = React.useState(!!propsIsCollapsed); + const { setTimeout } = useSetTimeout(); // Rather than pushing out logic elsewhere to prevent openCoachmark from being called repeatedly, // we'll track it here and only invoke the logic once. We do this with a ref, rather than just the state, // because the openCoachmark callback can be captured in scope for an effect const hasCoachmarkBeenOpened = React.useRef(!isCollapsed); - const openCoachmark = () => { + const openCoachmark = React.useCallback(() => { if (!hasCoachmarkBeenOpened.current) { setIsCollapsed(false); - props.onAnimationOpenStart?.(); + onAnimationOpenStart?.(); entityInnerHostElementRef.current?.addEventListener?.('transitionend', (): void => { // Need setTimeout to trigger narrator - async.setTimeout(() => { + setTimeout(() => { if (entityInnerHostElementRef.current) { focusFirstChild(entityInnerHostElementRef.current); } }, 1000); - props.onAnimationOpenEnd?.(); + onAnimationOpenEnd?.(); }); hasCoachmarkBeenOpened.current = true; } - }; + }, [entityInnerHostElementRef, onAnimationOpenEnd, onAnimationOpenStart, setTimeout]); React.useEffect(() => { - if (!props.isCollapsed) { + if (!propsIsCollapsed) { openCoachmark(); } - }, [props.isCollapsed]); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run if isCollapsed changes + }, [propsIsCollapsed]); return [isCollapsed, openCoachmark] as const; } @@ -116,36 +118,15 @@ function useBeakPosition( targetAlignment: RectangleEdge | undefined, targetPosition: RectangleEdge | undefined, ) { - const beakDirection = targetPosition === undefined ? RectangleEdge.bottom : getOppositeEdge(targetPosition); + const { theme } = props; - /** - * The left position of the beak - */ - const [beakLeft, setBeakLeft] = React.useState(); + return React.useMemo(() => { + const beakDirection = targetPosition === undefined ? RectangleEdge.bottom : getOppositeEdge(targetPosition); - /** - * The right position of the beak - */ - const [beakTop, setBeakTop] = React.useState(); + const beakPosition: BeakPosition = { direction: beakDirection }; - /** - * The right position of the beak - */ - const [beakRight, setBeakRight] = React.useState(); - - /** - * The bottom position of the beak - */ - const [beakBottom, setBeakBottom] = React.useState(); - - /** - * Transform origin of teaching bubble callout - */ - const [transformOrigin, setTransformOrigin] = React.useState(); - - React.useEffect(() => { - let transformOriginX; - let transformOriginY; + let transformOriginX: string; + let transformOriginY: string; const distanceAdjustment = '3px'; // Adjustment distance for Beak to shift towards Coachmark bubble. @@ -155,25 +136,25 @@ function useBeakPosition( case RectangleEdge.bottom: // If there is no target alignment, then beak is X-axis centered in callout if (!targetAlignment) { - setBeakLeft(`calc(50% - ${BEAK_WIDTH / 2}px)`); + beakPosition.left = `calc(50% - ${BEAK_WIDTH / 2}px)`; transformOriginX = 'center'; } else { // Beak is aligned to the left of target if (targetAlignment === RectangleEdge.left) { - setBeakLeft(`${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`); + beakPosition.left = `${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`; transformOriginX = 'left'; } else { // Beak is aligned to the right of target - setBeakRight(`${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`); + beakPosition.right = `${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`; transformOriginX = 'right'; } } if (beakDirection === RectangleEdge.top) { - setBeakTop(distanceAdjustment); + beakPosition.top = distanceAdjustment; transformOriginY = 'top'; } else { - setBeakBottom(distanceAdjustment); + beakPosition.bottom = distanceAdjustment; transformOriginY = 'bottom'; } break; @@ -182,45 +163,40 @@ function useBeakPosition( case RectangleEdge.right: // If there is no target alignment, then beak is Y-axis centered in callout if (!targetAlignment) { - setBeakTop(`calc(50% - ${BEAK_WIDTH / 2}px)`); + beakPosition.top = `calc(50% - ${BEAK_WIDTH / 2}px)`; transformOriginY = `center`; } else { // Beak is aligned to the top of target if (targetAlignment === RectangleEdge.top) { - setBeakTop(`${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`); + beakPosition.top = `${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`; transformOriginY = `top`; } else { // Beak is aligned to the bottom of target - setBeakBottom(`${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`); + beakPosition.bottom = `${COACHMARK_WIDTH / 2 - BEAK_WIDTH / 2}px`; transformOriginY = `bottom`; } } if (beakDirection === RectangleEdge.left) { - if (getRTL(props.theme)) { - setBeakRight(distanceAdjustment); + if (getRTL(theme)) { + beakPosition.right = distanceAdjustment; } else { - setBeakLeft(distanceAdjustment); + beakPosition.left = distanceAdjustment; } transformOriginX = 'left'; } else { - if (getRTL(props.theme)) { - setBeakLeft(distanceAdjustment); + if (getRTL(theme)) { + beakPosition.left = distanceAdjustment; } else { - setBeakRight(distanceAdjustment); + beakPosition.right = distanceAdjustment; } transformOriginX = 'right'; } break; } - setTransformOrigin(`${transformOriginX} ${transformOriginY}`); - }, [targetAlignment, targetPosition]); - - return [ - { direction: beakDirection, top: beakTop, bottom: beakBottom, left: beakLeft, right: beakRight } as const, - transformOrigin, - ] as const; + return [beakPosition as Readonly, `${transformOriginX} ${transformOriginY}`] as const; + }, [targetAlignment, targetPosition, theme]); } function useListeners( @@ -270,58 +246,50 @@ function useProximityHandlers( translateAnimationContainer: React.RefObject, openCoachmark: () => void, ) { - const async = useAsync(); + const { setTimeout, clearTimeout } = useSetTimeout(); - /** - * The target element the mouse would be in - * proximity to - */ + /** The target element the mouse would be in proximity to */ const targetElementRect = React.useRef(); - const setTargetElementRect = (): void => { - if (translateAnimationContainer?.current) { - targetElementRect.current = translateAnimationContainer.current.getBoundingClientRect(); - } - }; - React.useEffect(() => { + const setTargetElementRect = (): void => { + if (translateAnimationContainer.current) { + targetElementRect.current = translateAnimationContainer.current.getBoundingClientRect(); + } + }; + const events = new EventGroup({}); - // We don't want to the user to immediately trigger the Coachmark when it's opened - async.setTimeout(() => { + // We don't want the user to immediately trigger the Coachmark when it's opened + setTimeout(() => { const { mouseProximityOffset = 0 } = props; - /** - * An array of cached ids returned when setTimeout runs during - * the window resize event trigger. - */ + + /** Cached ids returned when setTimeout runs during the window resize event trigger. */ const timeoutIds: number[] = []; - // Take the initial measure out of the initial render to prevent - // an unnecessary render. - async.setTimeout(() => { + // Take the initial measure out of the initial render to prevent an unnecessary render. + setTimeout(() => { setTargetElementRect(); - // When the window resizes we want to async - // get the bounding client rectangle. - // Every time the event is triggered we want to - // setTimeout and then clear any previous instances - // of setTimeout. + // When the window resizes we want to async get the bounding client rectangle. + // Every time the event is triggered we want to setTimeout and then clear any previous + // instances of setTimeout. events.on(window, 'resize', (): void => { timeoutIds.forEach((value: number): void => { - clearInterval(value); + clearTimeout(value); }); + timeoutIds.splice(0, timeoutIds.length); // clear array timeoutIds.push( - async.setTimeout((): void => { + setTimeout((): void => { setTargetElementRect(); }, 100), ); }); }, 10); - // Every time the document's mouse move is triggered - // we want to check if inside of an element and - // set the state with the result. + // Every time the document's mouse move is triggered, we want to check if inside of an element + // and set the state with the result. events.on(document, 'mousemove', (e: MouseEvent) => { const mouseY = e.clientY; const mouseX = e.clientX; @@ -336,27 +304,27 @@ function useProximityHandlers( }, props.delayBeforeMouseOpen!); return () => events.dispose(); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run on mount }, []); } function useComponentRef(props: ICoachmarkProps) { + const { onDismiss } = props; React.useImperativeHandle( props.componentRef, (ev?: Event | React.MouseEvent | React.KeyboardEvent) => ({ dismiss() { - props.onDismiss?.(ev); + onDismiss?.(ev); }, }), - [props.onDismiss], + [onDismiss], ); } function useAriaAlert({ ariaAlertText }: ICoachmarkProps) { const async = useAsync(); - /** - * ARIA alert text to read aloud with Narrator once the Coachmark is mounted - */ + /** ARIA alert text to read aloud with Narrator once the Coachmark is mounted */ const [alertText, setAlertText] = React.useState(); React.useEffect(() => { @@ -364,13 +332,14 @@ function useAriaAlert({ ariaAlertText }: ICoachmarkProps) { async.requestAnimationFrame(() => { setAlertText(ariaAlertText); }); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run on mount }, []); return alertText; } function useAutoFocus({ preventFocusOnMount }: ICoachmarkProps) { - const async = useAsync(); + const { setTimeout } = useSetTimeout(); /** * The cached HTMLElement reference to the Entity Inner Host @@ -380,22 +349,18 @@ function useAutoFocus({ preventFocusOnMount }: ICoachmarkProps) { React.useEffect(() => { if (!preventFocusOnMount) { - async.setTimeout(() => entityHost.current?.focus(), 1000); + setTimeout(() => entityHost.current?.focus(), 1000); } + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run on mount }, []); return entityHost; } function useEntityHostMeasurements(props: ICoachmarkProps, entityInnerHostElementRef: React.RefObject) { - /** - * Is the teaching bubble currently retreiving the - * original dimensions of the hosted entity. - */ + /** Is the teaching bubble currently retrieving the original dimensions of the hosted entity. */ const [isMeasuring, setIsMeasuring] = React.useState(!!props.isCollapsed); - /** - * Cached width and height of _entityInnerHostElement - */ + /** Cached width and height of _entityInnerHostElement */ const [entityInnerHostRect, setEntityInnerHostRect] = React.useState( props.isCollapsed ? { width: 0, height: 0 } : {}, ); @@ -411,22 +376,28 @@ function useEntityHostMeasurements(props: ICoachmarkProps, entityInnerHostElemen setIsMeasuring(false); } }); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run on mount }, []); return [isMeasuring, entityInnerHostRect] as const; } function useDeprecationWarning(props: ICoachmarkProps) { - React.useEffect(() => { - warnDeprecations(COMPONENT_NAME, props, { - teachingBubbleRef: undefined, - collapsed: 'isCollapsed', - beakWidth: undefined, - beakHeight: undefined, - width: undefined, - height: undefined, + if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional + useWarnings({ + name: COMPONENT_NAME, + props, + deprecations: { + teachingBubbleRef: undefined, + collapsed: 'isCollapsed', + beakWidth: undefined, + beakHeight: undefined, + width: undefined, + height: undefined, + }, }); - }, []); + } } const COMPONENT_NAME = 'CoachmarkBase'; diff --git a/packages/react-next/src/components/Coachmark/PositioningContainer/PositioningContainer.tsx b/packages/react-next/src/components/Coachmark/PositioningContainer/PositioningContainer.tsx index 221b5dac502663..b8a3db89ecc17f 100644 --- a/packages/react-next/src/components/Coachmark/PositioningContainer/PositioningContainer.tsx +++ b/packages/react-next/src/components/Coachmark/PositioningContainer/PositioningContainer.tsx @@ -31,8 +31,7 @@ import { useMergedRefs, useAsync } from '@uifabric/react-hooks'; const OFF_SCREEN_STYLE = { opacity: 0 }; -// In order for some of the max height logic to work -// properly we need to set the border. +// In order for some of the max height logic to work properly we need to set the border. // The value is abitrary. const BORDER_WIDTH = 1; const SLIDE_ANIMATIONS = { @@ -53,15 +52,11 @@ function useTargets({ target }: IPositioningContainerProps, positionedHost: Reac const previousTargetProp = React.useRef(); const targetRef = React.useRef(null); - /** - * Stores an instance of Window, used to check - * for server side rendering and if focus was lost. - */ + /** Stores an instance of Window, used to check for server side rendering and if focus was lost. */ const targetWindowRef = React.useRef(); - // If the target element changed, find the new one. If we are tracking - // target with class name, always find element because we do not know if - // fabric has rendered a new element and disposed the old element. + // If the target element changed, find the new one. If we are tracking target with class name, + // always find element because the element may have been disposed and re-rendered. if (target !== previousTargetProp.current || typeof target === 'string') { const currentElement = positionedHost.current; @@ -96,10 +91,7 @@ function useTargets({ target }: IPositioningContainerProps, positionedHost: Reac } function useCachedBounds(props: IPositioningContainerProps, targetWindowRef: React.RefObject) { - /** - * The bounds used when determing if and where the - * PositioningContainer should be placed. - */ + /** The bounds used when determining if and where the PositioningContainer should be placed. */ const positioningBounds = React.useRef(); const getCachedBounds = (): IRectangle => { @@ -289,6 +281,7 @@ function useAutoDismissEvents( }, 0); return () => events.dispose(); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run on mount }, []); } @@ -304,9 +297,7 @@ export function useHeightOffset( const async = useAsync(); const setHeightOffsetTimer = React.useRef(0); - /** - * Animates the height if finalHeight was given. - */ + /** Animates the height if finalHeight was given. */ const setHeightOffsetEveryFrame = (): void => { if (contentHost && finalHeight) { setHeightOffsetTimer.current = async.requestAnimationFrame(() => { @@ -330,6 +321,7 @@ export function useHeightOffset( } }; + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only re-run if finalHeight changes React.useEffect(setHeightOffsetEveryFrame, [finalHeight]); return heightOffset; @@ -362,6 +354,7 @@ export const PositioningContainer = React.forwardRef( useSetInitialFocus(props, contentHost, positions); useAutoDismissEvents(props, positionedHost, targetWindowRef, targetRef, positions, updateAsyncPosition); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run on initial render React.useEffect(() => props.onLayerMounted?.(), []); // If there is no target window then we are likely in server side rendering and we should not render anything. diff --git a/packages/react-next/src/components/Fabric/Fabric.base.tsx b/packages/react-next/src/components/Fabric/Fabric.base.tsx index 1575b722ab252e..9b1cd9f019b9e6 100644 --- a/packages/react-next/src/components/Fabric/Fabric.base.tsx +++ b/packages/react-next/src/components/Fabric/Fabric.base.tsx @@ -89,7 +89,8 @@ function useApplyThemeToBody( }; } } - }, [bodyThemed]); + // TODO: verify with Michael + }, [bodyThemed, applyThemeToBody, rootElement]); return rootElement; } diff --git a/packages/react-next/src/components/Image/Image.base.tsx b/packages/react-next/src/components/Image/Image.base.tsx index 8ba3ba0c108bc8..84089323f70ff8 100644 --- a/packages/react-next/src/components/Image/Image.base.tsx +++ b/packages/react-next/src/components/Image/Image.base.tsx @@ -20,15 +20,17 @@ function useLoadState( /* onImageLoad */ (ev: React.SyntheticEvent) => void, /* onImageError */ (ev: React.SyntheticEvent) => void, ] { + const { onLoadingStateChange, onLoad, onError, src } = props; + const [loadState, setLoadState] = React.useState(ImageLoadState.notLoaded); React.useLayoutEffect(() => { // If the src property changes, reset the load state - if (loadState !== ImageLoadState.notLoaded) { - setLoadState(ImageLoadState.notLoaded); - } - }, [props.src]); + // (does nothing if the load state is already notLoaded) + setLoadState(ImageLoadState.notLoaded); + }, [src]); + // eslint-disable-next-line react-hooks/exhaustive-deps -- intended to run every render React.useEffect(() => { if (loadState === ImageLoadState.notLoaded) { // testing if naturalWidth and naturalHeight are greater than zero is better than checking @@ -36,8 +38,8 @@ function useLoadState( // for some browsers, SVG images do not have a naturalWidth or naturalHeight, so fall back // to checking .complete for these images. const isLoaded: boolean = imageElement.current - ? (props.src && imageElement.current.naturalWidth > 0 && imageElement.current.naturalHeight > 0) || - (imageElement.current.complete && SVG_REGEX.test(props.src!)) + ? (src && imageElement.current.naturalWidth > 0 && imageElement.current.naturalHeight > 0) || + (imageElement.current.complete && SVG_REGEX.test(src!)) : false; if (isLoaded) { @@ -47,25 +49,26 @@ function useLoadState( }); React.useEffect(() => { - props.onLoadingStateChange?.(loadState); + onLoadingStateChange?.(loadState); + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run when loadState changes }, [loadState]); const onImageLoaded = React.useCallback( (ev: React.SyntheticEvent) => { - props.onLoad?.(ev); - if (props.src) { + onLoad?.(ev); + if (src) { setLoadState(ImageLoadState.loaded); } }, - [props.src, props.onLoad], + [src, onLoad], ); const onImageError = React.useCallback( (ev: React.SyntheticEvent) => { - props.onError?.(ev); + onError?.(ev); setLoadState(ImageLoadState.error); }, - [props.src, props.onError], + [onError], ); return [loadState, onImageLoaded, onImageError] as const; diff --git a/packages/react-next/src/components/Link/useLink.ts b/packages/react-next/src/components/Link/useLink.ts index bb3036bf7c7b37..f35829ecbf9495 100644 --- a/packages/react-next/src/components/Link/useLink.ts +++ b/packages/react-next/src/components/Link/useLink.ts @@ -60,7 +60,7 @@ const useComponentRef = (props: ILinkProps, link: React.RefObject) => { } }, }), - [], + [link], ); }; diff --git a/packages/react-next/src/components/Persona/Persona.base.tsx b/packages/react-next/src/components/Persona/Persona.base.tsx index 26af2cc55a7640..53a8563ca4bace 100644 --- a/packages/react-next/src/components/Persona/Persona.base.tsx +++ b/packages/react-next/src/components/Persona/Persona.base.tsx @@ -28,6 +28,7 @@ const DEFAULT_PROPS = { function useDebugWarnings(props: IPersonaProps) { if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional useWarnings({ name: 'Persona', props, diff --git a/packages/react-next/src/components/Persona/PersonaCoin/PersonaCoin.base.tsx b/packages/react-next/src/components/Persona/PersonaCoin/PersonaCoin.base.tsx index 225aec7dc588e0..1ad4e88651c657 100644 --- a/packages/react-next/src/components/Persona/PersonaCoin/PersonaCoin.base.tsx +++ b/packages/react-next/src/components/Persona/PersonaCoin/PersonaCoin.base.tsx @@ -37,6 +37,7 @@ const DEFAULT_PROPS = { function useDebugWarnings(props: IPersonaCoinProps) { if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional useWarnings({ name: 'PersonaCoin', props, diff --git a/packages/react-next/src/components/Popup/Popup.tsx b/packages/react-next/src/components/Popup/Popup.tsx index 80751ea041bf4e..fdd6b159bb6129 100644 --- a/packages/react-next/src/components/Popup/Popup.tsx +++ b/packages/react-next/src/components/Popup/Popup.tsx @@ -64,12 +64,14 @@ function useRestoreFocus(props: IPopupProps, root: React.RefObject { if (doesElementContainFocus(root.current!)) { containsFocus.current = true; } + // eslint-disable-next-line react-hooks/exhaustive-deps -- should only run on first render }, []); useOnEvent( @@ -96,6 +98,7 @@ function useRestoreFocus(props: IPopupProps, root: React.RefObject void); diff --git a/packages/react-next/src/components/ResizeGroup/ResizeGroup.base.tsx b/packages/react-next/src/components/ResizeGroup/ResizeGroup.base.tsx index 72af5443cff277..7adce0fd15a3bc 100644 --- a/packages/react-next/src/components/ResizeGroup/ResizeGroup.base.tsx +++ b/packages/react-next/src/components/ResizeGroup/ResizeGroup.base.tsx @@ -455,6 +455,7 @@ function useResizingBehavior(props: IResizeGroupProps, rootRef: React.RefObject< function useDebugWarnings(props: IResizeGroupProps) { if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional useWarnings({ name: COMPONENT_NAME, props, diff --git a/packages/react-next/src/components/SearchBox/SearchBox.base.tsx b/packages/react-next/src/components/SearchBox/SearchBox.base.tsx index e34b39766c4175..93c84216ef449f 100644 --- a/packages/react-next/src/components/SearchBox/SearchBox.base.tsx +++ b/packages/react-next/src/components/SearchBox/SearchBox.base.tsx @@ -1,15 +1,17 @@ import * as React from 'react'; import { ISearchBoxProps, ISearchBoxStyleProps, ISearchBoxStyles, ISearchBox } from './SearchBox.types'; import { KeyCodes, classNamesFunction, getNativeProps, inputProperties } from '../../Utilities'; -import { useBoolean, useControllableValue, useId, useMergedRefs, useWarnings } from '@uifabric/react-hooks'; -import { IconButton } from '../../compat/Button'; -import { Icon } from '../../Icon'; +import { useControllableValue, useId, useMergedRefs, useWarnings } from '@uifabric/react-hooks'; +import { IconButton, IButtonProps, IButtonStyles } from '../../compat/Button'; +import { Icon, IIconProps } from '../../Icon'; const COMPONENT_NAME = 'SearchBox'; -const iconButtonStyles = { root: { height: 'auto' }, icon: { fontSize: '12px' } }; -const iconButtonProps = { iconName: 'Clear' }; +const iconButtonStyles: Partial = { root: { height: 'auto' }, icon: { fontSize: '12px' } }; +const iconButtonProps: IIconProps = { iconName: 'Clear' }; +const defaultClearButtonProps: IButtonProps = { ariaLabel: 'Clear text' }; const getClassNames = classNamesFunction(); + const useComponentRef = ( componentRef: React.Ref | undefined, inputElementRef: React.RefObject, @@ -26,27 +28,29 @@ const useComponentRef = ( }; export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwardedRef: React.Ref) => { - const [hasFocus, { setTrue: setHasFocusTrue, setFalse: setHasFocusFalse }] = useBoolean(false); + const [hasFocus, setHasFocus] = React.useState(false); const [value = '', setValue] = useControllableValue(props.value, props.defaultValue, props.onChange); const rootElementRef = React.useRef(null); const inputElementRef = React.useRef(null); const mergedRootRef = useMergedRefs(rootElementRef, forwardedRef); - const fallbackId = useId(COMPONENT_NAME); + const id = useId(COMPONENT_NAME, props.id); const { ariaLabel, - placeholder, className, disabled, underlined, styles, // eslint-disable-next-line deprecation/deprecation labelText, + // eslint-disable-next-line deprecation/deprecation + placeholder = labelText, theme, - clearButtonProps = { ariaLabel: 'Clear text' }, + clearButtonProps = defaultClearButtonProps, disableAnimation = false, + onClear: customOnClear, + onBlur: customOnBlur, iconProps, - id = fallbackId, } = props; const classNames = getClassNames(styles!, { @@ -55,7 +59,7 @@ export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwarded underlined, hasFocus, disabled, - hasInput: value!.length > 0, + hasInput: value.length > 0, disableAnimation, }); @@ -67,47 +71,48 @@ export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwarded 'value', ]); - const placeholderValue = placeholder !== undefined ? placeholder : labelText; - - const onClear = (ev: React.MouseEvent | React.KeyboardEvent) => { - props.onClear && props.onClear(ev); - if (!ev.defaultPrevented) { - setValue(''); - inputElementRef.current?.focus(); - ev.stopPropagation(); - ev.preventDefault(); - } - }; + const onClear = React.useCallback( + (ev: React.MouseEvent | React.KeyboardEvent) => { + customOnClear?.(ev); + if (!ev.defaultPrevented) { + setValue(''); + inputElementRef.current?.focus(); + ev.stopPropagation(); + ev.preventDefault(); + } + }, + [customOnClear, setValue], + ); - const onClearClick = (ev: React.MouseEvent) => { - if (clearButtonProps && clearButtonProps.onClick) { - clearButtonProps.onClick(ev); - } - if (!ev.defaultPrevented) { - onClear(ev); - } - }; + const onClearClick = React.useCallback( + (ev: React.MouseEvent) => { + clearButtonProps?.onClick?.(ev); + if (!ev.defaultPrevented) { + onClear(ev); + } + }, + [clearButtonProps, onClear], + ); const onFocusCapture = (ev: React.FocusEvent) => { - setHasFocusTrue(); - if (props.onFocus) { - props.onFocus(ev as React.FocusEvent); - } + setHasFocus(true); + props.onFocus?.(ev as React.FocusEvent); }; const onClickFocus = () => { if (inputElementRef.current) { - focus(); + inputElementRef.current.focus(); inputElementRef.current.selectionStart = inputElementRef.current.selectionEnd = 0; } }; - const onBlur = (ev: React.FocusEvent): void => { - setHasFocusFalse(); - if (props.onBlur) { - props.onBlur(ev); - } - }; + const onBlur = React.useCallback( + (ev: React.FocusEvent): void => { + setHasFocus(false); + customOnBlur?.(ev); + }, + [customOnBlur], + ); const onInputChange = (ev: React.ChangeEvent) => { setValue(ev.target.value); @@ -116,7 +121,7 @@ export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwarded const onKeyDown = (ev: React.KeyboardEvent) => { switch (ev.which) { case KeyCodes.escape: - props.onEscape && props.onEscape(ev); + props.onEscape?.(ev); if (!ev.defaultPrevented) { onClear(ev); } @@ -129,7 +134,7 @@ export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwarded // if we don't handle the enter press then we shouldn't prevent default return; default: - onKeyDown && onKeyDown(ev); + onKeyDown?.(ev); if (!ev.defaultPrevented) { return; } @@ -141,6 +146,7 @@ export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwarded }; if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional useWarnings({ name: COMPONENT_NAME, props, @@ -159,7 +165,7 @@ export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwarded {...nativeProps} id={id} className={classNames.field} - placeholder={placeholderValue} + placeholder={placeholder} onChange={onInputChange} onInput={onInputChange} onBlur={onBlur} @@ -173,12 +179,10 @@ export const SearchBoxBase = React.forwardRef((props: ISearchBoxProps, forwarded {value!.length > 0 && (
diff --git a/packages/react-next/src/components/Slider/Slider.base.tsx b/packages/react-next/src/components/Slider/Slider.base.tsx index ad38f9bd60b7ab..2ffb2bdd22cd70 100644 --- a/packages/react-next/src/components/Slider/Slider.base.tsx +++ b/packages/react-next/src/components/Slider/Slider.base.tsx @@ -14,6 +14,7 @@ export const SliderBase = React.forwardRef((props: ISliderProps, ref: React.Ref< const slotProps = useSlider(props, ref); if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional useWarnings({ name: COMPONENT_NAME, props, diff --git a/packages/react-next/src/components/Slider/useSlider.ts b/packages/react-next/src/components/Slider/useSlider.ts index 6d9a0e5e229a3e..f4c295149df476 100644 --- a/packages/react-next/src/components/Slider/useSlider.ts +++ b/packages/react-next/src/components/Slider/useSlider.ts @@ -50,7 +50,7 @@ const useComponentRef = (props: ISliderProps, thumb: React.RefObject { ); if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional useWarnings({ name: COMPONENT_NAME, props, diff --git a/packages/react-next/src/components/Toggle/useToggle.ts b/packages/react-next/src/components/Toggle/useToggle.ts index a5977ee439c0bd..904854ace1ba6d 100644 --- a/packages/react-next/src/components/Toggle/useToggle.ts +++ b/packages/react-next/src/components/Toggle/useToggle.ts @@ -20,7 +20,6 @@ export const useToggle = ( className, defaultChecked = false, disabled, - id: toggleId, inlineLabel, label, // eslint-disable-next-line deprecation/deprecation @@ -46,7 +45,7 @@ export const useToggle = ( onOffMissing: !onText && !offText, }); const badAriaLabel = checked ? onAriaLabel : offAriaLabel; - const id = toggleId || useId(); + const id = useId(COMPONENT_NAME, props.id); const labelId = `${id}-label`; const stateTextId = `${id}-stateText`; const stateText = checked ? onText : offText; @@ -74,6 +73,7 @@ export const useToggle = ( useComponentRef(props, checked, toggleButton); if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line react-hooks/rules-of-hooks -- build-time conditional useWarnings({ name: COMPONENT_NAME, props, @@ -166,6 +166,6 @@ const useComponentRef = ( } }, }), - [isChecked], + [isChecked, toggleButtonRef], ); }; diff --git a/packages/test-utilities/src/safeMount.ts b/packages/test-utilities/src/safeMount.ts index d44be0ed076f3d..99c79fb761b0af 100644 --- a/packages/test-utilities/src/safeMount.ts +++ b/packages/test-utilities/src/safeMount.ts @@ -2,9 +2,8 @@ import * as React from 'react'; import { mount, ReactWrapper } from 'enzyme'; /** - * Calls renderer.create from react-test-renderer, then calls the callback, - * then unmounts. This prevents mounted components from sitting around doing - * unfathomable things in the background while other tests start executing. + * Calls `mount` from enzyme, calls the callback, and unmounts. This prevents mounted components + * from sitting around doing unfathomable things in the background while other tests execute. * * @param content - JSX content to test. * @param callback - Function callback which receives the component to use. From 877c06f6142919aea3c2d45be484fd04d3a7b4ac Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Tue, 11 Aug 2020 18:34:28 -0700 Subject: [PATCH 2/6] Change files --- ...lint-plugin-2020-08-11-18-34-28-eslint-hooks-next.json | 8 ++++++++ ...-react-next-2020-08-11-18-34-28-eslint-hooks-next.json | 8 ++++++++ ...t-utilities-2020-08-11-18-34-28-eslint-hooks-next.json | 8 ++++++++ 3 files changed, 24 insertions(+) create mode 100644 change/@fluentui-eslint-plugin-2020-08-11-18-34-28-eslint-hooks-next.json create mode 100644 change/@fluentui-react-next-2020-08-11-18-34-28-eslint-hooks-next.json create mode 100644 change/@uifabric-test-utilities-2020-08-11-18-34-28-eslint-hooks-next.json diff --git a/change/@fluentui-eslint-plugin-2020-08-11-18-34-28-eslint-hooks-next.json b/change/@fluentui-eslint-plugin-2020-08-11-18-34-28-eslint-hooks-next.json new file mode 100644 index 00000000000000..95a73c3cd1bbd8 --- /dev/null +++ b/change/@fluentui-eslint-plugin-2020-08-11-18-34-28-eslint-hooks-next.json @@ -0,0 +1,8 @@ +{ + "type": "patch", + "comment": "Add rule to prevent accidental references to globals", + "packageName": "@fluentui/eslint-plugin", + "email": "elcraig@microsoft.com", + "dependentChangeType": "patch", + "date": "2020-08-12T01:34:12.441Z" +} diff --git a/change/@fluentui-react-next-2020-08-11-18-34-28-eslint-hooks-next.json b/change/@fluentui-react-next-2020-08-11-18-34-28-eslint-hooks-next.json new file mode 100644 index 00000000000000..69515cfb4643ef --- /dev/null +++ b/change/@fluentui-react-next-2020-08-11-18-34-28-eslint-hooks-next.json @@ -0,0 +1,8 @@ +{ + "type": "prerelease", + "comment": "Enable eslint-plugin-react-hooks in react-next and fix a few bugs", + "packageName": "@fluentui/react-next", + "email": "elcraig@microsoft.com", + "dependentChangeType": "patch", + "date": "2020-08-12T01:34:16.311Z" +} diff --git a/change/@uifabric-test-utilities-2020-08-11-18-34-28-eslint-hooks-next.json b/change/@uifabric-test-utilities-2020-08-11-18-34-28-eslint-hooks-next.json new file mode 100644 index 00000000000000..44f951d37d0d3e --- /dev/null +++ b/change/@uifabric-test-utilities-2020-08-11-18-34-28-eslint-hooks-next.json @@ -0,0 +1,8 @@ +{ + "type": "none", + "comment": "Fix misleading docs for safeMount", + "packageName": "@uifabric/test-utilities", + "email": "elcraig@microsoft.com", + "dependentChangeType": "none", + "date": "2020-08-12T01:34:27.992Z" +} From 148abd7707710ebbdcf99cc513f25b201e728620 Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Tue, 11 Aug 2020 19:01:58 -0700 Subject: [PATCH 3/6] lint fix --- packages/monaco-editor/src/configureEnvironment.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/monaco-editor/src/configureEnvironment.ts b/packages/monaco-editor/src/configureEnvironment.ts index 4a541871299569..6f78085f293c4f 100644 --- a/packages/monaco-editor/src/configureEnvironment.ts +++ b/packages/monaco-editor/src/configureEnvironment.ts @@ -10,6 +10,7 @@ export interface IMonacoConfig { crossDomain?: boolean; } +// eslint-disable-next-line no-restricted-globals const globalObj = (typeof self !== 'undefined' ? self : typeof window !== 'undefined' ? window : {}) as Window & { // eslint-disable-next-line @typescript-eslint/no-explicit-any MonacoEnvironment?: any; From c17c7c3e2c825d11fa52cb4b1a9f722493f81fc6 Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Tue, 11 Aug 2020 19:02:22 -0700 Subject: [PATCH 4/6] Change files --- ...naco-editor-2020-08-11-19-02-22-eslint-hooks-next.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 change/@uifabric-monaco-editor-2020-08-11-19-02-22-eslint-hooks-next.json diff --git a/change/@uifabric-monaco-editor-2020-08-11-19-02-22-eslint-hooks-next.json b/change/@uifabric-monaco-editor-2020-08-11-19-02-22-eslint-hooks-next.json new file mode 100644 index 00000000000000..448386d161d439 --- /dev/null +++ b/change/@uifabric-monaco-editor-2020-08-11-19-02-22-eslint-hooks-next.json @@ -0,0 +1,8 @@ +{ + "type": "none", + "comment": "Fix violation of newly added lint rule", + "packageName": "@uifabric/monaco-editor", + "email": "elcraig@microsoft.com", + "dependentChangeType": "none", + "date": "2020-08-12T02:02:22.263Z" +} From d16d6baaa5501cc4bce85acf3592a35d6243de30 Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Wed, 12 Aug 2020 13:08:14 -0700 Subject: [PATCH 5/6] PR feedback --- .../src/components/Coachmark/Coachmark.base.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-next/src/components/Coachmark/Coachmark.base.tsx b/packages/react-next/src/components/Coachmark/Coachmark.base.tsx index a958c7d64d9611..4bdaff030305bf 100644 --- a/packages/react-next/src/components/Coachmark/Coachmark.base.tsx +++ b/packages/react-next/src/components/Coachmark/Coachmark.base.tsx @@ -118,7 +118,7 @@ function useBeakPosition( targetAlignment: RectangleEdge | undefined, targetPosition: RectangleEdge | undefined, ) { - const { theme } = props; + const isRTL = getRTL(props.theme); return React.useMemo(() => { const beakDirection = targetPosition === undefined ? RectangleEdge.bottom : getOppositeEdge(targetPosition); @@ -178,14 +178,14 @@ function useBeakPosition( } if (beakDirection === RectangleEdge.left) { - if (getRTL(theme)) { + if (isRTL) { beakPosition.right = distanceAdjustment; } else { beakPosition.left = distanceAdjustment; } transformOriginX = 'left'; } else { - if (getRTL(theme)) { + if (isRTL) { beakPosition.left = distanceAdjustment; } else { beakPosition.right = distanceAdjustment; @@ -196,7 +196,7 @@ function useBeakPosition( } return [beakPosition as Readonly, `${transformOriginX} ${transformOriginY}`] as const; - }, [targetAlignment, targetPosition, theme]); + }, [targetAlignment, targetPosition, isRTL]); } function useListeners( From 98de017e6f195c75234ac036e3ee86d66b1bf249 Mon Sep 17 00:00:00 2001 From: Elizabeth Craig Date: Wed, 12 Aug 2020 16:01:20 -0700 Subject: [PATCH 6/6] snapshots --- .../Toggle/__snapshots__/Toggle.test.tsx.snap | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/react-next/src/components/Toggle/__snapshots__/Toggle.test.tsx.snap b/packages/react-next/src/components/Toggle/__snapshots__/Toggle.test.tsx.snap index 8ae4873afe479b..4c451c783199a3 100644 --- a/packages/react-next/src/components/Toggle/__snapshots__/Toggle.test.tsx.snap +++ b/packages/react-next/src/components/Toggle/__snapshots__/Toggle.test.tsx.snap @@ -14,7 +14,7 @@ exports[`Toggle renders hidden toggle correctly 1`] = ` data-is-focusable={true} data-ktp-target={true} hidden={true} - id="id__0" + id="Toggle0" onClick={[Function]} role="switch" type="button" @@ -56,8 +56,8 @@ exports[`Toggle renders toggle correctly 1`] = ` padding-top: 5px; word-wrap: break-word; } - htmlFor="id__0" - id="id__0-label" + htmlFor="Toggle0" + id="Toggle0-label" > Label @@ -66,11 +66,11 @@ exports[`Toggle renders toggle correctly 1`] = ` >