From 25315dd52b58483979be4b1dae444a30d0d45f41 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 10:25:39 +0100 Subject: [PATCH 1/8] Split scrolling into separate function component --- .../next/client/components/layout-router.tsx | 103 +++++++++++++----- 1 file changed, 74 insertions(+), 29 deletions(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 35898fd9dfda6de..3a285c2199b1749 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -1,10 +1,4 @@ 'use client' - -import React, { useContext, useEffect, useRef, use } from 'react' -import type { - ChildProp, - //Segment -} from '../../server/app-render' import type { AppRouterInstance, ChildSegmentMap, @@ -15,6 +9,14 @@ import type { // FlightDataPath, } from '../../server/app-render' import type { ErrorComponent } from './error-boundary' +import type { FocusAndScrollRef } from './reducer' + +import React, { useContext, useEffect, useRef, use } from 'react' +import { findDOMNode } from 'react-dom' +import type { + ChildProp, + //Segment +} from '../../server/app-render' import { CacheStates, LayoutRouterContext, @@ -85,6 +87,71 @@ function topOfElementInViewport(element: HTMLElement) { return rect.top >= 0 } +function Scroller({ + focusAndScrollRef, + children, +}: { + focusAndScrollRef: FocusAndScrollRef + children: React.ReactNode +}) { + const focusAndScrollElementRef = useRef(null) + + useEffect(() => { + // Handle scroll and focus, it's only applied once in the first useEffect that triggers that changed. + if (focusAndScrollRef.apply && focusAndScrollElementRef.current) { + // State is mutated to ensure that the focus and scroll is applied only once. + focusAndScrollRef.apply = false + // Set focus on the element + focusAndScrollElementRef.current.focus() + // Only scroll into viewport when the layout is not visible currently. + if (!topOfElementInViewport(focusAndScrollElementRef.current)) { + const htmlElement = document.documentElement + const existing = htmlElement.style.scrollBehavior + htmlElement.style.scrollBehavior = 'auto' + focusAndScrollElementRef.current.scrollIntoView() + htmlElement.style.scrollBehavior = existing + } + } + }, [focusAndScrollRef]) + + return ( +
+ {children} +
+ ) +} + +class Scroller2 extends React.Component<{ + focusAndScrollRef: FocusAndScrollRef + children: React.ReactNode +}> { + componentDidMount() { + // Handle scroll and focus, it's only applied once in the first useEffect that triggers that changed. + const { focusAndScrollRef } = this.props + const domNode = findDOMNode(this) + console.log({ domNode }) + + if (focusAndScrollRef.apply && domNode instanceof HTMLElement) { + // State is mutated to ensure that the focus and scroll is applied only once. + focusAndScrollRef.apply = false + // Set focus on the element + domNode.focus() + // Only scroll into viewport when the layout is not visible currently. + if (!topOfElementInViewport(domNode)) { + const htmlElement = document.documentElement + const existing = htmlElement.style.scrollBehavior + htmlElement.style.scrollBehavior = 'auto' + domNode.scrollIntoView() + htmlElement.style.scrollBehavior = existing + } + } + } + + render() { + return this.props.children + } +} + /** * InnerLayoutRouter handles rendering the provided segment based on the cache. */ @@ -117,26 +184,6 @@ export function InnerLayoutRouter({ const { changeByServerResponse, tree: fullTree, focusAndScrollRef } = context - const focusAndScrollElementRef = useRef(null) - - useEffect(() => { - // Handle scroll and focus, it's only applied once in the first useEffect that triggers that changed. - if (focusAndScrollRef.apply && focusAndScrollElementRef.current) { - // State is mutated to ensure that the focus and scroll is applied only once. - focusAndScrollRef.apply = false - // Set focus on the element - focusAndScrollElementRef.current.focus() - // Only scroll into viewport when the layout is not visible currently. - if (!topOfElementInViewport(focusAndScrollElementRef.current)) { - const htmlElement = document.documentElement - const existing = htmlElement.style.scrollBehavior - htmlElement.style.scrollBehavior = 'auto' - focusAndScrollElementRef.current.scrollIntoView() - htmlElement.style.scrollBehavior = existing - } - } - }, [focusAndScrollRef]) - // Read segment path from the parallel router cache node. let childNode = childNodes.get(path) @@ -257,9 +304,7 @@ export function InnerLayoutRouter({ // Ensure root layout is not wrapped in a div as the root layout renders `` return rootLayoutIncluded ? ( -
- {subtree} -
+ {subtree} ) : ( subtree ) From 11a6df237d667814038d4d95a9990d7a5c872f73 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 10:26:17 +0100 Subject: [PATCH 2/8] Use class component scroller --- .../next/client/components/layout-router.tsx | 38 +------------------ 1 file changed, 2 insertions(+), 36 deletions(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 3a285c2199b1749..86e29348145a658 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -87,41 +87,7 @@ function topOfElementInViewport(element: HTMLElement) { return rect.top >= 0 } -function Scroller({ - focusAndScrollRef, - children, -}: { - focusAndScrollRef: FocusAndScrollRef - children: React.ReactNode -}) { - const focusAndScrollElementRef = useRef(null) - - useEffect(() => { - // Handle scroll and focus, it's only applied once in the first useEffect that triggers that changed. - if (focusAndScrollRef.apply && focusAndScrollElementRef.current) { - // State is mutated to ensure that the focus and scroll is applied only once. - focusAndScrollRef.apply = false - // Set focus on the element - focusAndScrollElementRef.current.focus() - // Only scroll into viewport when the layout is not visible currently. - if (!topOfElementInViewport(focusAndScrollElementRef.current)) { - const htmlElement = document.documentElement - const existing = htmlElement.style.scrollBehavior - htmlElement.style.scrollBehavior = 'auto' - focusAndScrollElementRef.current.scrollIntoView() - htmlElement.style.scrollBehavior = existing - } - } - }, [focusAndScrollRef]) - - return ( -
- {children} -
- ) -} - -class Scroller2 extends React.Component<{ +class Scroller extends React.Component<{ focusAndScrollRef: FocusAndScrollRef children: React.ReactNode }> { @@ -304,7 +270,7 @@ export function InnerLayoutRouter({ // Ensure root layout is not wrapped in a div as the root layout renders `` return rootLayoutIncluded ? ( - {subtree} + {subtree} ) : ( subtree ) From 4ba096e563bb73ccd9e48b1255f48fe76f23b9d8 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 10:26:50 +0100 Subject: [PATCH 3/8] Remove debug log --- packages/next/client/components/layout-router.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 86e29348145a658..8d39b4375974581 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -95,7 +95,6 @@ class Scroller extends React.Component<{ // Handle scroll and focus, it's only applied once in the first useEffect that triggers that changed. const { focusAndScrollRef } = this.props const domNode = findDOMNode(this) - console.log({ domNode }) if (focusAndScrollRef.apply && domNode instanceof HTMLElement) { // State is mutated to ensure that the focus and scroll is applied only once. From 69bbc31c8530d1d497c760ead1f7a15154e5ddcd Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 10:44:11 +0100 Subject: [PATCH 4/8] Ensure findDOMNode hides strict mode warning --- .../next/client/components/layout-router.tsx | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 8d39b4375974581..910069241a73791 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -12,7 +12,7 @@ import type { ErrorComponent } from './error-boundary' import type { FocusAndScrollRef } from './reducer' import React, { useContext, useEffect, useRef, use } from 'react' -import { findDOMNode } from 'react-dom' +import { findDOMNode as ReactDOMfindDOMNode } from 'react-dom' import type { ChildProp, //Segment @@ -79,6 +79,31 @@ function walkAddRefetch( return treeToRecreate } +// TODO-APP: Replace with new React API for finding dom nodes without a `ref` when available +/** + * Wraps ReactDOM.findDOMNode with additional logic to hide React Strict Mode warning + */ +function findDOMNode( + instance: Parameters[0] +): ReturnType { + // Only apply strict mode warning when not in production + if (process.env.NODE_ENV !== 'production') { + const originalConsoleError = console.error + try { + console.error = (...messages) => { + // Ignore strict mode warning for the findDomNode call below + if (!messages[0].includes('Warning: %s is deprecated in StrictMode.')) { + originalConsoleError(...messages) + } + } + return ReactDOMfindDOMNode(instance) + } finally { + console.error = originalConsoleError! + } + } + return ReactDOMfindDOMNode(instance) +} + /** * Check if the top of the HTMLElement is in the viewport. */ From 07cf385a9c988b3549d2e9af0295f41050375eab Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 10:44:28 +0100 Subject: [PATCH 5/8] Remove unused useEffect --- packages/next/client/components/layout-router.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 910069241a73791..7c928c1a5198609 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -11,7 +11,7 @@ import type { import type { ErrorComponent } from './error-boundary' import type { FocusAndScrollRef } from './reducer' -import React, { useContext, useEffect, useRef, use } from 'react' +import React, { useContext, useEffect, use } from 'react' import { findDOMNode as ReactDOMfindDOMNode } from 'react-dom' import type { ChildProp, From fa32bf2846417fba650306a8c9dd650264d6af30 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 10:45:04 +0100 Subject: [PATCH 6/8] Rename component as it handles focus too --- packages/next/client/components/layout-router.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 7c928c1a5198609..ede5b7b2784d0de 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -112,7 +112,7 @@ function topOfElementInViewport(element: HTMLElement) { return rect.top >= 0 } -class Scroller extends React.Component<{ +class ScrollAndFocusHandler extends React.Component<{ focusAndScrollRef: FocusAndScrollRef children: React.ReactNode }> { @@ -294,7 +294,9 @@ export function InnerLayoutRouter({ // Ensure root layout is not wrapped in a div as the root layout renders `` return rootLayoutIncluded ? ( - {subtree} + + {subtree} + ) : ( subtree ) From 662225cd9819eddf0e64950550a1fff78970e13c Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 13:56:53 +0100 Subject: [PATCH 7/8] Remove comment --- packages/next/client/components/layout-router.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index ede5b7b2784d0de..47e3450821aa85b 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -13,10 +13,7 @@ import type { FocusAndScrollRef } from './reducer' import React, { useContext, useEffect, use } from 'react' import { findDOMNode as ReactDOMfindDOMNode } from 'react-dom' -import type { - ChildProp, - //Segment -} from '../../server/app-render' +import type { ChildProp } from '../../server/app-render' import { CacheStates, LayoutRouterContext, From 19589149bd79d3897b4b57d79b9e36ab9178f3d9 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Mon, 5 Dec 2022 13:57:32 +0100 Subject: [PATCH 8/8] Remove comment --- packages/next/client/components/layout-router.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index 47e3450821aa85b..050ed4a0271af80 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -6,7 +6,6 @@ import type { import type { FlightRouterState, FlightSegmentPath, - // FlightDataPath, } from '../../server/app-render' import type { ErrorComponent } from './error-boundary' import type { FocusAndScrollRef } from './reducer'