diff --git a/frontends/api/src/hooks/learningResources/index.ts b/frontends/api/src/hooks/learningResources/index.ts index a97bc4509c..13b930754a 100644 --- a/frontends/api/src/hooks/learningResources/index.ts +++ b/frontends/api/src/hooks/learningResources/index.ts @@ -427,6 +427,13 @@ const useSchoolsList = () => { return useQuery(learningResources.schools()) } +/* + * Not intended to be imported except for special cases. + * It's used in the ResourceCarousel to dynamically build a single useQueries hook + * from config because a React component cannot conditionally call hooks during renders. + */ +export { default as learningResourcesKeyFactory } from "./keyFactory" + export { useLearningResourcesList, useFeaturedLearningResourcesList, diff --git a/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx b/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx index 358f21718d..ed54fb90f0 100644 --- a/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx +++ b/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx @@ -83,19 +83,13 @@ describe("ResourceCarousel", () => { }, ] - const { resources, resolve } = setupApis({ autoResolve: false }) + const { resources } = setupApis({ autoResolve: true }) renderWithProviders( , ) - expectLastProps(spyLearningResourceCard, { - isLoading: true, - ...cardProps, - }) - resolve() - - const tabs = screen.getAllByRole("tab") + const tabs = await screen.findAllByRole("tab") expect(tabs).toHaveLength(2) expect(tabs[0]).toHaveTextContent("Resources") @@ -178,7 +172,7 @@ describe("ResourceCarousel", () => { expect(urlParams.get("professional")).toEqual("true") }) - it("Shows the correct title", () => { + it("Shows the correct title", async () => { const config: ResourceCarouselProps["config"] = [ { label: "Resources", @@ -193,8 +187,7 @@ describe("ResourceCarousel", () => { renderWithProviders( , ) - expect( - screen.getByRole("heading", { name: "My Favorite Carousel" }), - ).toBeInTheDocument() + + await screen.findByRole("heading", { name: "My Favorite Carousel" }) }) }) diff --git a/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx b/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx index b88abefe26..feac14bc43 100644 --- a/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx +++ b/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx @@ -1,9 +1,5 @@ import React from "react" -import { - useFeaturedLearningResourcesList, - useLearningResourcesList, - useLearningResourcesSearch, -} from "api/hooks/learningResources" +import { learningResourcesKeyFactory } from "api/hooks/learningResources" import { Carousel, TabButton, @@ -13,14 +9,14 @@ import { styled, Typography, } from "ol-components" -import type { - TabConfig, - ResourceDataSource, - SearchDataSource, - FeaturedDataSource, -} from "./types" -import { LearningResource } from "api" +import type { TabConfig } from "./types" +import { LearningResource, PaginatedLearningResourceList } from "api" import { ResourceCard } from "../ResourceCard/ResourceCard" +import { + useQueries, + UseQueryResult, + UseQueryOptions, +} from "@tanstack/react-query" const StyledCarousel = styled(Carousel)({ /** @@ -40,107 +36,6 @@ const StyledCarousel = styled(Carousel)({ }, }) -type DataPanelProps = { - dataConfig: T - isLoading?: boolean - children: ({ - resources, - childrenLoading, - }: { - resources: LearningResource[] - childrenLoading: boolean - }) => React.ReactNode -} - -type LoadTabButtonProps = { - config: FeaturedDataSource - label: React.ReactNode - key: number - value: string -} - -const ResourcesData: React.FC> = ({ - dataConfig, - children, -}) => { - const { data, isLoading } = useLearningResourcesList(dataConfig.params) - return children({ - resources: data?.results ?? [], - childrenLoading: isLoading, - }) -} - -const SearchData: React.FC> = ({ - dataConfig, - children, -}) => { - const { data, isLoading } = useLearningResourcesSearch(dataConfig.params) - return children({ - resources: data?.results ?? [], - childrenLoading: isLoading, - }) -} - -const FeaturedData: React.FC> = ({ - dataConfig, - children, -}) => { - const { data, isLoading } = useFeaturedLearningResourcesList( - dataConfig.params, - ) - return children({ - resources: data?.results ?? [], - childrenLoading: isLoading, - }) -} - -/** - * A wrapper to load data based `TabConfig.data`. - * - * For each `TabConfig.data.type`, a different API endpoint, and hence - * react-query hook, is used. Since hooks can't be called conditionally within - * a single component, each type of data is handled in a separate component. - */ -const DataPanel: React.FC = ({ - dataConfig, - isLoading, - children, -}) => { - if (!isLoading) { - switch (dataConfig.type) { - case "resources": - return {children} - case "lr_search": - return {children} - case "lr_featured": - return {children} - default: - // @ts-expect-error This will always be an error if the switch statement - // is exhaustive since dataConfig will have type `never` - throw new Error(`Unknown data type: ${dataConfig.type}`) - } - } else - return children({ - resources: [], - childrenLoading: true, - }) -} - -/** - * Tab button that loads the resource, so we can determine if it needs to be - * displayed or not. This shouldn't cause double-loading since React Query - * should only run the thing once - when you switch into the tab, the data - * should already be in the cache. - */ - -const LoadFeaturedTabButton: React.FC = (props) => { - const { data, isLoading } = useFeaturedLearningResourcesList( - props.config.params, - ) - - return !isLoading && data && data.count > 0 ? : null -} - const HeaderRow = styled.div(({ theme }) => ({ display: "flex", flexWrap: "wrap", @@ -195,48 +90,46 @@ const TabsList = styled(TabButtonList)({ type ContentProps = { resources: LearningResource[] - isLoading?: boolean + childrenLoading?: boolean tabConfig: TabConfig } type PanelChildrenProps = { config: TabConfig[] + queries: UseQueryResult[] children: (props: ContentProps) => React.ReactNode - isLoading?: boolean } const PanelChildren: React.FC = ({ config, + queries, children, - isLoading, }) => { if (config.length === 1) { - return ( - - {({ resources, childrenLoading }) => - children({ - resources, - isLoading: childrenLoading || isLoading, - tabConfig: config[0], - }) - } - - ) + const { data, isLoading } = queries[0] + const resources = data?.results ?? [] + + return children({ + resources, + childrenLoading: isLoading, + tabConfig: config[0], + }) } return ( <> - {config.map((tabConfig, index) => ( - - - {({ resources, childrenLoading }) => - children({ - resources, - isLoading: childrenLoading || isLoading, - tabConfig, - }) - } - - - ))} + {config.map((tabConfig, index) => { + const { data, isLoading } = queries[index] + const resources = data?.results ?? [] + + return ( + + {children({ + resources, + childrenLoading: isLoading, + tabConfig, + })} + + ) + })} ) } @@ -258,6 +151,7 @@ type ResourceCarouselProps = { title: string className?: string isLoading?: boolean + "data-testid"?: string } /** * A tabbed carousel that fetches resources based on the configuration provided. @@ -275,12 +169,45 @@ const ResourceCarousel: React.FC = ({ title, className, isLoading, + "data-testid": dataTestId, }) => { const [tab, setTab] = React.useState("0") const [ref, setRef] = React.useState(null) + const queries = useQueries({ + queries: config.map( + ( + tab, + ): UseQueryOptions< + PaginatedLearningResourceList, + unknown, + unknown, + // The factory-generated types for queryKeys are very specific (tuples not arrays) + // and assignable to the loose QueryKey (readonly unknown[]) on the UseQueryOptions generic. + // But! as a queryFn arg the more specific QueryKey cannot be assigned to the looser QueryKey. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + any + > => { + switch (tab.data.type) { + case "resources": + return learningResourcesKeyFactory.list(tab.data.params) + case "lr_search": + return learningResourcesKeyFactory.search(tab.data.params) + case "lr_featured": + return learningResourcesKeyFactory.featured(tab.data.params) + } + }, + ), + }) + + const allChildrenLoaded = queries.every(({ isLoading }) => !isLoading) + const allChildrenEmpty = queries.every(({ data }) => !data?.count) + if (!isLoading && allChildrenLoaded && allChildrenEmpty) { + return null + } + return ( - + {title} @@ -288,29 +215,32 @@ const ResourceCarousel: React.FC = ({ {config.length > 1 ? ( setTab(newValue)}> - {config.map((tabConfig, index) => - tabConfig.data.type === "lr_featured" ? ( - - ) : ( + {config.map((tabConfig, index) => { + if ( + !isLoading && + !queries[index].isLoading && + !queries[index].data?.count + ) { + return null + } + return ( - ), - )} + ) + })} ) : null} - - {({ resources, isLoading: childrenLoading, tabConfig }) => ( + []} + > + {({ resources, childrenLoading, tabConfig }) => ( {isLoading || childrenLoading ? Array.from({ length: 6 }).map((_, index) => ( diff --git a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx index 0797f67eb4..55c0606839 100644 --- a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx +++ b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx @@ -35,7 +35,7 @@ const setupApis = ( ) setMockResponse.get( expect.stringContaining(urls.learningResources.featured()), - factories.learningResources.resources({ count: 0 }), + factories.learningResources.resources({ count: 10 }), ) const urlParams = new URLSearchParams(channelPatch?.search_filter) @@ -132,13 +132,14 @@ describe("ChannelPage", () => { undefined, ) }) - expect( - makeRequest.mock.calls.filter(([method, url]) => { - return ( - method === "get" && url.includes(urls.learningResources.featured()) - ) - }).length, - ).toBe(1) + + await waitFor(() => { + expect(makeRequest).toHaveBeenCalledWith( + "get", + urls.learningResources.featured({ limit: 12 }), + undefined, + ) + }) }) it("Does not display a featured carousel if the channel type is not 'unit'", async () => { const { channel } = setupApis({ diff --git a/frontends/mit-open/src/pages/DashboardPage/Dashboard.test.tsx b/frontends/mit-open/src/pages/DashboardPage/Dashboard.test.tsx index 099bd581da..0e6b9ff33e 100644 --- a/frontends/mit-open/src/pages/DashboardPage/Dashboard.test.tsx +++ b/frontends/mit-open/src/pages/DashboardPage/Dashboard.test.tsx @@ -26,7 +26,7 @@ describe("DashboardPage", () => { suggestions: [], aggregations: {}, }, - count: 0, + count: results.length, results: results, next: null, previous: null, @@ -60,6 +60,7 @@ describe("DashboardPage", () => { ) const topPicks = factories.learningResources.courses({ count: 10 }) + topPicks.results.forEach((course) => { course.topics = topics?.map((topic) => factories.learningResources.topic({ name: topic }), @@ -71,6 +72,7 @@ describe("DashboardPage", () => { : { code: "online", name: "Online" }, ) }) + const topicsCourses: CourseResource[] = [] topics?.forEach((topic) => { const topicCourses = factories.learningResources.courses({ count: 10 }) @@ -267,13 +269,12 @@ describe("DashboardPage", () => { const topPicksTitle = await screen.findByText("Top picks for you") expect(topPicksTitle).toBeInTheDocument() - const topPicksCarousel = await screen.findByTestId("top-picks-carousel") - topPicks.results.forEach(async (course) => { - const courseTitle = await within(topPicksCarousel).findByText( - course.title, - ) - expect(courseTitle).toBeInTheDocument() - }) + + await Promise.all( + topPicks.results.map((course) => { + return screen.findAllByText(course.title) + }), + ) profile.preference_search_filters.topic?.forEach(async (topic) => { const topicTitle = await screen.findByText(`Popular courses in ${topic}`) diff --git a/frontends/mit-open/src/pages/DashboardPage/DashboardPage.tsx b/frontends/mit-open/src/pages/DashboardPage/DashboardPage.tsx index 3efd076e6b..a8917dc3bb 100644 --- a/frontends/mit-open/src/pages/DashboardPage/DashboardPage.tsx +++ b/frontends/mit-open/src/pages/DashboardPage/DashboardPage.tsx @@ -25,14 +25,14 @@ import { useUserMe } from "api/hooks/user" import { useLocation } from "react-router" import { UserListListingComponent } from "../UserListListingPage/UserListListingPage" import { UserList } from "api" -import { ListDetailsComponent } from "../ListDetailsPage/ListDetailsPage" -import { ListType } from "api/constants" -import { manageListDialogs } from "@/page-components/ManageListDialogs/ManageListDialogs" -import { ProfileEditForm } from "./ProfileEditForm" import { useInfiniteUserListItems, useUserListsDetail, } from "api/hooks/learningResources" +import { ListDetailsComponent } from "../ListDetailsPage/ListDetailsPage" +import { ListType } from "api/constants" +import { manageListDialogs } from "@/page-components/ManageListDialogs/ManageListDialogs" +import { ProfileEditForm } from "./ProfileEditForm" import { useProfileMeQuery } from "api/hooks/profile" import { TopPicksCarouselConfig, @@ -248,7 +248,7 @@ const HomeHeaderRight = styled.div(({ theme }) => ({ }, })) -const CarouselContainer = styled.div(({ theme }) => ({ +const StyledResourceCarousel = styled(ResourceCarousel)(({ theme }) => ({ padding: "40px 0", [theme.breakpoints.down("md")]: { padding: "16px 0", @@ -442,54 +442,46 @@ const DashboardPage: React.FC = () => { - - + {topics?.map((topic, index) => ( + - - {topics?.map((topic) => ( - - - + /> ))} {certification === true ? ( - - - - ) : ( - - - - )} - - - - - - + )} + + {userListAction === "list" ? ( diff --git a/frontends/mit-open/src/pages/HomePage/HomePage.tsx b/frontends/mit-open/src/pages/HomePage/HomePage.tsx index dfb8c23fcf..1ec4254b69 100644 --- a/frontends/mit-open/src/pages/HomePage/HomePage.tsx +++ b/frontends/mit-open/src/pages/HomePage/HomePage.tsx @@ -26,6 +26,7 @@ const FeaturedCoursesCarousel = styled(ResourceCarousel)(({ theme }) => ({ marginTop: "0px", }, })) + const MediaCarousel = styled(ResourceCarousel)(({ theme }) => ({ margin: "80px 0", minHeight: "388px", diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx index 817c8503d0..c8b0aa4b33 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx @@ -110,7 +110,7 @@ const StartDate: React.FC<{ resource: LearningResource; size?: Size }> = ({ resource, size, }) => { - const startDate = getStartDate(resource) + const startDate = getStartDate(resource, size) if (!startDate) return null