From 7239c2309a30b8d114e5280f66422e7eaef923e2 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 28 Feb 2025 10:11:01 -0500 Subject: [PATCH] Tie chatbots to URL parameters (#2076) * make LinkAdapter use shallow * tie AskTim to drawer query param * display syllabus chat based on query param --- frontends/main/src/common/metadata.ts | 6 +- frontends/main/src/common/urls.ts | 7 +- .../AiChat/AiChatWithEntryScreen.tsx | 2 +- .../AiChat/AiRecommendationBotDrawer.tsx | 60 ++++++------- .../AiChat/AskTimDrawerButton.test.tsx | 42 +++++++++ .../AiChat/AskTimDrawerButton.tsx | 14 +-- .../LearningResourceDrawer.test.tsx | 87 ++++++++++++++++++- .../LearningResourceDrawer.tsx | 20 +++-- .../useResourceDrawerHref.ts | 4 +- .../AiChatSyllabusSlideDown.test.tsx | 12 ++- .../AiChatSyllabusSlideDown.tsx | 13 ++- .../LearningResourceExpanded.test.tsx | 35 ++++++-- .../LearningResourceExpanded.tsx | 33 ++++++- .../ResourceCard/ResourceCard.test.tsx | 4 +- frontends/main/src/test-utils/index.tsx | 4 +- .../src/components/Link/Link.tsx | 45 +++------- .../components/LinkAdapter/LinkAdapter.tsx | 51 +++++++++++ .../components/RoutedDrawer/RoutedDrawer.tsx | 7 ++ .../ThemeProvider/ThemeProvider.tsx | 12 ++- 19 files changed, 347 insertions(+), 111 deletions(-) create mode 100644 frontends/main/src/page-components/AiChat/AskTimDrawerButton.test.tsx create mode 100644 frontends/ol-components/src/components/LinkAdapter/LinkAdapter.tsx diff --git a/frontends/main/src/common/metadata.ts b/frontends/main/src/common/metadata.ts index 583e5a6383..b41966d98c 100644 --- a/frontends/main/src/common/metadata.ts +++ b/frontends/main/src/common/metadata.ts @@ -1,4 +1,4 @@ -import { RESOURCE_DRAWER_QUERY_PARAM } from "@/common/urls" +import { RESOURCE_DRAWER_PARAMS } from "@/common/urls" import { learningResourcesApi } from "api/clients" import type { Metadata } from "next" import handleNotFound from "./handleNotFound" @@ -28,7 +28,9 @@ export const getMetadataAsync = async ({ ...otherMeta }: MetadataAsyncProps) => { // The learning resource drawer is open - const learningResourceId = (await searchParams)?.[RESOURCE_DRAWER_QUERY_PARAM] + const learningResourceId = (await searchParams)?.[ + RESOURCE_DRAWER_PARAMS.resource + ] if (learningResourceId) { const { data } = await handleNotFound( learningResourcesApi.learningResourcesRetrieve({ diff --git a/frontends/main/src/common/urls.ts b/frontends/main/src/common/urls.ts index b070f9b48f..9611dd62ef 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -112,7 +112,12 @@ export const UNITS = "/units" export const CONTACT = "mailto:mitlearn-support@mit.edu" -export const RESOURCE_DRAWER_QUERY_PARAM = "resource" +export const RECOMMENDER_QUERY_PARAM = "recommender" + +export const RESOURCE_DRAWER_PARAMS = { + resource: "resource", + syllabus: "syllabus", +} as const export const querifiedSearchUrl = ( params: diff --git a/frontends/main/src/page-components/AiChat/AiChatWithEntryScreen.tsx b/frontends/main/src/page-components/AiChat/AiChatWithEntryScreen.tsx index d95beddcc4..d6213f5a18 100644 --- a/frontends/main/src/page-components/AiChat/AiChatWithEntryScreen.tsx +++ b/frontends/main/src/page-components/AiChat/AiChatWithEntryScreen.tsx @@ -170,7 +170,7 @@ const AiChatWithEntryScreen = ({ return ( {showEntryScreen ? ( - + diff --git a/frontends/main/src/page-components/AiChat/AiRecommendationBotDrawer.tsx b/frontends/main/src/page-components/AiChat/AiRecommendationBotDrawer.tsx index 146fa934bd..4026ba4986 100644 --- a/frontends/main/src/page-components/AiChat/AiRecommendationBotDrawer.tsx +++ b/frontends/main/src/page-components/AiChat/AiRecommendationBotDrawer.tsx @@ -1,10 +1,11 @@ import React from "react" -import { styled, Drawer } from "ol-components" +import { styled, RoutedDrawer } from "ol-components" import { RiCloseLine } from "@remixicon/react" import { ActionButton } from "@mitodl/smoot-design" import type { AiChatProps } from "@mitodl/smoot-design/ai" import AiChatWithEntryScreen from "./AiChatWithEntryScreen" import { getCsrfToken } from "@/common/utils" +import { RECOMMENDER_QUERY_PARAM } from "@/common/urls" const CloseButton = styled(ActionButton)(({ theme }) => ({ position: "absolute", @@ -51,37 +52,15 @@ const STARTERS = [ }, ] -const AiRecommendationBotDrawer = ({ - open, - setOpen, -}: { - open: boolean - setOpen: (open: boolean) => void -}) => { - const closeDrawer = () => { - setOpen(false) - // setShowEntryScreen(true) - } - +const DrawerContent: React.FC<{ + onClose?: () => void +}> = ({ onClose }) => { return ( - ({ - [theme.breakpoints.down("md")]: { - width: "100%", - }, - }), - }, - }} - > + <> @@ -104,7 +83,30 @@ const AiRecommendationBotDrawer = ({ }), }} /> - + + ) +} + +const DRAWER_REQUIRED_PARAMS = [RECOMMENDER_QUERY_PARAM] as const +const AiRecommendationBotDrawer = () => { + return ( + ({ + [theme.breakpoints.down("md")]: { + width: "100%", + }, + }), + }, + }} + > + {({ closeDrawer }) => } + ) } diff --git a/frontends/main/src/page-components/AiChat/AskTimDrawerButton.test.tsx b/frontends/main/src/page-components/AiChat/AskTimDrawerButton.test.tsx new file mode 100644 index 0000000000..106d45493f --- /dev/null +++ b/frontends/main/src/page-components/AiChat/AskTimDrawerButton.test.tsx @@ -0,0 +1,42 @@ +import React from "react" +import { renderWithProviders, screen, user, waitFor } from "@/test-utils" +import AskTIMButton from "./AskTimDrawerButton" +import { RECOMMENDER_QUERY_PARAM } from "@/common/urls" + +describe("AskTIMButton", () => { + it.each([ + { url: "", open: false }, + { url: `?${RECOMMENDER_QUERY_PARAM}`, open: true }, + ])("Opens drawer based on URL param", async ({ url, open }) => { + renderWithProviders(, { + url, + }) + + const aiChat = screen.queryByTestId("ai-chat-entry-screen") + expect(!!aiChat).toBe(open) + }) + + test("Clicking button opens / closes drawer", async () => { + const { location } = renderWithProviders() + + expect(location.current.searchParams.has(RECOMMENDER_QUERY_PARAM)).toBe( + false, + ) + + const askTim = screen.getByRole("link", { name: /ask tim/i }) + + await user.click(askTim) + + expect(location.current.searchParams.has(RECOMMENDER_QUERY_PARAM)).toBe( + true, + ) + + await user.click(screen.getByRole("button", { name: "Close" })) + + await waitFor(() => { + expect(location.current.searchParams.has(RECOMMENDER_QUERY_PARAM)).toBe( + false, + ) + }) + }) +}) diff --git a/frontends/main/src/page-components/AiChat/AskTimDrawerButton.tsx b/frontends/main/src/page-components/AiChat/AskTimDrawerButton.tsx index 5be9380656..962d90d329 100644 --- a/frontends/main/src/page-components/AiChat/AskTimDrawerButton.tsx +++ b/frontends/main/src/page-components/AiChat/AskTimDrawerButton.tsx @@ -1,10 +1,11 @@ -import React, { useState } from "react" +import React from "react" import { Typography, styled } from "ol-components" -import { Button } from "@mitodl/smoot-design" +import { ButtonLink } from "@mitodl/smoot-design" import { RiSparkling2Line } from "@remixicon/react" import AiRecommendationBotDrawer from "./AiRecommendationBotDrawer" +import { RECOMMENDER_QUERY_PARAM } from "@/common/urls" -const StyledButton = styled(Button)(({ theme }) => ({ +const StyledButton = styled(ButtonLink)(({ theme }) => ({ display: "flex", flexDirection: "row", gap: "8px", @@ -30,21 +31,20 @@ const StyledButton = styled(Button)(({ theme }) => ({ })) const AskTIMButton = () => { - const [open, setOpen] = useState(false) - return ( <> setOpen(true)} + href={`?${RECOMMENDER_QUERY_PARAM}`} > AskTIM - + ) } diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx index 967badc543..4f244a59de 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx @@ -1,19 +1,21 @@ import React from "react" import { + expectLastProps, expectProps, renderWithProviders, screen, + user, waitFor, within, } from "@/test-utils" import LearningResourceDrawer from "./LearningResourceDrawer" import { urls, factories, setMockResponse } from "api/test-utils" import { LearningResourceExpanded } from "../LearningResourceExpanded/LearningResourceExpanded" -import { RESOURCE_DRAWER_QUERY_PARAM } from "@/common/urls" +import { RESOURCE_DRAWER_PARAMS } from "@/common/urls" import { LearningResource, ResourceTypeEnum } from "api" import { makeUserSettings } from "@/test-utils/factories" import type { User } from "api/hooks/user" -import { usePostHog } from "posthog-js/react" +import { useFeatureFlagEnabled, usePostHog } from "posthog-js/react" jest.mock("../LearningResourceExpanded/LearningResourceExpanded", () => { const actual = jest.requireActual( @@ -31,6 +33,9 @@ jest.mocked(usePostHog).mockReturnValue( // @ts-expect-error Not mocking all of posthog { capture: mockedPostHogCapture }, ) +const mockedUseFeatureFlagEnabled = jest + .mocked(useFeatureFlagEnabled) + .mockImplementation(() => false) describe("LearningResourceDrawer", () => { const setupApis = ( @@ -94,7 +99,7 @@ describe("LearningResourceDrawer", () => { : "" renderWithProviders(, { - url: `?dog=woof&${RESOURCE_DRAWER_QUERY_PARAM}=${resource.id}`, + url: `?dog=woof&${RESOURCE_DRAWER_PARAMS.resource}=${resource.id}`, }) expect(LearningResourceExpanded).toHaveBeenCalled() await waitFor(() => { @@ -220,4 +225,80 @@ describe("LearningResourceDrawer", () => { similarResources.some((r) => text.includes(r.title)), ) }) + + it.each([ + { extraQueryParams: "", expectChat: false }, + { + extraQueryParams: `&${RESOURCE_DRAWER_PARAMS.syllabus}`, + expectChat: true, + }, + ])( + "Renders drawer with chatExpanded based on URL", + async ({ extraQueryParams, expectChat }) => { + mockedUseFeatureFlagEnabled.mockReturnValue(true) + const { resource } = setupApis({ + resource: { + // Chat is only enabled for courses + resource_type: ResourceTypeEnum.Course, + }, + }) + renderWithProviders(, { + url: `?resource=${resource.id}${extraQueryParams}`, + }) + + await screen.findByText(resource.title) + + await waitFor(() => { + expectLastProps(LearningResourceExpanded, { + resource, + chatExpanded: expectChat, + }) + }) + }, + ) + + test("If chat is not supported, 'syllabus' param removed from URL", async () => { + mockedUseFeatureFlagEnabled.mockReturnValue(true) + const { resource } = setupApis({ + resource: { + // Chat is only enabled for courses; NOT enabled here + resource_type: ResourceTypeEnum.Program, + }, + }) + const { location } = renderWithProviders(, { + url: `?resource=${resource.id}&syllabus`, + }) + + expect(location.current.searchParams.has("syllabus")).toBe(true) + + await waitFor(() => { + expectLastProps(LearningResourceExpanded, { + resource, + chatExpanded: false, + }) + }) + expect(location.current.searchParams.has("syllabus")).toBe(false) + }) + + test("Clicking 'Ask Tim' toggles chat query param", async () => { + mockedUseFeatureFlagEnabled.mockReturnValue(true) + const { resource } = setupApis({ + resource: { + // Chat is only enabled for courses + resource_type: ResourceTypeEnum.Course, + }, + }) + const { location } = renderWithProviders(, { + url: `?resource=${resource.id}`, + }) + + const askTimButton = await screen.findByRole("button", { name: /Ask\sTIM/ }) + expect(askTimButton).toBeInTheDocument() + + expect(location.current.searchParams.has("syllabus")).toBe(false) + await user.click(askTimButton) + expect(location.current.searchParams.has("syllabus")).toBe(true) + await user.click(askTimButton) + expect(location.current.searchParams.has("syllabus")).toBe(false) + }) }) diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx index 975ffb8d25..28e350885b 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx @@ -7,7 +7,7 @@ import type { } from "ol-components" import { useLearningResourcesDetail } from "api/hooks/learningResources" -import { RESOURCE_DRAWER_QUERY_PARAM } from "@/common/urls" +import { RESOURCE_DRAWER_PARAMS } from "@/common/urls" import { useUserMe } from "api/hooks/user" import NiceModal from "@ebay/nice-modal-react" import { @@ -23,7 +23,11 @@ import { TopicCarouselConfig } from "@/common/carousels" import { ResourceTypeEnum } from "api" import { PostHogEvents } from "@/common/constants" -const RESOURCE_DRAWER_PARAMS = [RESOURCE_DRAWER_QUERY_PARAM] as const +const REQUIRED_PARAMS = [RESOURCE_DRAWER_PARAMS.resource] as const +const ALL_PARAMS = [ + RESOURCE_DRAWER_PARAMS.resource, + RESOURCE_DRAWER_PARAMS.syllabus, +] as const const useCapturePageView = (resourceId: number) => { const { data, isSuccess } = useLearningResourcesDetail(Number(resourceId)) @@ -54,7 +58,8 @@ const DrawerContent: React.FC<{ resourceId: number titleId: string closeDrawer: () => void -}> = ({ resourceId, closeDrawer, titleId }) => { + chatExpanded: boolean +}> = ({ resourceId, closeDrawer, titleId, chatExpanded }) => { /** * Ideally the resource data should already exist in the query cache, e.g., by: * - a server-side prefetch @@ -207,8 +212,9 @@ const DrawerContent: React.FC<{ resource={resource.data} topCarousels={topCarousels} bottomCarousels={bottomCarousels} + chatExpanded={chatExpanded} user={user} - shareUrl={`${window.location.origin}/search?${RESOURCE_DRAWER_QUERY_PARAM}=${resourceId}`} + shareUrl={`${window.location.origin}/search?${RESOURCE_DRAWER_PARAMS.resource}=${resourceId}`} inLearningPath={inLearningPath} inUserList={inUserList} onAddToLearningPathClick={handleAddToLearningPathClick} @@ -244,7 +250,8 @@ const LearningResourceDrawer = () => { { {({ params, closeDrawer }) => { return ( ) diff --git a/frontends/main/src/page-components/LearningResourceDrawer/useResourceDrawerHref.ts b/frontends/main/src/page-components/LearningResourceDrawer/useResourceDrawerHref.ts index e4b7ce055f..ed000e47a4 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/useResourceDrawerHref.ts +++ b/frontends/main/src/page-components/LearningResourceDrawer/useResourceDrawerHref.ts @@ -1,5 +1,5 @@ import { useCallback } from "react" -import { RESOURCE_DRAWER_QUERY_PARAM } from "@/common/urls" +import { RESOURCE_DRAWER_PARAMS } from "@/common/urls" import { ReadonlyURLSearchParams, useSearchParams } from "next/navigation" const getOpenDrawerSearchParams = ( @@ -7,7 +7,7 @@ const getOpenDrawerSearchParams = ( resourceId: number, ) => { const newSearchParams = new URLSearchParams(current) - newSearchParams.set(RESOURCE_DRAWER_QUERY_PARAM, resourceId.toString()) + newSearchParams.set(RESOURCE_DRAWER_PARAMS.resource, resourceId.toString()) return newSearchParams } diff --git a/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.test.tsx b/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.test.tsx index c06675376a..3c390b1056 100644 --- a/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.test.tsx +++ b/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.test.tsx @@ -18,7 +18,11 @@ describe("AiChatSyllabus", () => { setMockResponse.get(urls.userMe.get(), userMe) renderWithProviders( - , + , ) await user.click( @@ -37,7 +41,11 @@ describe("AiChatSyllabus", () => { setMockResponse.get(urls.userMe.get(), {}, { code: 403 }) renderWithProviders( - , + , ) const input = screen.getByRole("textbox") diff --git a/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.tsx b/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.tsx index 8da3c7afd2..e7455fed37 100644 --- a/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.tsx +++ b/frontends/main/src/page-components/LearningResourceExpanded/AiChatSyllabusSlideDown.tsx @@ -1,4 +1,4 @@ -import React, { useState } from "react" +import React from "react" import { Typography, styled } from "ol-components" import { Button } from "@mitodl/smoot-design" import { @@ -116,18 +116,14 @@ const getInitialMessage = ( const AiChatSyllabusSlideDown = ({ resource, onToggleOpen, + open, }: { resource?: LearningResource + open: boolean onToggleOpen: (open: boolean) => void }) => { - const [open, setOpen] = useState(false) const user = useUserMe() - const toggleOpen = () => { - setOpen(!open) - onToggleOpen(!open) - } - if (!resource) return null return ( @@ -136,8 +132,9 @@ const AiChatSyllabusSlideDown = ({ onToggleOpen(open)} > diff --git a/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.test.tsx b/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.test.tsx index 159d82992f..785b6189f4 100644 --- a/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.test.tsx +++ b/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.test.tsx @@ -41,6 +41,7 @@ const setup = (props: SetupProps, opts?: SetupOpts) => { setMockResponse.get(urls.userMe.get(), user) const allProps: LearningResourceExpandedProps = { user: user, + chatExpanded: false, shareUrl: `https://learn.mit.edu/search?resource=${resourceId}`, imgConfig: IMG_CONFIG, ...props, @@ -388,16 +389,15 @@ describe.each([true, false])( resource_type: ResourceTypeEnum.PodcastEpisode, }) - const { rerender } = setup({ resource: course1 }) + const { rerender } = setup({ + resource: course1, + chatExpanded: true, + }) await user.click( screen.getByRole("button", { name: "Ask TIM about this course" }), ) - const input = screen.getByRole("textbox") - expect(input).toBeInTheDocument() - await user.type(input, "tell me more{enter}") - - const dataTestId = "ai-chat-screen" + const dataTestId = "ai-chat-entry-screen" expect(screen.getByTestId(dataTestId)).toBeInTheDocument() rerender({ resource: course2 }) expect(screen.getByTestId(dataTestId)).toBeInTheDocument() @@ -406,5 +406,28 @@ describe.each([true, false])( expect(screen.queryByTestId(dataTestId)).toBe(null) }) }) + + test.each([ + { chatExpanded: false, expectChat: false }, + { chatExpanded: true, expectChat: true }, + ])( + "When `chatExpanded=true`, chat button is pressed and interactive", + ({ chatExpanded, expectChat }) => { + if (!enabled) return + const resource = factories.learningResources.resource({ + resource_type: ResourceTypeEnum.Course, + }) + setup({ resource, chatExpanded }) + + screen.getByRole("button", { + name: /Ask\sTIM/, + pressed: chatExpanded, + }) + + // AiChat is always in the dom, but it's hidden and inert when not expanded. + const aiChat = screen.getByTestId("ai-chat-entry-screen") + expect(!!aiChat.closest("[inert]")).toBe(!expectChat) + }, + ) }, ) diff --git a/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.tsx b/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.tsx index 1dc1db6d70..183f7406aa 100644 --- a/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.tsx +++ b/frontends/main/src/page-components/LearningResourceExpanded/LearningResourceExpanded.tsx @@ -4,7 +4,6 @@ import { theme } from "ol-components" import type { ImageConfig, LearningResourceCardProps } from "ol-components" import { ResourceTypeEnum } from "api" import type { LearningResource } from "api" -import { useToggle } from "ol-utilities" import InfoSection from "./InfoSection" import type { User } from "api/hooks/user" import TitleSection from "./TitleSection" @@ -13,6 +12,7 @@ import ResourceDescription from "./ResourceDescription" import { FeatureFlags } from "@/common/feature_flags" import { useFeatureFlagEnabled } from "posthog-js/react" import AiSyllabusBotSlideDown from "./AiChatSyllabusSlideDown" +import { RESOURCE_DRAWER_PARAMS } from "@/common/urls" const DRAWER_WIDTH = "900px" @@ -112,6 +112,7 @@ const TopCarouselContainer = styled.div({ type LearningResourceExpandedProps = { resourceId: number + chatExpanded: boolean titleId?: string resource?: LearningResource user?: User @@ -126,6 +127,25 @@ type LearningResourceExpandedProps = { closeDrawer?: () => void } +const closeChat = () => { + const params = new URLSearchParams(window.location.search) + params.delete(RESOURCE_DRAWER_PARAMS.syllabus) + window.history.replaceState({}, "", `?${params.toString()}`) +} +const openChat = () => { + const params = new URLSearchParams(window.location.search) + params.set(RESOURCE_DRAWER_PARAMS.syllabus, "") + window.history.replaceState({}, "", `?${params.toString()}`) +} +const toggleChat = () => { + const params = new URLSearchParams(window.location.search) + if (params.has(RESOURCE_DRAWER_PARAMS.syllabus)) { + closeChat() + } else { + openChat() + } +} + const LearningResourceExpanded: React.FC = ({ resourceId, resource, @@ -140,12 +160,18 @@ const LearningResourceExpanded: React.FC = ({ onAddToLearningPathClick, onAddToUserListClick, closeDrawer, + chatExpanded, }) => { const chatEnabled = useFeatureFlagEnabled(FeatureFlags.LrDrawerChatbot) && resource?.resource_type === ResourceTypeEnum.Course - const [chatExpanded, setChatExpanded] = useToggle(false) + useEffect(() => { + // If URL indicates syllabus open, but it's not enabled, update URL + if (resource && !chatEnabled) { + closeChat() + } + }, [resource, chatEnabled]) const outerContainerRef = useRef(null) const titleSectionRef = useRef(null) @@ -186,7 +212,8 @@ const LearningResourceExpanded: React.FC = ({ ) : null} diff --git a/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx index 8d65a9dfee..406152d409 100644 --- a/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx +++ b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx @@ -18,7 +18,7 @@ import { } from "../Dialogs/AddToListDialog" import type { ResourceCardProps } from "./ResourceCard" import { urls, factories, setMockResponse } from "api/test-utils" -import { RESOURCE_DRAWER_QUERY_PARAM } from "@/common/urls" +import { RESOURCE_DRAWER_PARAMS } from "@/common/urls" import invariant from "tiny-invariant" import { LearningResourceCard, LearningResourceListCard } from "ol-components" @@ -220,7 +220,7 @@ describe.each([ const href = link.getAttribute("href") invariant(href) const url = new URL(href, window.location.href) - expect(url.searchParams.get(RESOURCE_DRAWER_QUERY_PARAM)).toBe( + expect(url.searchParams.get(RESOURCE_DRAWER_PARAMS.resource)).toBe( String(resource.id), ) }) diff --git a/frontends/main/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index 17d27f01af..063defd903 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -1,7 +1,7 @@ /* eslint-disable import/no-extraneous-dependencies */ import React from "react" import { QueryClientProvider } from "@tanstack/react-query" -import { ThemeProvider } from "@mitodl/smoot-design" +import { ThemeProvider } from "ol-components" import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" import type { QueryClient } from "@tanstack/react-query" @@ -128,7 +128,7 @@ const expectProps = ( */ const expectLastProps = ( // eslint-disable-next-line @typescript-eslint/no-explicit-any - fc: jest.Mock, + fc: (...args: any[]) => void, partialProps: unknown, ) => { expect(fc).toHaveBeenLastCalledWith( diff --git a/frontends/ol-components/src/components/Link/Link.tsx b/frontends/ol-components/src/components/Link/Link.tsx index 9b31b2eb73..cdcdbb1603 100644 --- a/frontends/ol-components/src/components/Link/Link.tsx +++ b/frontends/ol-components/src/components/Link/Link.tsx @@ -1,9 +1,8 @@ import React from "react" import styled from "@emotion/styled" import { css } from "@emotion/react" -import { default as NextLink } from "next/link" import { theme } from "../ThemeProvider/ThemeProvider" -import invariant from "tiny-invariant" +import { LinkAdapter } from "../LinkAdapter/LinkAdapter" type LinkStyleProps = { size?: "small" | "medium" | "large" @@ -19,6 +18,12 @@ const DEFAULT_PROPS: Required = { nohover: false, } +const NO_FORWARD = Object.keys({ + size: false, + color: false, + hovercolor: false, + nohover: false, +} satisfies Record) /** * Generate styles used for the Link component. * @@ -73,38 +78,6 @@ type LinkProps = LinkStyleProps & prefetch?: boolean } -const BaseLink = ({ - href, - shallow, - nohover, - scroll, - onClick, - ...rest -}: LinkProps) => { - if (process.env.NODE_ENV === "development") { - invariant( - !shallow || href?.startsWith("?"), - "Shallow routing should only be used to update search params", - ) - } - return ( - { - e.preventDefault() - window.history.pushState({}, "", href) - } - : undefined) - } - /> - ) -} - /** * A styled link. By default, renders a medium-sized black link using the Link * component from `next/link`. This is appropriate for in-app routing. @@ -114,7 +87,9 @@ const BaseLink = ({ * * For a link styled as a button, use ButtonLink. */ -const Link = styled(BaseLink)(linkStyles) +const Link = styled(LinkAdapter, { + shouldForwardProp: (propName) => !NO_FORWARD.includes(propName), +})(linkStyles) export { Link, linkStyles } export type { LinkProps } diff --git a/frontends/ol-components/src/components/LinkAdapter/LinkAdapter.tsx b/frontends/ol-components/src/components/LinkAdapter/LinkAdapter.tsx new file mode 100644 index 0000000000..6e95f372cf --- /dev/null +++ b/frontends/ol-components/src/components/LinkAdapter/LinkAdapter.tsx @@ -0,0 +1,51 @@ +import React from "react" +import NextLink from "next/link" +import type { LinkProps } from "next/link" +import invariant from "tiny-invariant" + +type LinkAdapterExtraProps = Pick & { + /* + * If true, enables client-side-only routing via window.history.pushState. + * This is ONLY available for query-param updates, e.g., + * `href="?resource=123"`. + * + * This avoids calls to the NextJS server for RSC payloads that can cause + * performance and hydration mismatch issues for example where we are only + * updating the URL search params for modal views within the page, such as the + * resource drawer, and do not want to trigger calls to the server page which + * may re-fetch API data. + */ + shallow?: boolean + // Note: NextJS LinkProps actually does have a `shallow` prop, but at time of + // writing it is only supported by the Pages router, so the docs for it are + // unhelpful. +} + +type LinkAdapterProps = React.ComponentProps<"a"> & LinkAdapterExtraProps + +/** + * Default link implementation used for our smoot-design theme. + */ +const LinkAdapter = ({ shallow, href = "", ...props }: LinkAdapterProps) => { + invariant( + !shallow || href.startsWith("?"), + "shallow links must start with '?'", + ) + return ( + { + if (shallow) { + e.preventDefault() + window.history.pushState({}, "", href) + } else { + props.onClick?.(e) + } + }} + /> + ) +} + +export { LinkAdapter } +export type { LinkAdapterProps, LinkAdapterExtraProps } diff --git a/frontends/ol-components/src/components/RoutedDrawer/RoutedDrawer.tsx b/frontends/ol-components/src/components/RoutedDrawer/RoutedDrawer.tsx index 82b81f9ca8..fd9927a314 100644 --- a/frontends/ol-components/src/components/RoutedDrawer/RoutedDrawer.tsx +++ b/frontends/ol-components/src/components/RoutedDrawer/RoutedDrawer.tsx @@ -28,6 +28,13 @@ type RoutedDrawerProps = { }) => React.ReactNode } & Omit +/** + * Drawer that opens & closes based on the presence of required URL params. + * + * This is particularly useful when the drawer content depends on the URL + * parameters: the drawer handles removing the URL params *after* its closing + * animation. + */ const RoutedDrawer = ( props: RoutedDrawerProps, ) => { diff --git a/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx b/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx index e6475aedab..8fb20be34a 100644 --- a/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx +++ b/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx @@ -5,12 +5,20 @@ import { } from "@mitodl/smoot-design" import type {} from "@mitodl/smoot-design/type-augmentation" import type {} from "@mui/lab/themeAugmentation" -import Link from "next/link" import Image from "next/image" +import { LinkAdapter } from "../LinkAdapter/LinkAdapter" +import type { LinkAdapterExtraProps } from "../LinkAdapter/LinkAdapter" + +declare module "@mitodl/smoot-design" { + // Add extra props to smoot-design's LinkAdapter + // See https://mitodl.github.io/smoot-design/?path=/docs/smoot-design-themeprovider--docs + // eslint-disable-next-line @typescript-eslint/no-empty-object-type + interface LinkAdapterPropsOverrides extends LinkAdapterExtraProps {} +} const theme = createTheme({ custom: { - LinkAdapter: Link, + LinkAdapter, ImgAdapter: Image, }, components: {