Fix menu flicker (#2658)

* Persist header and footer across ImportedPage content changes * Check for DefaultLayout before applying wrapper * Pull Layout to top level * Extract setLayoutParameters Set layout for general pages Avoid re-setting data to equal value * Optimize data reset * Use a layout context Further enapsulate LayoutContext; test coverage Ensure that normal pages get default layout Move layout up to the Routes level * Code coverage for HeroBlock * Code review * Use reducer for layoutParameters; derive layoutData * More code review items * CR again * Load null until layout name is right,.
openstax · Sep 6, 2024 · 8be052d · 8be052d
1 parent e2ff9f8
commit 8be052d
Show file tree

Hide file tree

Showing 7 changed files with 268 additions and 80 deletions.
diff --git a/package.json b/package.json
@@ -33,6 +33,7 @@
         "boxible": "^1.7.1",
         "classnames": "^2.3.2",
         "color": "^4.2.3",
+        "deep-equal": "^2.2.3",
         "esbuild-loader": "^3.0.1",
         "js-cookie": "^3.0.5",
         "lodash": "^4.17.21",
@@ -59,6 +60,7 @@
         "@testing-library/preact": "^3.2.3",
         "@testing-library/user-event": "^14.4.3",
         "@types/color": "^3.0.6",
+        "@types/deep-equal": "^1.0.4",
         "@types/js-cookie": "^3.0.6",
         "@types/lodash": "^4.17.1",
         "@types/react": "^18.3.3",

diff --git a/src/app/components/shell/router-helpers/fallback-to.js b/src/app/components/shell/router-helpers/fallback-to.js
@@ -4,19 +4,12 @@ import FlexPage from '~/pages/flex-page/flex-page';
 import usePageData from '~/helpers/use-page-data';
 import loadable from 'react-loadable';
 import LoadingPlaceholder from '~/components/loading-placeholder/loading-placeholder';
+import useLayoutContext from '~/contexts/layout';
 
 const FallbackToGeneralPage = loadable({
     loader: () => import('./fallback-to-general.js'),
     loading: () => <h1>...General</h1>
 });
-const Layout = ({name, children, data}) => {
-    const LoadableLayout = loadable({
-        loader: () => import(`~/layouts/${name}/${name}`),
-        loading: LoadingPlaceholder
-    });
-
-    return <LoadableLayout data={data}>{children}</LoadableLayout>;
-};
 
 export default function FallbackTo({name}) {
     const data = usePageData(`pages/${name}`, true);
@@ -25,20 +18,40 @@ export default function FallbackTo({name}) {
         return <LoadingPlaceholder />;
     }
 
+    return <LoadedPage name={name} data={data} />;
+}
+
+// eslint-disable-next-line complexity
+function LoadedPage({data, name}) {
+    const {setLayoutParameters, layoutParameters} = useLayoutContext();
+    const hasError = 'error' in data;
+    const isFlex =
+        !hasError &&
+        ['pages.FlexPage', 'pages.RootPage'].includes(data.meta.type);
+
+    React.useEffect(() => {
+        if (isFlex) {
+            setLayoutParameters({
+                data,
+                name: data.layout[0]?.type || 'default'
+            });
+            return;
+        }
+        setLayoutParameters();
+    }, [data, isFlex, setLayoutParameters]);
+
     // can we correctly identify missing pages here?
     // i think page-data-utils:fetchFromCMS would have to be updated
     // to do something special on a 404 status
-    if ('error' in data) {
-        return <Layout name="default"><Error404 /></Layout>;
+    if (hasError) {
+        return layoutParameters.name === 'default' ? <Error404 /> : null;
     }
 
-    if (['pages.FlexPage', 'pages.RootPage'].includes(data.meta.type)) {
-        return <Layout data={data} name={data.layout[0]?.type || 'default'}>
-            <FlexPage data={data} />
-        </Layout>;
+    if (isFlex) {
+        const expectName = data.layout[0]?.type || 'default';
+
+        return layoutParameters.name === expectName ? <FlexPage data={data} /> : null;
     }
 
-    return <Layout name="default">
-        <FallbackToGeneralPage name={name} />
-    </Layout>;
+    return layoutParameters.name === 'default' ? <FallbackToGeneralPage name={name} /> : null;
 }
diff --git a/src/app/components/shell/router.js b/src/app/components/shell/router.js
@@ -12,6 +12,7 @@ import useLinkHandler from './router-helpers/use-link-handler';
 import useRouterContext, {RouterContextProvider} from './router-context';
 import loadable from 'react-loadable';
 import LoadingPlaceholder from '~/components/loading-placeholder/loading-placeholder';
+import useLayoutContext, { LayoutContextProvider } from '~/contexts/layout';
 import './skip-to-content.scss';
 
 function useAnalyticsPageView() {
@@ -31,41 +32,62 @@ const Error404 = loadable({
     loader: () => import('~/pages/404/404'),
     loading: () => <h1>404</h1>
 });
-const DefaultLayout = loadable({
-    loader: () => import('~/layouts/default/default'),
-    loading: LoadingPlaceholder
-});
 
-function ImportedPage({name}) {
+function useLoading(name) {
     const {pathname} = useLocation();
-    const Page = React.useMemo(
-        () => {
-            function Loading({error, pastDelay, retry}) {
-                if (error) {
-                    if (error.code === 'MODULE_NOT_FOUND') {
-                        return pathname.endsWith('/') ? <Fallback name={name} />
-                        : <Navigate to={`${pathname}/`} replace />;
-                    }
-                    return <div>Error! <button onClick={ retry }>Retry</button></div>;
-                }
-                if (pastDelay) {
-                    return <LoadingPlaceholder />;
-                }
-                return null;
-            }
-
-            return loadable({
-                loader: () => import(`~/pages/${name}/${name}`),
-                loading: Loading,
-                render(loaded, props) {
-                    const Component = loaded.default;
 
-                    return <DefaultLayout><Component {...props} /></DefaultLayout>;
+    return React.useCallback(
+        ({error, pastDelay, retry}) => {
+            if (error) {
+                if (error.code === 'MODULE_NOT_FOUND') {
+                    return pathname.endsWith('/')
+                    ? <Fallback name={name} />
+                    : <Navigate to={`${pathname}/`} replace />;
                 }
-            });
+                return <div>Error! <button onClick={ retry }>Retry</button></div>;
+            }
+            if (pastDelay) {
+                return <LoadingPlaceholder />;
+            }
+            return null;
         },
         [name, pathname]
     );
+}
+
+function DefaultLayout({children}) {
+    const {setLayoutParameters, layoutParameters} = useLayoutContext();
+
+    React.useEffect(
+        () => setLayoutParameters(),
+        [setLayoutParameters]
+    );
+
+    return layoutParameters.name === 'default' ? children : null;
+}
+
+function usePage(name) {
+    const loading = useLoading(name);
+
+    return React.useMemo(
+        () => loadable({
+            loader: () => import(`~/pages/${name}/${name}`),
+            loading,
+            render(loaded, props) {
+                const Component = loaded.default;
+
+                return <DefaultLayout>
+                    <Component {...props} />
+                </DefaultLayout>;
+            }
+        }),
+        [name, loading]
+    );
+}
+
+function ImportedPage({name}) {
+    const {pathname} = useLocation();
+    const Page = usePage(name);
 
     useAnalyticsPageView();
 
@@ -76,8 +98,7 @@ function ImportedPage({name}) {
         [name, pathname]
     );
 
-
-    return (<Page />);
+    return <Page />;
 }
 
 const FOOTER_PAGES = [
@@ -106,37 +127,41 @@ function RedirectToCanonicalDetailsPage() {
 }
 
 function MainRoutes() {
+    const {Layout} = useLayoutContext();
+
     return (
-        <Routes>
-            <Route path="/" element={<ImportedPage name="home" />} />
-            {
-                FOOTER_PAGES.map(
-                    (path) => <Route path={path} key={path} element={<ImportedPage name="footer-page" />} />
-                )
-            }
-            <Route path="/errata/" element={<ImportedPage name="errata-summary" />} />
-            <Route path="/errata/form/" element={<ImportedPage name="errata-form" />} />
-            <Route path="/errata/*" element={<ImportedPage name="errata-detail" />} />
-            <Route path="/details/books/:title" element={<ImportedPage name="details" />} />
-            <Route path="/details/:title" element={<RedirectToCanonicalDetailsPage />} />
-            <Route path="/details/" element={<Navigate to="/subjects" replace />} />
-            <Route path="/books/:title" element={<RedirectToCanonicalDetailsPage />} />
-            <Route path="/textbooks/:title" element={<RedirectToCanonicalDetailsPage />} />
-            <Route path="/subjects-preview/*" element={<ImportedPage name="subjects" />} />
-            <Route path="/k12/*" element={<ImportedPage name="k12" />} />
-            <Route path="/blog/*" element={<ImportedPage name="blog" />} />
-            <Route path="/webinars/*" element={<ImportedPage name="webinars" />} />
-            <Route path="/general/*" element={<ImportedPage name="general" />} />
-            <Route path="/confirmation/*" element={<ImportedPage name="confirmation" />} />
-            <Route path="/campaign/*" element={<ImportedPage name="campaign" />} />
-            <Route path="/press/*" element={<ImportedPage name="press" />} />
-            <Route
-                path="/edtech-partner-program"
-                element={<ImportedPage name="/openstax-ally-technology-partner-program" />}
-            />
-            <Route path="/:name/*" element={<TopLevelPage />} />
-            <Route element={<h1>Fell through</h1>} />
-        </Routes>
+        <Layout>
+            <Routes>
+                <Route path="/" element={<ImportedPage name="home" />} />
+                {
+                    FOOTER_PAGES.map(
+                        (path) => <Route path={path} key={path} element={<ImportedPage name="footer-page" />} />
+                    )
+                }
+                <Route path="/errata/" element={<ImportedPage name="errata-summary" />} />
+                <Route path="/errata/form/" element={<ImportedPage name="errata-form" />} />
+                <Route path="/errata/*" element={<ImportedPage name="errata-detail" />} />
+                <Route path="/details/books/:title" element={<ImportedPage name="details" />} />
+                <Route path="/details/:title" element={<RedirectToCanonicalDetailsPage />} />
+                <Route path="/details/" element={<Navigate to="/subjects" replace />} />
+                <Route path="/books/:title" element={<RedirectToCanonicalDetailsPage />} />
+                <Route path="/textbooks/:title" element={<RedirectToCanonicalDetailsPage />} />
+                <Route path="/subjects-preview/*" element={<ImportedPage name="subjects" />} />
+                <Route path="/k12/*" element={<ImportedPage name="k12" />} />
+                <Route path="/blog/*" element={<ImportedPage name="blog" />} />
+                <Route path="/webinars/*" element={<ImportedPage name="webinars" />} />
+                <Route path="/general/*" element={<ImportedPage name="general" />} />
+                <Route path="/confirmation/*" element={<ImportedPage name="confirmation" />} />
+                <Route path="/campaign/*" element={<ImportedPage name="campaign" />} />
+                <Route path="/press/*" element={<ImportedPage name="press" />} />
+                <Route
+                    path="/edtech-partner-program"
+                    element={<ImportedPage name="/openstax-ally-technology-partner-program" />}
+                />
+                <Route path="/:name/*" element={<TopLevelPage />} />
+                <Route element={<h1>Fell through</h1>} />
+            </Routes>
+        </Layout>
     );
 }
 
@@ -180,7 +205,9 @@ export default function Router() {
         <RouterContextProvider>
             <PageTitleConfirmation />
             <SkipToContent />
-            <MainRoutes />
+            <LayoutContextProvider>
+                <MainRoutes />
+            </LayoutContextProvider>
         </RouterContextProvider>
     );
 }
diff --git a/src/app/contexts/layout.tsx b/src/app/contexts/layout.tsx
@@ -0,0 +1,53 @@
+import React from 'react';
+import buildContext from '~/components/jsx-helpers/build-context';
+import loadable from 'react-loadable';
+import LoadingPlaceholder from '~/components/loading-placeholder/loading-placeholder';
+import deepEqual from 'deep-equal';
+
+// Webpack wasn't able to make the dynamic strings work in the Context value.
+type LayoutName = 'default' | 'landing';
+const loaders = {
+    default: () => import('~/layouts/default/default'),
+    landing: () => import('~/layouts/landing/landing')
+};
+
+type LayoutParameters = {
+    name: LayoutName;
+    data?: any; // eslint-disable-line @typescript-eslint/no-explicit-any
+}
+const defaultLayoutParameters: LayoutParameters = {name: 'default'};
+
+function useContextValue() {
+    const [layoutParameters, setLayoutParameters] = React.useReducer(
+        (state: LayoutParameters, newState: LayoutParameters) => {
+            if (newState === undefined) {
+                return defaultLayoutParameters;
+            }
+            if (deepEqual(state, newState)) {
+                return state;
+            }
+            return newState;
+        },
+        defaultLayoutParameters
+    );
+    const LoadableLayout = React.useMemo(
+        () =>
+            loadable({
+                loader: () => loaders[layoutParameters.name](),
+                loading: LoadingPlaceholder
+            }),
+        [layoutParameters.name]
+    );
+    const Layout = React.useCallback(
+        ({children}: React.PropsWithChildren<object>) => (
+            <LoadableLayout data={layoutParameters.data}>{children}</LoadableLayout>
+        ),
+        [LoadableLayout, layoutParameters.data]
+    );
+
+    return {Layout, setLayoutParameters, layoutParameters};
+}
+
+const {useContext, ContextProvider} = buildContext({useContextValue});
+
+export {useContext as default, ContextProvider as LayoutContextProvider};
diff --git a/test/src/contexts/layout.test.tsx b/test/src/contexts/layout.test.tsx
@@ -0,0 +1,41 @@
+import React from 'react';
+import {render, screen} from '@testing-library/preact';
+import useLayoutContext, { LayoutContextProvider } from '~/contexts/layout';
+
+function Component() {
+    const {Layout, setLayoutParameters} = useLayoutContext();
+
+    React.useEffect(
+        () => setLayoutParameters({name: 'landing', data: {title: 'title', layout: [
+            {
+                value: {
+                    navLinks: [{
+                        text: 'Landing page link',
+                        target: {
+                            type: 'href',
+                            value: 'whatever'
+                        }
+                    }]
+                }
+            }
+        ]}}),
+        [setLayoutParameters]
+    );
+
+    return (
+        <Layout>
+            <div>No menus</div>
+        </Layout>
+    );
+}
+
+describe('layout-context', () => {
+    it('renders a landing page', async () => {
+        render(
+            <LayoutContextProvider>
+                <Component />
+            </LayoutContextProvider>
+        );
+        await screen.findByText('Landing page link');
+    });
+});