Improved AdminX scrolling behaviour (#18640)

refs https://github.com/TryGhost/Product/issues/3832

Hopefully the scrolling finally works consistently

- Fixed a bug where clicking the navigated section wouldn't scroll to it
- Fixed a bug where the first click after opening settings wouldn't
animate the scroll
- Fixed a bug where the sidebar would always animate scroll even on the
initial page load

---

<!-- Leave the line below if you'd like GitHub Copilot to generate a
summary from your commit -->
<!--
copilot:summary
-->
### <samp>🤖 Generated by Copilot at 0996b8b</samp>

This pull request improves the scrolling and navigation functionality of
the settings page by using a custom hook and a context provider. It
refactors the `RoutingProvider` and the `useScrollSection` hook to
handle the route and sidebar changes more efficiently, and simplifies
the code by removing unnecessary components and state. It also adds new
functions to the scroll section context data to update and scroll to the
desired section.

apps/admin-x-settings/src/App.tsx

@@ -8,6 +8,7 @@ import {DefaultHeaderTypes} from './unsplash/UnsplashTypes';
 import {FetchKoenigLexical, OfficialTheme, ServicesProvider} from './components/providers/ServiceProvider';
 import {GlobalDirtyStateProvider} from './hooks/useGlobalDirtyState';
 import {QueryClient, QueryClientProvider} from '@tanstack/react-query';
+import {ScrollSectionProvider} from './hooks/useScrollSection';
 import {ErrorBoundary as SentryErrorBoundary} from '@sentry/react';
 import {Toaster} from 'react-hot-toast';
 import {UpgradeStatusType} from './utils/globalTypes';
@@ -51,22 +52,24 @@ function App({ghostVersion, officialThemes, zapierTemplates, externalNavigate, d
             <QueryClientProvider client={queryClient}>
                 <ServicesProvider fetchKoenigLexical={fetchKoenigLexical} ghostVersion={ghostVersion} officialThemes={officialThemes} sentryDSN={sentryDSN} unsplashConfig={unsplashConfig} upgradeStatus={upgradeStatus} zapierTemplates={zapierTemplates} onDelete={onDelete} onInvalidate={onInvalidate} onUpdate={onUpdate}>
                     <GlobalDataProvider>
-                        <RoutingProvider externalNavigate={externalNavigate}>
-                            <GlobalDirtyStateProvider>
-                                <DesignSystemProvider>
-                                    <div className={appClassName} id="admin-x-root" style={{
-                                        height: '100vh',
-                                        width: '100%'
-                                    }}
-                                    >
-                                        <Toaster />
-                                        <NiceModal.Provider>
-                                            <MainContent />
-                                        </NiceModal.Provider>
-                                    </div>
-                                </DesignSystemProvider>
-                            </GlobalDirtyStateProvider>
-                        </RoutingProvider>
+                        <ScrollSectionProvider>
+                            <RoutingProvider externalNavigate={externalNavigate}>
+                                <GlobalDirtyStateProvider>
+                                    <DesignSystemProvider>
+                                        <div className={appClassName} id="admin-x-root" style={{
+                                            height: '100vh',
+                                            width: '100%'
+                                        }}
+                                        >
+                                            <Toaster />
+                                            <NiceModal.Provider>
+                                                <MainContent />
+                                            </NiceModal.Provider>
+                                        </div>
+                                    </DesignSystemProvider>
+                                </GlobalDirtyStateProvider>
+                            </RoutingProvider>
+                        </ScrollSectionProvider>
                     </GlobalDataProvider>
                 </ServicesProvider>
             </QueryClientProvider>

apps/admin-x-settings/src/components/providers/RoutingProvider.tsx

@@ -1,6 +1,6 @@
 import NiceModal from '@ebay/nice-modal-react';
 import React, {createContext, useCallback, useEffect, useState} from 'react';
-import {ScrollSectionProvider} from '../../hooks/useScrollSection';
+import {useScrollSectionContext} from '../../hooks/useScrollSection';
 import type {ModalComponent, ModalName} from './routing/modals';
 
 export type RouteParams = {[key: string]: string}
@@ -124,6 +124,7 @@ type RouteProviderProps = {
 const RoutingProvider: React.FC<RouteProviderProps> = ({externalNavigate, children}) => {
     const [route, setRoute] = useState<string | undefined>(undefined);
     const [loadingModal, setLoadingModal] = useState(false);
+    const {updateNavigatedSection, scrollToSection} = useScrollSectionContext();
 
     useEffect(() => {
         // Preload all the modals after initial render to avoid a delay when opening them
@@ -142,12 +143,14 @@ const RoutingProvider: React.FC<RouteProviderProps> = ({externalNavigate, childr
 
         const newPath = options.route;
 
-        if (newPath) {
+        if (newPath === route) {
+            scrollToSection(newPath.split('/')[0]);
+        } else if (newPath) {
             window.location.hash = `/settings/${newPath}`;
         } else {
             window.location.hash = `/settings`;
         }
-    }, [externalNavigate]);
+    }, [externalNavigate, route, scrollToSection]);
 
     useEffect(() => {
         const handleHashChange = () => {
@@ -172,6 +175,12 @@ const RoutingProvider: React.FC<RouteProviderProps> = ({externalNavigate, childr
         };
     }, []); // eslint-disable-line react-hooks/exhaustive-deps
 
+    useEffect(() => {
+        if (route !== undefined) {
+            updateNavigatedSection(route.split('/')[0]);
+        }
+    }, [route, updateNavigatedSection]);
+
     if (route === undefined) {
         return null;
     }
@@ -184,9 +193,7 @@ const RoutingProvider: React.FC<RouteProviderProps> = ({externalNavigate, childr
                 loadingModal
             }}
         >
-            <ScrollSectionProvider navigatedSection={route.split('/')[0]}>
-                {children}
-            </ScrollSectionProvider>
+            {children}
         </RouteContext.Provider>
     );
 };

apps/admin-x-settings/src/hooks/useScrollSection.tsx

@@ -4,12 +4,16 @@ interface ScrollSectionContextData {
     updateSection: (id: string, element: HTMLDivElement) => void;
     updateNav: (id: string, element: HTMLLIElement) => void;
     currentSection: string | null;
+    updateNavigatedSection: (id: string) => void;
+    scrollToSection: (id: string) => void;
 }
 
 const ScrollSectionContext = createContext<ScrollSectionContextData>({
     updateSection: () => {},
     updateNav: () => {},
-    currentSection: null
+    currentSection: null,
+    updateNavigatedSection: () => {},
+    scrollToSection: () => {}
 });
 
 export const useScrollSectionContext = () => useContext(ScrollSectionContext);
@@ -63,51 +67,70 @@ const scrollSidebarNav = (navElement: HTMLLIElement, doneInitialScroll: boolean)
     }
 };
 
+const getIntersectingSections = (current: string[], entries: IntersectionObserverEntry[], sectionElements: Record<string, HTMLDivElement>) => {
+    const entriesWithId = entries.map(({isIntersecting, target}) => ({
+        isIntersecting,
+        id: Object.entries(sectionElements).find(([, element]) => element === target)?.[0]
+    })).filter(entry => entry.id) as {id: string; isIntersecting: boolean}[];
+
+    const newlyIntersectingIds = entriesWithId.filter(entry => !current.includes(entry.id) && entry.isIntersecting).map(entry => entry.id);
+    const unintersectingIds = entriesWithId.filter(entry => !entry.isIntersecting).map(entry => entry.id);
+
+    const newSections = current.filter(section => !unintersectingIds.includes(section)).concat(newlyIntersectingIds);
+
+    newSections.sort((first, second) => {
+        const firstElement = sectionElements[first];
+        const secondElement = sectionElements[second];
+
+        if (!firstElement || !secondElement) {
+            return 0;
+        }
+
+        return firstElement.getBoundingClientRect().top - secondElement.getBoundingClientRect().top;
+    });
+
+    return newSections;
+};
+
 export const ScrollSectionProvider: React.FC<{
-    navigatedSection: string;
     children: ReactNode;
-}> = ({navigatedSection, children}) => {
+}> = ({children}) => {
+    const [navigatedSection, _setNavigatedSection] = useState<string | null>(null);
     const sectionElements = useRef<Record<string, HTMLDivElement>>({});
+    const intersectionObserver = useRef<IntersectionObserver | null>(null);
     const [intersectingSections, setIntersectingSections] = useState<string[]>([]);
     const [lastIntersectedSection, setLastIntersectedSection] = useState<string | null>(null);
 
+    const [hasUpdatedNavigatedSection, setHasUpdatedNavigatedSection] = useState(false);
     const [doneInitialScroll, setDoneInitialScroll] = useState(false);
     const [, setDoneSidebarScroll] = useState(false);
 
+    const setNavigatedSection = useCallback((value: string) => {
+        _setNavigatedSection(value);
+        setHasUpdatedNavigatedSection(true);
+    }, []);
+
     const navElements = useRef<Record<string, HTMLLIElement>>({});
 
-    const intersectionObserver = useMemo(() => new IntersectionObserver((entries) => {
-        const entriesWithId = entries.map(({isIntersecting, target}) => ({
-            isIntersecting,
-            id: Object.entries(sectionElements.current).find(([, element]) => element === target)?.[0]
-        })).filter(entry => entry.id) as {id: string; isIntersecting: boolean}[];
+    const setupIntersectionObserver = useCallback(() => {
+        const observer = new IntersectionObserver((entries) => {
+            setIntersectingSections((sections) => {
+                const newSections = getIntersectingSections(sections, entries, sectionElements.current);
 
-        setIntersectingSections((sections) => {
-            const newlyIntersectingIds = entriesWithId.filter(entry => !sections.includes(entry.id) && entry.isIntersecting).map(entry => entry.id);
-            const unintersectingIds = entriesWithId.filter(entry => !entry.isIntersecting).map(entry => entry.id);
-
-            const newSections = sections.filter(section => !unintersectingIds.includes(section)).concat(newlyIntersectingIds);
-
-            newSections.sort((first, second) => {
-                const firstElement = sectionElements.current[first];
-                const secondElement = sectionElements.current[second];
-
-                if (!firstElement || !secondElement) {
-                    return 0;
+                if (newSections.length) {
+                    setLastIntersectedSection(newSections[0]);
                 }
 
-                return firstElement.getBoundingClientRect().top - secondElement.getBoundingClientRect().top;
+                return newSections;
             });
-
-            if (newSections.length) {
-                setLastIntersectedSection(newSections[0]);
-            }
-
-            return newSections;
+        }, {
+            rootMargin: `-${scrollMargin - 50}px 0px -40% 0px`
         });
-    }, {
-        rootMargin: `-${scrollMargin - 50}px 0px -40% 0px`
-    }), []);
+
+        Object.values(sectionElements.current).forEach(element => observer.observe(element));
+
+        return observer;
+    }, []);
 
     const updateSection = useCallback((id: string, element: HTMLDivElement) => {
         if (sectionElements.current[id] === element) {
@@ -115,11 +138,11 @@ export const ScrollSectionProvider: React.FC<{
         }
 
         if (sectionElements.current[id]) {
-            intersectionObserver.unobserve(sectionElements.current[id]);
+            intersectionObserver.current?.unobserve(sectionElements.current[id]);
         }
 
         sectionElements.current[id] = element;
-        intersectionObserver.observe(element);
+        intersectionObserver.current?.observe(element);
 
         if (!doneInitialScroll && id === navigatedSection) {
             scrollToSection(element, false);
@@ -131,6 +154,12 @@ export const ScrollSectionProvider: React.FC<{
         navElements.current[id] = element;
     }, []);
 
+    const scrollTo = useCallback((id: string) => {
+        if (sectionElements.current[id]) {
+            scrollToSection(sectionElements.current[id], true);
+        }
+    }, []);
+
     const currentSection = useMemo(() => {
         if (navigatedSection && intersectingSections.includes(navigatedSection)) {
             return navigatedSection;
@@ -144,28 +173,40 @@ export const ScrollSectionProvider: React.FC<{
     }, [intersectingSections, lastIntersectedSection, navigatedSection]);
 
     useEffect(() => {
+        if (!hasUpdatedNavigatedSection) {
+            return;
+        }
+
         if (navigatedSection && sectionElements.current[navigatedSection]) {
             setDoneInitialScroll((done) => {
                 scrollToSection(sectionElements.current[navigatedSection], done);
                 return true;
             });
+        } else {
+            // No navigated section means opening settings without a path
+            setDoneInitialScroll(true);
         }
-    }, [navigatedSection]);
+
+        // Wait for the initial scroll so that the intersecting sections are correct
+        setTimeout(() => setupIntersectionObserver());
+    }, [hasUpdatedNavigatedSection, navigatedSection, setupIntersectionObserver]);
 
     useEffect(() => {
-        if (currentSection && navElements.current[currentSection]) {
+        if (hasUpdatedNavigatedSection && currentSection && navElements.current[currentSection]) {
             setDoneSidebarScroll((done) => {
                 scrollSidebarNav(navElements.current[currentSection], done);
                 return true;
             });
         }
-    }, [currentSection]);
+    }, [hasUpdatedNavigatedSection, currentSection]);
 
     return (
         <ScrollSectionContext.Provider value={{
             updateSection,
             updateNav,
-            currentSection
+            currentSection,
+            updateNavigatedSection: setNavigatedSection,
+            scrollToSection: scrollTo
         }}>
             {children}
         </ScrollSectionContext.Provider>