From 8f07ee5ce2ca983807e1273a7f1059e9ddbca760 Mon Sep 17 00:00:00 2001 From: Chance Strickland Date: Mon, 22 Jun 2020 07:39:28 -0700 Subject: [PATCH 1/3] housekeeping --- packages/alert/src/index.tsx | 31 +++++++++++++++---------------- packages/utils/src/index.tsx | 8 ++++---- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/packages/alert/src/index.tsx b/packages/alert/src/index.tsx index cdc8079ee..4faf73031 100644 --- a/packages/alert/src/index.tsx +++ b/packages/alert/src/index.tsx @@ -61,7 +61,7 @@ let renderTimer: number | null; * @see Docs https://reacttraining.com/reach-ui/alert */ export const Alert = forwardRef(function Alert( - { children, type = "polite", ...props }, + { children, type: regionType = "polite", ...props }, forwardedRef ) { const ownRef = useRef(null); @@ -75,7 +75,7 @@ export const Alert = forwardRef(function Alert( // eslint-disable-next-line react-hooks/exhaustive-deps [children, props] ); - useMirrorEffects(type, child, ownRef); + useMirrorEffects(regionType, child, ownRef); return child; }); @@ -141,8 +141,8 @@ function renderAlerts() { } renderTimer = window.setTimeout(() => { Object.keys(elements).forEach((elementType) => { - let type: RegionTypes = elementType as RegionTypes; - let container = liveRegions[type]!; + let regionType: RegionTypes = elementType as RegionTypes; + let container = liveRegions[regionType]!; if (container) { render( @@ -154,18 +154,18 @@ function renderAlerts() { // will send out an accessible status event to assistive // technology products which can then notify the user about it. // https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_status_role - role={type === "assertive" ? "alert" : "status"} - aria-live={type} + role={regionType === "assertive" ? "alert" : "status"} + aria-live={regionType} > - {Object.keys(elements[type]).map((key) => - React.cloneElement(elements[type][key], { + {Object.keys(elements[regionType]).map((key) => + React.cloneElement(elements[regionType][key], { key, ref: null, }) )} , - liveRegions[type] + liveRegions[regionType] ); } }); @@ -173,28 +173,27 @@ function renderAlerts() { } function useMirrorEffects( - type: RegionTypes, + regionType: RegionTypes, element: JSX.Element, ref: React.RefObject ) { - const prevType = usePrevious(type); + const prevType = usePrevious(regionType); const mirror = useRef(null); const mounted = useRef(false); useEffect(() => { const ownerDocument = getOwnerDocument(ref.current) || document; if (!mounted.current) { mounted.current = true; - mirror.current = createMirror(type, ownerDocument); + mirror.current = createMirror(regionType, ownerDocument); mirror.current.mount(element); - } else if (prevType !== type) { + } else if (prevType !== regionType) { mirror.current && mirror.current.unmount(); - mirror.current = createMirror(type, ownerDocument); + mirror.current = createMirror(regionType, ownerDocument); mirror.current.mount(element); } else { mirror.current && mirror.current.update(element); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [element, type, prevType]); + }, [element, regionType, prevType, ref]); useEffect(() => { return () => { diff --git a/packages/utils/src/index.tsx b/packages/utils/src/index.tsx index 74f501eb3..0ca460f1a 100644 --- a/packages/utils/src/index.tsx +++ b/packages/utils/src/index.tsx @@ -157,10 +157,10 @@ export function boolOrBoolString(value: any): value is "true" | true { } export function canUseDOM() { - return ( + return !!( typeof window !== "undefined" && - typeof window.document !== "undefined" && - typeof window.document.createElement !== "undefined" + window.document && + window.document.createElement ); } @@ -615,7 +615,7 @@ export function useForkedRef( }); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, refs); + }, [...refs]); } /** From 0df9a02415c8ecf04a3fcac09ff4db6f30143baf Mon Sep 17 00:00:00 2001 From: Chance Strickland Date: Mon, 22 Jun 2020 07:40:47 -0700 Subject: [PATCH 2/3] rect: fix over-rendering and needless attachment of listeners --- packages/rect/src/index.tsx | 49 ++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/packages/rect/src/index.tsx b/packages/rect/src/index.tsx index 8464cd676..5be0e407c 100644 --- a/packages/rect/src/index.tsx +++ b/packages/rect/src/index.tsx @@ -91,43 +91,52 @@ export function useRect( observe: boolean = true, onChange?: (rect: DOMRect) => void ): null | DOMRect { + let [element, setElement] = useState(nodeRef.current); let initialRectSet = useRef(false); let [rect, setRect] = useState(null); - let observerRef = useRef(null); + let onChangeRef = useRef(); + useIsomorphicLayoutEffect(() => { - const cleanup = () => { - observerRef.current && observerRef.current.unobserve(); - }; + onChangeRef.current = onChange; + }); - if (!nodeRef.current) { - console.warn("You need to place the ref"); - return cleanup; + useIsomorphicLayoutEffect(() => { + if (nodeRef.current !== element) { + setElement(nodeRef.current); } + }); - if (!observerRef.current) { - observerRef.current = observeRect(nodeRef.current, (rect: DOMRect) => { - onChange && onChange(rect); - setRect(rect); - }); + useIsomorphicLayoutEffect(() => { + if (element && !initialRectSet.current) { + initialRectSet.current = true; + setRect(element.getBoundingClientRect()); } + }, [element]); - if (!initialRectSet.current) { - initialRectSet.current = true; - setRect(nodeRef.current.getBoundingClientRect()); + useIsomorphicLayoutEffect(() => { + if (!element) { + console.warn("You need to place the ref"); + return cleanup; } - observe && observerRef.current.observe(); + let observer = observeRect(element, (rect) => { + onChangeRef.current && onChangeRef.current(rect); + setRect(rect); + }); + + observe && observer.observe(); return cleanup; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [observe, onChange]); + + function cleanup() { + observer.unobserve(); + } + }, [observe, element]); return rect; } export default Rect; -export type PartialRect = Partial; - export type PRect = Partial & { readonly bottom: number; readonly height: number; From a3ca034b3539f3739566cb5753c37431c382dfbf Mon Sep 17 00:00:00 2001 From: Chance Strickland Date: Mon, 22 Jun 2020 14:23:36 -0700 Subject: [PATCH 3/3] rect: update deps + broaden types --- .../examples/change-observed-ref.example.tsx | 29 +++++++++++++++++++ packages/rect/package.json | 2 +- packages/rect/src/index.tsx | 14 +++++---- yarn.lock | 8 ++--- 4 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 packages/rect/examples/change-observed-ref.example.tsx diff --git a/packages/rect/examples/change-observed-ref.example.tsx b/packages/rect/examples/change-observed-ref.example.tsx new file mode 100644 index 000000000..ba50d93d0 --- /dev/null +++ b/packages/rect/examples/change-observed-ref.example.tsx @@ -0,0 +1,29 @@ +import React from "react"; +import { useRect } from "@reach/rect"; + +let name = "Change the observed ref"; + +function Example() { + const refLeft = React.useRef(null); + const refRight = React.useRef(null); + const [whichRect, setWhichRect] = React.useState(true); + const rect = useRect(whichRect ? refLeft : refRight); + return ( +
+ 
+        {whichRect ? "left" : "right"}: {JSON.stringify(rect, null, 2)}
+
+ +
+