From e75bd0cca67db7eacf610408b9ad72f082835c26 Mon Sep 17 00:00:00 2001 From: minnakt <47064971+minnakt@users.noreply.github.com> Date: Mon, 21 Oct 2024 19:40:29 -0400 Subject: [PATCH] DEVPROD-11363: Support scrolling to different sections on Build Information page (#425) --- .../banners/github_username_banner.ts | 4 - .../integration/image/build_information.ts | 26 ++++++ .../cypress/integration/image/navigation.ts | 4 + .../src/analytics/image/useImageAnalytics.ts | 5 ++ .../Table/TableLoader/LoadingRow.tsx | 2 +- .../__snapshots__/BaseTable_Loading.storyshot | 5 ++ apps/spruce/src/constants/routes.ts | 8 +- apps/spruce/src/hooks/index.ts | 1 + .../src/hooks/useScrollToAnchor/index.ts | 4 +- .../useScrollToAnchor.test.tsx | 6 +- .../hooks/useTopmostVisibleElement/index.ts | 63 +++++++++++++ .../useTopmostVisibleElement.test.tsx | 51 +++++++++++ .../src/pages/image/GeneralTable/index.tsx | 7 +- apps/spruce/src/pages/image/index.tsx | 40 +++++---- .../BuildInformationNavItem.tsx | 88 +++++++++++++++++++ .../tabs/BuildInformationTab/constants.ts | 29 ++++++ .../image/tabs/BuildInformationTab/index.tsx | 31 +++++-- 17 files changed, 339 insertions(+), 35 deletions(-) create mode 100644 apps/spruce/src/hooks/useTopmostVisibleElement/index.ts create mode 100644 apps/spruce/src/hooks/useTopmostVisibleElement/useTopmostVisibleElement.test.tsx create mode 100644 apps/spruce/src/pages/image/tabs/BuildInformationTab/BuildInformationNavItem.tsx create mode 100644 apps/spruce/src/pages/image/tabs/BuildInformationTab/constants.ts diff --git a/apps/spruce/cypress/integration/banners/github_username_banner.ts b/apps/spruce/cypress/integration/banners/github_username_banner.ts index 19390e990..2683495d8 100644 --- a/apps/spruce/cypress/integration/banners/github_username_banner.ts +++ b/apps/spruce/cypress/integration/banners/github_username_banner.ts @@ -3,8 +3,4 @@ describe("Github username banner", () => { cy.visit("/"); cy.dataCy("github-username-banner").should("exist"); }); - it("should not show the banner on other pages, even if user doesn't have a github username", () => { - cy.visit("/hosts"); - cy.dataCy("github-username-banner").should("not.exist"); - }); }); diff --git a/apps/spruce/cypress/integration/image/build_information.ts b/apps/spruce/cypress/integration/image/build_information.ts index 9711f8b8c..6d900fff9 100644 --- a/apps/spruce/cypress/integration/image/build_information.ts +++ b/apps/spruce/cypress/integration/image/build_information.ts @@ -167,3 +167,29 @@ describe("build information", () => { }); }); }); + +describe("side nav", () => { + beforeEach(() => { + cy.visit("/image/ubuntu2204/build-information"); + cy.contains("ubuntu2204").should("be.visible"); + cy.dataCy("table-loader-loading-row").should("not.exist"); + }); + + it("highlights different sections as the user scrolls", () => { + cy.dataCy("general-card").scrollIntoView(); + cy.dataCy("navitem-general").should("have.attr", "data-active", "true"); + cy.dataCy("navitem-distros").should("have.attr", "data-active", "false"); + + cy.dataCy("distros-card").scrollIntoView(); + cy.dataCy("navitem-general").should("have.attr", "data-active", "false"); + cy.dataCy("navitem-distros").should("have.attr", "data-active", "true"); + }); + + it("can click to navigate to different sections", () => { + cy.dataCy("navitem-packages").should("have.attr", "data-active", "false"); + cy.dataCy("packages-card").should("not.be.visible"); + cy.dataCy("navitem-packages").click(); + cy.dataCy("navitem-packages").should("have.attr", "data-active", "true"); + cy.dataCy("packages-card").should("be.visible"); + }); +}); diff --git a/apps/spruce/cypress/integration/image/navigation.ts b/apps/spruce/cypress/integration/image/navigation.ts index 8a26d2c2e..64abecddf 100644 --- a/apps/spruce/cypress/integration/image/navigation.ts +++ b/apps/spruce/cypress/integration/image/navigation.ts @@ -6,7 +6,9 @@ describe("/image/imageId/random redirect route", () => { cy.location("pathname").should("not.contain", "/random"); cy.location("pathname").should("eq", "/image/imageId/build-information"); }); +}); +describe("image dropdown", () => { it("navigates to the image when clicked", () => { cy.visit("/image/amazon2/build-information"); cy.dataCy("images-select").should("be.visible").as("button"); @@ -24,7 +26,9 @@ describe("/image/imageId/random redirect route", () => { }); }); }); +}); +describe("task metadata", () => { it("shows the image visibility guide cue on task metadata", () => { cy.setCookie(SEEN_IMAGE_VISIBILITY_GUIDE_CUE, "false"); cy.visit( diff --git a/apps/spruce/src/analytics/image/useImageAnalytics.ts b/apps/spruce/src/analytics/image/useImageAnalytics.ts index bbb18d2e7..21d02e895 100644 --- a/apps/spruce/src/analytics/image/useImageAnalytics.ts +++ b/apps/spruce/src/analytics/image/useImageAnalytics.ts @@ -28,6 +28,11 @@ type Action = name: "Changed tab"; tab: ImageTabRoutes; } + | { + name: "Clicked section"; + tab: ImageTabRoutes; + "tab.section": string; + } | { name: "Clicked 'Load more events' button"; } diff --git a/apps/spruce/src/components/Table/TableLoader/LoadingRow.tsx b/apps/spruce/src/components/Table/TableLoader/LoadingRow.tsx index 797edef41..bbf63e055 100644 --- a/apps/spruce/src/components/Table/TableLoader/LoadingRow.tsx +++ b/apps/spruce/src/components/Table/TableLoader/LoadingRow.tsx @@ -5,7 +5,7 @@ interface LoadingRowProps { numColumns: number; } const LoadingRow: React.FC = ({ numColumns }) => ( - + {Array.from({ length: numColumns }, (_, i) => ( diff --git a/apps/spruce/src/components/Table/__snapshots__/BaseTable_Loading.storyshot b/apps/spruce/src/components/Table/__snapshots__/BaseTable_Loading.storyshot index 068b5b157..1edded576 100644 --- a/apps/spruce/src/components/Table/__snapshots__/BaseTable_Loading.storyshot +++ b/apps/spruce/src/components/Table/__snapshots__/BaseTable_Loading.storyshot @@ -235,6 +235,7 @@ PageNames.Patches }`; -export const getImageRoute = (imageId: string, tab?: ImageTabRoutes) => - `${paths.image}/${imageId}/${tab || ImageTabRoutes.BuildInformation}`; +export const getImageRoute = ( + imageId: string, + tab?: ImageTabRoutes, + anchor?: string, +) => + `${paths.image}/${imageId}/${tab ?? ImageTabRoutes.BuildInformation}${anchor ? `#${anchor}` : ""}`; export const getProjectSettingsRoute = ( projectId: string, diff --git a/apps/spruce/src/hooks/index.ts b/apps/spruce/src/hooks/index.ts index 3f22732ef..97c07dfdf 100644 --- a/apps/spruce/src/hooks/index.ts +++ b/apps/spruce/src/hooks/index.ts @@ -24,3 +24,4 @@ export { useFirstImage } from "./useFirstImage"; export { useBreadcrumbRoot } from "./useBreadcrumbRoot"; export { useResize } from "./useResize"; export { useRunningTime } from "./useRunningTime"; +export { useTopmostVisibleElement } from "./useTopmostVisibleElement"; diff --git a/apps/spruce/src/hooks/useScrollToAnchor/index.ts b/apps/spruce/src/hooks/useScrollToAnchor/index.ts index 07a9023f8..132e4efd0 100644 --- a/apps/spruce/src/hooks/useScrollToAnchor/index.ts +++ b/apps/spruce/src/hooks/useScrollToAnchor/index.ts @@ -1,6 +1,8 @@ import { useEffect, useRef } from "react"; import { useLocation } from "react-router-dom"; +export const anchorScrollTime = 100; + /** * `useScrollToAnchor` scrolls to an anchor element on the page if the URL contains an anchor. */ @@ -18,7 +20,7 @@ const useScrollToAnchor = () => { if (element) { element.scrollIntoView({ behavior: "smooth" }); } - }, 500); + }, anchorScrollTime); }, [anchor]); useEffect( diff --git a/apps/spruce/src/hooks/useScrollToAnchor/useScrollToAnchor.test.tsx b/apps/spruce/src/hooks/useScrollToAnchor/useScrollToAnchor.test.tsx index 429593d7b..6f16ed950 100644 --- a/apps/spruce/src/hooks/useScrollToAnchor/useScrollToAnchor.test.tsx +++ b/apps/spruce/src/hooks/useScrollToAnchor/useScrollToAnchor.test.tsx @@ -25,7 +25,7 @@ describe("useScrollToAnchor", () => { ); renderHook(() => useScrollToAnchor(), { wrapper }); - vi.advanceTimersByTime(500); + vi.runOnlyPendingTimers(); expect(document.getElementById).toHaveBeenCalledWith("test-anchor"); expect(mockElement.scrollIntoView).toHaveBeenCalledWith({ @@ -40,7 +40,7 @@ describe("useScrollToAnchor", () => { ); renderHook(() => useScrollToAnchor(), { wrapper }); - vi.advanceTimersByTime(500); + vi.runOnlyPendingTimers(); expect(document.getElementById).not.toHaveBeenCalled(); }); @@ -53,7 +53,7 @@ describe("useScrollToAnchor", () => { const { unmount } = renderHook(() => useScrollToAnchor(), { wrapper }); unmount(); - vi.advanceTimersByTime(500); + vi.runOnlyPendingTimers(); expect(mockElement.scrollIntoView).not.toHaveBeenCalled(); }); diff --git a/apps/spruce/src/hooks/useTopmostVisibleElement/index.ts b/apps/spruce/src/hooks/useTopmostVisibleElement/index.ts new file mode 100644 index 000000000..46d9d2271 --- /dev/null +++ b/apps/spruce/src/hooks/useTopmostVisibleElement/index.ts @@ -0,0 +1,63 @@ +import { useEffect, useState } from "react"; + +const findTopmostVisibleElement = ({ + elements, + scrollTop, +}: { + elements: HTMLElement[]; + scrollTop: number; +}) => { + let minDistance = Number.MAX_VALUE; + let minDistanceId = ""; + + elements.forEach((el) => { + const currDistance = Math.abs(scrollTop - el.offsetTop); + if (currDistance < minDistance) { + minDistance = currDistance; + minDistanceId = el.id; + } + }); + + return minDistanceId; +}; + +/** + * `useTopmostVisibleElement` is used to track the ID of the element that is visible on the screen and closest + * to the top of the element with scrollContainerId. + * @param obj - object representing arguments to `useTopmostVisibleElement` hook + * @param obj.elements - list of elements from which to determine the topmost visible element + * @param obj.scrollContainerId - the ID of the scroll container. Referencing the scroll container is necessary + * because the hook listens on the "scroll" event. + * @returns the ID of the topmost visible element (string) + */ +export const useTopmostVisibleElement = ({ + elements, + scrollContainerId, +}: { + elements: HTMLElement[]; + scrollContainerId: string; +}) => { + const [topmostVisibleElementId, setTopmostVisibleElementId] = useState(""); + + useEffect(() => { + const scrollElement = document.getElementById( + scrollContainerId, + ) as HTMLElement; + + const handleScroll = () => { + const id = findTopmostVisibleElement({ + scrollTop: scrollElement.scrollTop, + elements, + }); + setTopmostVisibleElementId(id); + }; + + scrollElement.addEventListener("scroll", handleScroll); + return () => { + scrollElement.removeEventListener("scroll", handleScroll); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [elements]); + + return topmostVisibleElementId; +}; diff --git a/apps/spruce/src/hooks/useTopmostVisibleElement/useTopmostVisibleElement.test.tsx b/apps/spruce/src/hooks/useTopmostVisibleElement/useTopmostVisibleElement.test.tsx new file mode 100644 index 000000000..002712423 --- /dev/null +++ b/apps/spruce/src/hooks/useTopmostVisibleElement/useTopmostVisibleElement.test.tsx @@ -0,0 +1,51 @@ +import { act, renderHook, render } from "@evg-ui/lib/test_utils"; +import { useTopmostVisibleElement } from "."; + +describe("useTopmostVisibleElement", () => { + const scrollPage = (scrollContainerId: string, height: number) => { + act(() => { + const scrollElement = document.getElementById( + scrollContainerId, + ) as HTMLElement; + scrollElement.scrollTop = height; + scrollElement.dispatchEvent(new window.Event("scroll")); + }); + }; + + it("should correctly detect the topmost visible element", async () => { + const scrollContainerId = "scroll-container-id"; + + render( +
+ + +
, + ); + + // JSDom doesn't actually support layouting HTML so scroll/offset positions must be mocked. + const firstRefElement = document.getElementById("span-1") as HTMLElement; + vi.spyOn(firstRefElement, "offsetTop", "get").mockImplementation(() => 0); + + const secondRefElement = document.getElementById("span-2") as HTMLElement; + vi.spyOn(secondRefElement, "offsetTop", "get").mockImplementation( + () => 500, + ); + + const elements = Array.from( + document.querySelectorAll("span"), + ) as HTMLElement[]; + + const { result } = renderHook(() => + useTopmostVisibleElement({ + elements, + scrollContainerId, + }), + ); + + scrollPage(scrollContainerId, 408); + expect(result.current).toBe("span-2"); + + scrollPage(scrollContainerId, 121); + expect(result.current).toBe("span-1"); + }); +}); diff --git a/apps/spruce/src/pages/image/GeneralTable/index.tsx b/apps/spruce/src/pages/image/GeneralTable/index.tsx index fab98b071..8e076c983 100644 --- a/apps/spruce/src/pages/image/GeneralTable/index.tsx +++ b/apps/spruce/src/pages/image/GeneralTable/index.tsx @@ -38,8 +38,11 @@ export const GeneralTable: React.FC = ({ imageId }) => { const getDateCopy = useDateFormat(); const data = useMemo(() => { - const image = imageData?.image; + if (loading) { + return []; + } + const image = imageData?.image; return [ { property: "Last deployed", @@ -83,7 +86,7 @@ export const GeneralTable: React.FC = ({ imageId }) => { diff --git a/apps/spruce/src/pages/image/index.tsx b/apps/spruce/src/pages/image/index.tsx index 98b648eef..65dd56c17 100644 --- a/apps/spruce/src/pages/image/index.tsx +++ b/apps/spruce/src/pages/image/index.tsx @@ -4,7 +4,6 @@ import { Link, useParams, Navigate } from "react-router-dom"; import { useImageAnalytics } from "analytics"; import { SideNav, - SideNavGroup, SideNavItem, SideNavPageContent, SideNavPageWrapper, @@ -12,9 +11,9 @@ import { import { ImageTabRoutes, getImageRoute, slugs } from "constants/routes"; import { size } from "constants/tokens"; import { useFirstImage } from "hooks"; -import { getTabTitle } from "./getTabTitle"; import { ImageSelect } from "./ImageSelect"; import { ImageTabs } from "./Tabs"; +import BuildInformationNavItem from "./tabs/BuildInformationTab/BuildInformationNavItem"; const Image: React.FC = () => { const { [slugs.imageId]: imageId, [slugs.tab]: currentTab } = useParams<{ @@ -25,6 +24,7 @@ const Image: React.FC = () => { const { image: firstImage } = useFirstImage(); const selectedImage = imageId ?? firstImage; + const scrollContainerId = "side-nav-page-content"; if ( currentTab === undefined || @@ -44,22 +44,27 @@ const Image: React.FC = () => { - - {Object.values(ImageTabRoutes).map((tab) => ( - sendEvent({ name: "Changed tab", tab })} - to={getImageRoute(selectedImage, tab)} - > - {getTabTitle(tab).title} - - ))} - + + + sendEvent({ name: "Changed tab", tab: ImageTabRoutes.EventLog }) + } + to={getImageRoute(selectedImage, ImageTabRoutes.EventLog)} + > + Event Log + - + @@ -71,6 +76,7 @@ const ButtonsContainer = styled.div` flex-direction: column; gap: ${size.xs}; margin: 0 ${sideNavItemSidePadding}px; + margin-bottom: ${size.xs}; `; export default Image; diff --git a/apps/spruce/src/pages/image/tabs/BuildInformationTab/BuildInformationNavItem.tsx b/apps/spruce/src/pages/image/tabs/BuildInformationTab/BuildInformationNavItem.tsx new file mode 100644 index 000000000..5205bfc50 --- /dev/null +++ b/apps/spruce/src/pages/image/tabs/BuildInformationTab/BuildInformationNavItem.tsx @@ -0,0 +1,88 @@ +import { css } from "@emotion/react"; +import { Link } from "react-router-dom"; +import { useImageAnalytics } from "analytics"; +import { SideNavItem } from "components/styles"; +import { ImageTabRoutes, getImageRoute } from "constants/routes"; +import { useTopmostVisibleElement } from "hooks"; +import { tocItems } from "./constants"; + +type BuildInformationNavItemProps = { + currentTab: ImageTabRoutes; + imageId: string; + scrollContainerId: string; +}; + +const BuildInformationNavItem: React.FC = ({ + currentTab, + imageId, + scrollContainerId, +}) => { + const { sendEvent } = useImageAnalytics(); + const tabIsActive = ImageTabRoutes.BuildInformation === currentTab; + + const elements = Array.from( + document.querySelectorAll("h3[id^='toc-item']"), + ) as HTMLElement[]; + + const activeItemId = useTopmostVisibleElement({ + elements, + scrollContainerId, + }); + + return ( + <> + + sendEvent({ + name: "Changed tab", + tab: ImageTabRoutes.BuildInformation, + }) + } + to={getImageRoute(imageId, ImageTabRoutes.BuildInformation)} + > + Build Information + + {Object.keys(tocItems).map((key) => { + const item = tocItems[key]; + const isActive = tabIsActive && item.observedElementId === activeItemId; + return ( + + sendEvent({ + name: "Clicked section", + tab: ImageTabRoutes.BuildInformation, + "tab.section": item.id, + }) + } + to={getImageRoute( + imageId, + ImageTabRoutes.BuildInformation, + item.id, + )} + > + {item.title} + + ); + })} + + ); +}; + +export default BuildInformationNavItem; diff --git a/apps/spruce/src/pages/image/tabs/BuildInformationTab/constants.ts b/apps/spruce/src/pages/image/tabs/BuildInformationTab/constants.ts new file mode 100644 index 000000000..8dd555947 --- /dev/null +++ b/apps/spruce/src/pages/image/tabs/BuildInformationTab/constants.ts @@ -0,0 +1,29 @@ +export const tocItems: { + [key: string]: { observedElementId: string; id: string; title: string }; +} = { + general: { + id: "general", + observedElementId: "toc-item-general", + title: "General", + }, + distros: { + id: "distros", + observedElementId: "toc-item-distros", + title: "Distros", + }, + os: { + id: "operating-system", + observedElementId: "toc-item-os", + title: "Operating System", + }, + packages: { + id: "packages", + observedElementId: "toc-item-packages", + title: "Packages", + }, + toolchains: { + id: "toolchains", + observedElementId: "toc-item-toolchains", + title: "Toolchains", + }, +}; diff --git a/apps/spruce/src/pages/image/tabs/BuildInformationTab/index.tsx b/apps/spruce/src/pages/image/tabs/BuildInformationTab/index.tsx index 07909818b..5661cdfee 100644 --- a/apps/spruce/src/pages/image/tabs/BuildInformationTab/index.tsx +++ b/apps/spruce/src/pages/image/tabs/BuildInformationTab/index.tsx @@ -4,6 +4,7 @@ import { GeneralTable } from "pages/image/GeneralTable"; import { OperatingSystemTable } from "pages/image/OperatingSystemTable"; import { PackagesTable } from "pages/image/PackagesTable"; import { ToolchainsTable } from "pages/image/ToolchainsTable"; +import { tocItems } from "./constants"; type BuildInformationTabProps = { imageId: string; @@ -13,19 +14,39 @@ export const BuildInformationTab: React.FC = ({ imageId, }) => ( <> - + - + - + - + - +