From c94534983ffdbbed201f6472fd0d965277ef88b6 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 26 Jun 2024 18:25:29 -0400 Subject: [PATCH 1/5] Add inUserList, inLearningPath to card props --- .../ResourceCard/ResourceCard.tsx | 21 ++++- .../LearningResourceCard.tsx | 50 ++++++++---- .../LearningResourceListCard.tsx | 76 +++++++++++++------ 3 files changed, 105 insertions(+), 42 deletions(-) diff --git a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx index 9636f5380f..94bf59ffbc 100644 --- a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx +++ b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx @@ -12,8 +12,9 @@ import { import { useResourceDrawerHref } from "../LearningResourceDrawer/LearningResourceDrawer" import { useUserMe } from "api/hooks/user" import { SignupPopover } from "../SignupPopover/SignupPopover" +import { LearningResource } from "api" -const useResourceCard = () => { +const useResourceCard = (resource?: LearningResource | null) => { const getDrawerHref = useResourceDrawerHref() const { data: user } = useUserMe() @@ -48,12 +49,17 @@ const useResourceCard = () => { } }, [user]) + const inUserList = !!resource?.user_list_parents?.length + const inLearningPath = !!resource?.learning_path_parents?.length + return { getDrawerHref, anchorEl, handleClosePopover, handleAddToLearningPathClick, handleAddToUserListClick, + inUserList, + inLearningPath, } } @@ -76,7 +82,9 @@ const ResourceCard: React.FC = ({ resource, ...others }) => { handleClosePopover, handleAddToLearningPathClick, handleAddToUserListClick, - } = useResourceCard() + inUserList, + inLearningPath, + } = useResourceCard(resource) return ( <> = ({ resource, ...others }) => { href={resource ? getDrawerHref(resource.id) : undefined} onAddToLearningPathClick={handleAddToLearningPathClick} onAddToUserListClick={handleAddToUserListClick} + inUserList={inUserList} + inLearningPath={inLearningPath} {...others} /> @@ -113,7 +123,10 @@ const ResourceListCard: React.FC = ({ handleClosePopover, handleAddToLearningPathClick, handleAddToUserListClick, - } = useResourceCard() + inUserList, + inLearningPath, + } = useResourceCard(resource) + console.log(inUserList) return ( <> = ({ href={resource ? getDrawerHref(resource.id) : undefined} onAddToLearningPathClick={handleAddToLearningPathClick} onAddToUserListClick={handleAddToUserListClick} + inUserList={inUserList} + inLearningPath={inLearningPath} {...others} /> diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx index b37d9dbf79..fa5904cbcf 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx @@ -1,7 +1,12 @@ import React from "react" import styled from "@emotion/styled" import Skeleton from "@mui/material/Skeleton" -import { RiMenuAddLine, RiBookmarkLine, RiAwardFill } from "@remixicon/react" +import { + RiMenuAddLine, + RiBookmarkLine, + RiBookmarkFill, + RiAwardFill, +} from "@remixicon/react" import { LearningResource, ResourceTypeEnum, PlatformEnum } from "api" import { findBestRun, @@ -13,7 +18,7 @@ import { import { Card } from "../Card/Card" import type { Size } from "../Card/Card" import { TruncateText } from "../TruncateText/TruncateText" -import { ActionButton } from "../Button/Button" +import { ActionButton, ActionButtonProps } from "../Button/Button" import { imgConfigs } from "../../constants/imgConfigs" import { theme } from "../ThemeProvider/ThemeProvider" @@ -119,6 +124,25 @@ interface LearningResourceCardProps { href?: string onAddToLearningPathClick?: ResourceIdCallback | null onAddToUserListClick?: ResourceIdCallback | null + inUserList?: boolean + inLearningPath?: boolean +} + +const FILLED_PROPS = { variant: "primary" } as const +const UNFILLED_PROPS = { color: "secondary", variant: "secondary" } as const +const CardActionButton: React.FC< + Pick & { + filled?: boolean + } +> = ({ filled, ...props }) => { + return ( + + ) } const LearningResourceCard: React.FC = ({ @@ -129,6 +153,8 @@ const LearningResourceCard: React.FC = ({ href, onAddToLearningPathClick, onAddToUserListClick, + inLearningPath, + inUserList, }) => { if (isLoading) { const { width, height } = imgConfigs["column"] @@ -166,28 +192,22 @@ const LearningResourceCard: React.FC = ({ {onAddToLearningPathClick && ( - onAddToLearningPathClick(event, resource.id)} > - + )} {onAddToUserListClick && ( - onAddToUserListClick(event, resource.id)} > - - + {inUserList ? : } + )} diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 33884b704a..15ac403987 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -1,7 +1,12 @@ import React from "react" import styled from "@emotion/styled" import Skeleton from "@mui/material/Skeleton" -import { RiMenuAddLine, RiBookmarkLine, RiAwardFill } from "@remixicon/react" +import { + RiMenuAddLine, + RiBookmarkLine, + RiAwardFill, + RiBookmarkFill, +} from "@remixicon/react" import { LearningResource, ResourceTypeEnum, PlatformEnum } from "api" import { findBestRun, @@ -12,7 +17,7 @@ import { pluralize, } from "ol-utilities" import { ListCard } from "../Card/ListCard" -import { ActionButton } from "../Button/Button" +import { ActionButton, ActionButtonProps } from "../Button/Button" import { theme } from "../ThemeProvider/ThemeProvider" import { useMuiBreakpointAtLeast } from "../../hooks/useBreakpoint" @@ -80,15 +85,6 @@ const BorderSeparator = styled.div` } ` -const StyledActionButton = styled(ActionButton)<{ edge: string }>` - ${({ edge }) => - edge === "none" - ? ` - width: 16px; - height: 16px;` - : ""} -` - type ResourceIdCallback = ( event: React.MouseEvent, resourceId: number, @@ -356,6 +352,40 @@ interface LearningResourceListCardProps { onAddToLearningPathClick?: ResourceIdCallback | null onAddToUserListClick?: ResourceIdCallback | null editMenu?: React.ReactNode | null + inUserList?: boolean + inLearningPath?: boolean +} + +const StyledActionButton = styled(ActionButton)<{ isMobile?: boolean }>( + ({ theme }) => ({ + [theme.breakpoints.down("md")]: { + borderStyle: "none", + width: "24px", + height: "24px", + svg: { + width: "16px", + height: "16px", + }, + }, + }), +) +const FILLED_PROPS = { variant: "primary" } as const +const UNFILLED_PROPS = { color: "secondary", variant: "secondary" } as const +const CardActionButton: React.FC< + Pick & { + filled?: boolean + isMobile?: boolean + } +> = ({ filled, isMobile, ...props }) => { + return ( + + ) } const LearningResourceListCard: React.FC = ({ @@ -366,6 +396,8 @@ const LearningResourceListCard: React.FC = ({ onAddToLearningPathClick, onAddToUserListClick, editMenu, + inLearningPath, + inUserList, }) => { const isMobile = !useMuiBreakpointAtLeast("md") @@ -397,28 +429,24 @@ const LearningResourceListCard: React.FC = ({ {resource.title} {onAddToLearningPathClick && ( - onAddToLearningPathClick(event, resource.id)} > - + )} {onAddToUserListClick && ( - onAddToUserListClick(event, resource.id)} > - - + {inUserList ? : } + )} {editMenu} From 68e62894d64f7231c4a40a3715a50ccad83e65b8 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 26 Jun 2024 18:27:13 -0400 Subject: [PATCH 2/5] update instead of invalidate featured results --- .../src/hooks/learningResources/index.test.ts | 363 ++++++++++++++---- .../api/src/hooks/learningResources/index.ts | 61 ++- .../src/hooks/learningResources/keyFactory.ts | 125 ++++-- 3 files changed, 422 insertions(+), 127 deletions(-) diff --git a/frontends/api/src/hooks/learningResources/index.test.ts b/frontends/api/src/hooks/learningResources/index.test.ts index 4e5939efa7..8e1ab47b58 100644 --- a/frontends/api/src/hooks/learningResources/index.test.ts +++ b/frontends/api/src/hooks/learningResources/index.test.ts @@ -2,7 +2,10 @@ import { renderHook, waitFor } from "@testing-library/react" import { faker } from "@faker-js/faker/locale/en" import { setupReactQueryTest } from "../test-utils" -import keyFactory, { invalidateResourceQueries } from "./keyFactory" +import keyFactory, { + invalidateResourceQueries, + invalidateUserListQueries, +} from "./keyFactory" import { useLearningResourcesDetail, useLearningResourcesList, @@ -16,10 +19,16 @@ import { useLearningpathRelationshipMove, useLearningpathRelationshipCreate, useLearningpathRelationshipDestroy, + useFeaturedLearningResourcesList, + useUserListRelationshipCreate, + useUserListRelationshipDestroy, } from "./index" import { setMockResponse, urls, makeRequest } from "../../test-utils" -import * as factory from "../../test-utils/factories/learningResources" +import * as factories from "../../test-utils/factories" import { UseQueryResult } from "@tanstack/react-query" +import { LearningResource } from "../../generated/v1" + +const factory = factories.learningResources jest.mock("./keyFactory", () => { const actual = jest.requireActual("./keyFactory") @@ -27,6 +36,7 @@ jest.mock("./keyFactory", () => { __esModule: true, ...actual, invalidateResourceQueries: jest.fn(), + invalidateUserListQueries: jest.fn(), } }) @@ -41,7 +51,12 @@ const assertApiCalled = async ( data: unknown, ) => { await waitFor(() => expect(result.current.isLoading).toBe(false)) - expect(makeRequest).toHaveBeenCalledWith(method, url, expect.anything()) + expect( + makeRequest.mock.calls.some((args) => { + // Don't use toHaveBeenCalledWith. It doesn't handle undefined 3rd arg. + return args[0].toUpperCase() === method && args[1] === url + }), + ).toBe(true) expect(result.current.data).toEqual(data) } @@ -55,7 +70,7 @@ describe("useLearningResourcesList", () => { setMockResponse.get(url, data) const useTestHook = () => useLearningResourcesList(params) const { result } = renderHook(useTestHook, { wrapper }) - assertApiCalled(result, url, "GET", data) + await assertApiCalled(result, url, "GET", data) }, ) }) @@ -71,7 +86,7 @@ describe("useLearningResourcesRetrieve", () => { const useTestHook = () => useLearningResourcesDetail(params.id) const { result } = renderHook(useTestHook, { wrapper }) - assertApiCalled(result, url, "GET", data) + await assertApiCalled(result, url, "GET", data) }) }) @@ -87,7 +102,7 @@ describe("useLearningPathsList", () => { const useTestHook = () => useLearningPathsList(params) const { result } = renderHook(useTestHook, { wrapper }) - assertApiCalled(result, url, "GET", data) + await assertApiCalled(result, url, "GET", data) }, ) }) @@ -103,7 +118,7 @@ describe("useLearningPathsRetrieve", () => { const useTestHook = () => useLearningPathsDetail(params.id) const { result } = renderHook(useTestHook, { wrapper }) - assertApiCalled(result, url, "GET", data) + await assertApiCalled(result, url, "GET", data) }) }) @@ -119,7 +134,7 @@ describe("useLearningResourceTopics", () => { const useTestHook = () => useLearningResourceTopics(params) const { result } = renderHook(useTestHook, { wrapper }) - assertApiCalled(result, url, "GET", data) + await assertApiCalled(result, url, "GET", data) }, ) }) @@ -170,7 +185,6 @@ describe("LearningPath CRUD", () => { const relationship = factory.learningPathRelationship({ parent: path.id }) const keys = { learningResources: keyFactory._def, - childResource: keyFactory.detail(relationship.child).queryKey, relationshipListing: keyFactory.learningpaths._ctx.detail(path.id)._ctx .infiniteItems._def, } @@ -185,7 +199,15 @@ describe("LearningPath CRUD", () => { learning_resource_id: path.id, }), } - return { path, relationship, pathUrls, keys } + + const resourceWithoutList: LearningResource = { + ...relationship.resource, + learning_path_parents: + relationship.resource.learning_path_parents?.filter( + (m) => m.id !== relationship.id, + ) ?? null, + } + return { path, relationship, pathUrls, keys, resourceWithoutList } } test("useLearningpathCreate calls correct API", async () => { @@ -262,68 +284,273 @@ describe("LearningPath CRUD", () => { ) }) - test("useLearningpathRelationshipCreate calls correct API and patches child resource cache", async () => { - const { relationship, pathUrls, keys } = makeData() - const url = pathUrls.relationshipList - const requestData = { - child: relationship.child, - parent: relationship.parent, - position: relationship.position, - } - setMockResponse.post(url, relationship) - - const { wrapper, queryClient } = setupReactQueryTest() - const { result } = renderHook(useLearningpathRelationshipCreate, { - wrapper, - }) - result.current.mutate(requestData) - - await waitFor(() => expect(result.current.isSuccess).toBe(true)) - expect(makeRequest).toHaveBeenCalledWith("post", url, requestData) - - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.child, - ) - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.parent, - ) - - // Check patches cached result - expect(queryClient.getQueryData(keys.childResource)).toEqual( - relationship.resource, - ) - }) + test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( + "useLearningpathRelationshipCreate calls correct API and patches featured resources", + async ({ isChildFeatured }) => { + const { relationship, pathUrls, resourceWithoutList } = makeData() + + const featured = factory.resources({ count: 3 }) + if (isChildFeatured) { + featured.results[0] = resourceWithoutList + } + setMockResponse.get(urls.learningResources.featured(), featured) + + const url = pathUrls.relationshipList + const requestData = { + child: relationship.child, + parent: relationship.parent, + position: relationship.position, + } + setMockResponse.post(url, relationship) + + const { wrapper, queryClient } = setupReactQueryTest() + const { result } = renderHook(useLearningpathRelationshipCreate, { + wrapper, + }) + const { result: featuredResult } = renderHook( + useFeaturedLearningResourcesList, + { wrapper }, + ) + await waitFor(() => expect(featuredResult.current.data).toBe(featured)) + + result.current.mutate(requestData) + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(makeRequest).toHaveBeenCalledWith("post", url, requestData) + + expect(invalidateResourceQueries).toHaveBeenCalledWith( + queryClient, + relationship.child, + { skipFeatured: true }, + ) + expect(invalidateResourceQueries).toHaveBeenCalledWith( + queryClient, + relationship.parent, + ) + + // Assert featured API called only once and that the result has been + // patched correctly. When the child is featured, we do NOT want to make + // a new API call to /featured, because the results of that API are randomly + // ordered. + expect( + makeRequest.mock.calls.filter((call) => call[0] === "get").length, + ).toEqual(1) + if (isChildFeatured) { + expect(featuredResult.current.data?.results).toEqual([ + relationship.resource, + ...featured.results.slice(1), + ]) + } else { + expect(featuredResult.current.data).toEqual(featured) + } + }, + ) - test("useLearningpathRelationshipDestroy calls correct API and patches child resource cache", async () => { - const { relationship, pathUrls, keys } = makeData() - const url = pathUrls.relationshipDetails + test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( + "useLearningpathRelationshipDestroy calls correct API and patches child resource cache (isChildFeatured=$isChildFeatured)", + async ({ isChildFeatured }) => { + const { relationship, pathUrls, resourceWithoutList } = makeData() + const url = pathUrls.relationshipDetails + + const featured = factory.resources({ count: 3 }) + if (isChildFeatured) { + featured.results[0] = relationship.resource + } + setMockResponse.get(urls.learningResources.featured(), featured) + + setMockResponse.delete(url, null) + const { wrapper, queryClient } = setupReactQueryTest() + + const { result } = renderHook(useLearningpathRelationshipDestroy, { + wrapper, + }) + const { result: featuredResult } = renderHook( + useFeaturedLearningResourcesList, + { wrapper }, + ) + + await waitFor(() => expect(featuredResult.current.data).toBe(featured)) + result.current.mutate(relationship) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + expect(makeRequest).toHaveBeenCalledWith("delete", url, undefined) + expect(invalidateResourceQueries).toHaveBeenCalledWith( + queryClient, + relationship.child, + { skipFeatured: true }, + ) + expect(invalidateResourceQueries).toHaveBeenCalledWith( + queryClient, + relationship.parent, + ) + + // Assert featured API called only once and that the result has been + // patched correctly. When the child is featured, we do NOT want to make + // a new API call to /featured, because the results of that API are randomly + // ordered. + expect( + makeRequest.mock.calls.filter((call) => call[0] === "get").length, + ).toEqual(1) + if (isChildFeatured) { + expect(featuredResult.current.data?.results).toEqual([ + resourceWithoutList, + ...featured.results.slice(1), + ]) + } else { + expect(featuredResult.current.data).toEqual(featured) + } + }, + ) +}) - setMockResponse.delete(url, null) - const { wrapper, queryClient } = setupReactQueryTest() - queryClient.setQueryData(keys.childResource, relationship.resource) - const { result } = renderHook(useLearningpathRelationshipDestroy, { - wrapper, +describe("userlist CRUD", () => { + const makeData = () => { + const list = factories.userLists.userList() + const relationship = factories.userLists.userListRelationship({ + parent: list.id, }) + const keys = { + learningResources: keyFactory._def, + relationshipListing: keyFactory.userlists._ctx.detail(list.id)._ctx + .infiniteItems._def, + } + const listUrls = { + list: urls.userLists.list(), + details: urls.userLists.details({ id: list.id }), + relationshipList: urls.userLists.resources({ + userlist_id: list.id, + }), + relationshipDetails: urls.userLists.resourceDetails({ + id: relationship.id, + userlist_id: list.id, + }), + } - result.current.mutate(relationship) - await waitFor(() => expect(result.current.isSuccess).toBe(true)) + const resourceWithoutList: LearningResource = { + ...relationship.resource, + user_list_parents: + relationship.resource.user_list_parents?.filter( + (m) => m.id !== relationship.id, + ) ?? null, + } + return { path: list, relationship, listUrls, keys, resourceWithoutList } + } - expect(makeRequest).toHaveBeenCalledWith("delete", url, undefined) - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.child, - ) - expect(invalidateResourceQueries).toHaveBeenCalledWith( - queryClient, - relationship.parent, - ) + test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( + "useUserListRelationshipCreate calls correct API and patches featured resources", + async ({ isChildFeatured }) => { + const { relationship, listUrls, resourceWithoutList } = makeData() + + const featured = factory.resources({ count: 3 }) + if (isChildFeatured) { + featured.results[0] = resourceWithoutList + } + setMockResponse.get(urls.learningResources.featured(), featured) + + const url = listUrls.relationshipList + const requestData = { + child: relationship.child, + parent: relationship.parent, + position: relationship.position, + } + setMockResponse.post(url, relationship) + + const { wrapper, queryClient } = setupReactQueryTest() + const { result } = renderHook(useUserListRelationshipCreate, { + wrapper, + }) + const { result: featuredResult } = renderHook( + useFeaturedLearningResourcesList, + { wrapper }, + ) + await waitFor(() => expect(featuredResult.current.data).toBe(featured)) + + result.current.mutate(requestData) + + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(makeRequest).toHaveBeenCalledWith("post", url, requestData) + + expect(invalidateResourceQueries).toHaveBeenCalledWith( + queryClient, + relationship.child, + { skipFeatured: true }, + ) + expect(invalidateUserListQueries).toHaveBeenCalledWith( + queryClient, + relationship.parent, + ) + + // Assert featured API called only once and that the result has been + // patched correctly. When the child is featured, we do NOT want to make + // a new API call to /featured, because the results of that API are randomly + // ordered. + expect( + makeRequest.mock.calls.filter((call) => call[0] === "get").length, + ).toEqual(1) + if (isChildFeatured) { + expect(featuredResult.current.data?.results).toEqual([ + relationship.resource, + ...featured.results.slice(1), + ]) + } else { + expect(featuredResult.current.data).toEqual(featured) + } + }, + ) - // Patched existing resource - expect(queryClient.getQueryData(keys.childResource)).toEqual({ - ...relationship.resource, - learning_path_parents: [], - }) - }) + test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( + "useUserListRelationshipDestroy calls correct API and patches child resource cache (isChildFeatured=$isChildFeatured)", + async ({ isChildFeatured }) => { + const { relationship, listUrls, resourceWithoutList } = makeData() + const url = listUrls.relationshipDetails + + const featured = factory.resources({ count: 3 }) + if (isChildFeatured) { + featured.results[0] = relationship.resource + } + setMockResponse.get(urls.learningResources.featured(), featured) + + setMockResponse.delete(url, null) + const { wrapper, queryClient } = setupReactQueryTest() + + const { result } = renderHook(useUserListRelationshipDestroy, { + wrapper, + }) + const { result: featuredResult } = renderHook( + useFeaturedLearningResourcesList, + { wrapper }, + ) + + await waitFor(() => expect(featuredResult.current.data).toBe(featured)) + result.current.mutate(relationship) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + + expect(makeRequest).toHaveBeenCalledWith("delete", url, undefined) + expect(invalidateResourceQueries).toHaveBeenCalledWith( + queryClient, + relationship.child, + { skipFeatured: true }, + ) + expect(invalidateUserListQueries).toHaveBeenCalledWith( + queryClient, + relationship.parent, + ) + + // Assert featured API called only once and that the result has been + // patched correctly. When the child is featured, we do NOT want to make + // a new API call to /featured, because the results of that API are randomly + // ordered. + expect( + makeRequest.mock.calls.filter((call) => call[0] === "get").length, + ).toEqual(1) + if (isChildFeatured) { + expect(featuredResult.current.data?.results).toEqual([ + resourceWithoutList, + ...featured.results.slice(1), + ]) + } else { + expect(featuredResult.current.data).toEqual(featured) + } + }, + ) }) diff --git a/frontends/api/src/hooks/learningResources/index.ts b/frontends/api/src/hooks/learningResources/index.ts index dadbc54775..524e0b2d7a 100644 --- a/frontends/api/src/hooks/learningResources/index.ts +++ b/frontends/api/src/hooks/learningResources/index.ts @@ -16,7 +16,6 @@ import type { LearningPathResource, LearningPathRelationshipRequest, MicroLearningPathRelationship, - LearningResource, LearningResourcesSearchApiLearningResourcesSearchRetrieveRequest as LRSearchRequest, UserlistsApiUserlistsListRequest as ULListRequest, UserlistsApiUserlistsCreateRequest as ULCreateRequest, @@ -28,10 +27,13 @@ import type { MicroUserListRelationship, PlatformsApiPlatformsListRequest, FeaturedApiFeaturedListRequest as FeaturedListParams, + PaginatedLearningResourceList, } from "../../generated/v1" import learningResources, { invalidateResourceQueries, invalidateUserListQueries, + updateListParentsOnAdd, + updateListParentsOnDestroy, } from "./keyFactory" import { ListType } from "../../common/constants" @@ -169,13 +171,13 @@ const useLearningpathRelationshipCreate = () => { LearningPathRelationshipRequest: params, }), onSuccess: (response, _vars) => { - queryClient.setQueryData( - learningResources.detail(response.data.child).queryKey, - response.data.resource, + queryClient.setQueriesData( + learningResources.featured({}).queryKey, + (old) => updateListParentsOnAdd(response.data, old), ) }, - onSettled: (response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child) + onSettled: (_response, _err, vars) => { + invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) invalidateResourceQueries(queryClient, vars.parent) }, }) @@ -190,21 +192,17 @@ const useLearningpathRelationshipDestroy = () => { learning_resource_id: params.parent, }), onSuccess: (_response, vars) => { - queryClient.setQueryData( - learningResources.detail(vars.child).queryKey, - (old: LearningResource | undefined) => { - if (!old) return - const parents = - old.learning_path_parents?.filter(({ id }) => vars.id !== id) ?? [] - return { - ...old, - learning_path_parents: parents, - } - }, + queryClient.setQueriesData( + learningResources.featured().queryKey, + (old) => + updateListParentsOnDestroy( + { value: vars, type: "learning_path" }, + old, + ), ) }, onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child) + invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) invalidateResourceQueries(queryClient, vars.parent) }, }) @@ -298,13 +296,13 @@ const useUserListRelationshipCreate = () => { UserListRelationshipRequest: params, }), onSuccess: (response, _vars) => { - queryClient.setQueryData( - learningResources.detail(response.data.child).queryKey, - response.data.resource, + queryClient.setQueriesData( + learningResources.featured({}).queryKey, + (old) => updateListParentsOnAdd(response.data, old), ) }, - onSettled: (response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child) + onSettled: (_response, _err, vars) => { + invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) invalidateUserListQueries(queryClient, vars.parent) }, }) @@ -319,21 +317,14 @@ const useUserListRelationshipDestroy = () => { userlist_id: params.parent, }), onSuccess: (_response, vars) => { - queryClient.setQueryData( - learningResources.detail(vars.child).queryKey, - (old: LearningResource | undefined) => { - if (!old) return - const parents = - old.user_list_parents?.filter(({ id }) => vars.id !== id) ?? [] - return { - ...old, - user_list_parents: parents, - } - }, + queryClient.setQueriesData( + learningResources.featured({}).queryKey, + (old) => + updateListParentsOnDestroy({ value: vars, type: "userlist" }, old), ) }, onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child) + invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) invalidateUserListQueries(queryClient, vars.parent) }, }) diff --git a/frontends/api/src/hooks/learningResources/keyFactory.ts b/frontends/api/src/hooks/learningResources/keyFactory.ts index 8224222542..3306023bc2 100644 --- a/frontends/api/src/hooks/learningResources/keyFactory.ts +++ b/frontends/api/src/hooks/learningResources/keyFactory.ts @@ -27,6 +27,10 @@ import type { OfferorsApiOfferorsListRequest, PlatformsApiPlatformsListRequest, FeaturedApiFeaturedListRequest as FeaturedListParams, + UserListRelationship, + LearningPathRelationship, + MicroLearningPathRelationship, + MicroUserListRelationship, } from "../../generated/v1" import { createQueryKeys } from "@lukemorales/query-key-factory" @@ -45,7 +49,7 @@ const learningResources = createQueryKeys("learningResources", { .learningResourcesList(params) .then((res) => res.data), }), - featured: (params: FeaturedListParams) => ({ + featured: (params: FeaturedListParams = {}) => ({ queryKey: [params], queryFn: () => featuredApi.featuredList(params).then((res) => res.data), }), @@ -143,25 +147,12 @@ const learningResources = createQueryKeys("learningResources", { }, }) -const learningPathHasResource = +const listHasResource = (resourceId: number) => (query: Query): boolean => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const data = query.state.data as any - const resources: LearningResource[] = data.pages - ? data.pages.flatMap( - (page: PaginatedLearningResourceList) => page.results, - ) - : data.results - return resources.some((res) => res.id === resourceId) - } - -const userListHasResource = - (resourceId: number) => - (query: Query): boolean => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const data = query.state.data as any - const resources: UserList[] = data.pages + const resources: LearningResource[] | UserList[] = data.pages ? data.pages.flatMap( (page: PaginatedLearningResourceList) => page.results, ) @@ -169,9 +160,20 @@ const userListHasResource = return resources.some((res) => res.id === resourceId) } +/** + * Invalidate Resource queries that a specific resource appears in. + * + * By default, this will invalidate featured list queries. This can result in + * odd behavior because the featured list item order is randomized: when the + * featured list cache is invalidated, the newly fetched data may be in a + * different order. To maintain the order, use skipFeatured to skip invalidation + * of featured lists and instead manually update the cached data via + * `updateListParentsOnAdd`. + */ const invalidateResourceQueries = ( queryClient: QueryClient, resourceId: LearningResource["id"], + { skipFeatured = false } = {}, ) => { /** * Invalidate details queries. @@ -191,17 +193,13 @@ const invalidateResourceQueries = ( const lists = [ learningResources.list._def, learningResources.learningpaths._ctx.list._def, - learningResources.userlists._ctx.list._def, - learningResources.featured._def, + learningResources.search._def, + ...(skipFeatured ? [] : [learningResources.featured._def]), ] lists.forEach((queryKey) => { queryClient.invalidateQueries({ queryKey, - predicate: learningPathHasResource(resourceId), - }) - queryClient.invalidateQueries({ - queryKey, - predicate: userListHasResource(resourceId), + predicate: listHasResource(resourceId), }) }) } @@ -213,7 +211,86 @@ const invalidateUserListQueries = ( queryClient.invalidateQueries( learningResources.userlists._ctx.detail(userListId).queryKey, ) + const lists = [learningResources.userlists._ctx.list._def] + lists.forEach((queryKey) => { + queryClient.invalidateQueries({ + queryKey, + predicate: listHasResource(userListId), + }) + }) +} + +/** + * Given + * - a list of learning resources L + * - a new relationship between learningpath/userlist and a resource R + * Update the list L so that it includes the updated resource R. (If the list + * did not contain R to begin with, no change is made) + */ +const updateListParentsOnAdd = ( + relationship: LearningPathRelationship | UserListRelationship, + oldList?: PaginatedLearningResourceList, +) => { + if (!oldList) return oldList + const matchIndex = oldList.results.findIndex( + (res) => res.id === relationship.child, + ) + if (matchIndex === -1) return oldList + const updatesResults = [...oldList.results] + updatesResults[matchIndex] = relationship.resource + return { + ...oldList, + results: updatesResults, + } +} + +type WrappedRelationship = + | { + value?: MicroLearningPathRelationship + type: "learning_path" + } + | { + value?: MicroUserListRelationship + type: "userlist" + } +/** + * Given + * - a list of learning resources L + * - a destroyed relationship between learningpath/userlist and a resource R + * Update the list L so that it includes the updated resource R. (If the list + * did not contain R to begin with, no change is made) + */ +const updateListParentsOnDestroy = ( + relationship: WrappedRelationship, + list?: PaginatedLearningResourceList, +) => { + if (!list) return list + const { value } = relationship + if (!value) return list + const matchIndex = list.results.findIndex((res) => res.id === value.child) + if (matchIndex === -1) return list + const updatedResults = [...list.results] + const newResource = { ...updatedResults[matchIndex] } + if (relationship.type === "learning_path") { + newResource.learning_path_parents = + newResource.learning_path_parents?.filter((m) => m.id !== value.id) ?? + null + } + if (relationship.type === "userlist") { + newResource.user_list_parents = + newResource.user_list_parents?.filter((m) => m.id !== value.id) ?? null + } + updatedResults[matchIndex] = newResource + return { + ...list, + results: updatedResults, + } } export default learningResources -export { invalidateResourceQueries, invalidateUserListQueries } +export { + invalidateResourceQueries, + invalidateUserListQueries, + updateListParentsOnAdd, + updateListParentsOnDestroy, +} From ab5fa74e96201928a369fb797b9324e625a19ca8 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 27 Jun 2024 15:32:38 -0400 Subject: [PATCH 3/5] another test, remove console.log --- .../ResourceCard/ResourceCard.test.tsx | 63 ++++++++++++++++++- .../ResourceCard/ResourceCard.tsx | 1 - 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx index 4163040d6c..eab4c6980c 100644 --- a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx +++ b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx @@ -1,6 +1,11 @@ import React from "react" import * as NiceModal from "@ebay/nice-modal-react" -import { renderWithProviders, user, screen } from "../../test-utils" +import { + renderWithProviders, + user, + screen, + expectProps, +} from "../../test-utils" import type { User } from "../../test-utils" import { ResourceCard, ResourceListCard } from "./ResourceCard" import { @@ -11,6 +16,17 @@ import type { ResourceCardProps } from "./ResourceCard" import { urls, factories, setMockResponse } from "api/test-utils" import { RESOURCE_DRAWER_QUERY_PARAM } from "@/common/urls" import invariant from "tiny-invariant" +import { LearningResourceCard, LearningResourceListCard } from "ol-components" + +jest.mock("ol-components", () => { + const actual = jest.requireActual("ol-components") + return { + __esModule: true, + ...actual, + LearningResourceCard: jest.fn(actual.LearningResourceCard), + LearningResourceListCard: jest.fn(actual.LearningResourceListCard), + } +}) jest.mock("@ebay/nice-modal-react", () => { const actual = jest.requireActual("@ebay/nice-modal-react") @@ -24,11 +40,13 @@ jest.mock("@ebay/nice-modal-react", () => { describe.each([ { CardComponent: ResourceCard, + BaseComponent: LearningResourceCard, }, { CardComponent: ResourceListCard, + BaseComponent: LearningResourceListCard, }, -])("$CardComponent", ({ CardComponent }) => { +])("$CardComponent", ({ CardComponent, BaseComponent }) => { const makeResource = factories.learningResources.resource type SetupOptions = { user?: Partial @@ -84,6 +102,47 @@ describe.each([ }, ) + test.each([ + { + userlist: { count: 1, inList: true }, + learningpath: { count: 1, inList: true }, + }, + { + userlist: { count: 0, inList: false }, + learningpath: { count: 1, inList: true }, + }, + { + userlist: { count: 1, inList: true }, + learningpath: { count: 0, inList: false }, + }, + { + userlist: { count: 0, inList: false }, + learningpath: { count: 0, inList: false }, + }, + ])( + "'Add to ...' buttons are filled based on membership in list", + ({ userlist, learningpath }) => { + const resource = makeResource() + const { microLearningPathRelationship } = factories.learningResources + const { microUserListRelationship } = factories.userLists + resource.learning_path_parents = Array.from( + { length: learningpath.count }, + () => microLearningPathRelationship({ child: resource.id }), + ) + resource.user_list_parents = Array.from({ length: userlist.count }, () => + microUserListRelationship({ child: resource.id }), + ) + + setup({ user: { is_authenticated: true }, props: { resource } }) + + expectProps(BaseComponent, { + resource, + inLearningPath: learningpath.inList, + inUserList: userlist.inList, + }) + }, + ) + test("Clicking add to list button opens AddToListDialog when authenticated", async () => { const showModal = jest.mocked(NiceModal.show) diff --git a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx index 94bf59ffbc..ce14cb591f 100644 --- a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx +++ b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx @@ -126,7 +126,6 @@ const ResourceListCard: React.FC = ({ inUserList, inLearningPath, } = useResourceCard(resource) - console.log(inUserList) return ( <> Date: Thu, 27 Jun 2024 16:00:49 -0400 Subject: [PATCH 4/5] DO invalidate featured lists when learningpath membership changes --- .../src/hooks/learningResources/index.test.ts | 38 +------------ .../api/src/hooks/learningResources/index.ts | 57 ++++++++++++------- .../src/hooks/learningResources/keyFactory.ts | 34 +++-------- 3 files changed, 47 insertions(+), 82 deletions(-) diff --git a/frontends/api/src/hooks/learningResources/index.test.ts b/frontends/api/src/hooks/learningResources/index.test.ts index 8e1ab47b58..f9785a3266 100644 --- a/frontends/api/src/hooks/learningResources/index.test.ts +++ b/frontends/api/src/hooks/learningResources/index.test.ts @@ -321,35 +321,19 @@ describe("LearningPath CRUD", () => { expect(invalidateResourceQueries).toHaveBeenCalledWith( queryClient, relationship.child, - { skipFeatured: true }, + { skipFeatured: false }, ) expect(invalidateResourceQueries).toHaveBeenCalledWith( queryClient, relationship.parent, ) - - // Assert featured API called only once and that the result has been - // patched correctly. When the child is featured, we do NOT want to make - // a new API call to /featured, because the results of that API are randomly - // ordered. - expect( - makeRequest.mock.calls.filter((call) => call[0] === "get").length, - ).toEqual(1) - if (isChildFeatured) { - expect(featuredResult.current.data?.results).toEqual([ - relationship.resource, - ...featured.results.slice(1), - ]) - } else { - expect(featuredResult.current.data).toEqual(featured) - } }, ) test.each([{ isChildFeatured: false }, { isChildFeatured: true }])( "useLearningpathRelationshipDestroy calls correct API and patches child resource cache (isChildFeatured=$isChildFeatured)", async ({ isChildFeatured }) => { - const { relationship, pathUrls, resourceWithoutList } = makeData() + const { relationship, pathUrls } = makeData() const url = pathUrls.relationshipDetails const featured = factory.resources({ count: 3 }) @@ -377,28 +361,12 @@ describe("LearningPath CRUD", () => { expect(invalidateResourceQueries).toHaveBeenCalledWith( queryClient, relationship.child, - { skipFeatured: true }, + { skipFeatured: false }, ) expect(invalidateResourceQueries).toHaveBeenCalledWith( queryClient, relationship.parent, ) - - // Assert featured API called only once and that the result has been - // patched correctly. When the child is featured, we do NOT want to make - // a new API call to /featured, because the results of that API are randomly - // ordered. - expect( - makeRequest.mock.calls.filter((call) => call[0] === "get").length, - ).toEqual(1) - if (isChildFeatured) { - expect(featuredResult.current.data?.results).toEqual([ - resourceWithoutList, - ...featured.results.slice(1), - ]) - } else { - expect(featuredResult.current.data).toEqual(featured) - } }, ) }) diff --git a/frontends/api/src/hooks/learningResources/index.ts b/frontends/api/src/hooks/learningResources/index.ts index 524e0b2d7a..a97bc4509c 100644 --- a/frontends/api/src/hooks/learningResources/index.ts +++ b/frontends/api/src/hooks/learningResources/index.ts @@ -170,14 +170,15 @@ const useLearningpathRelationshipCreate = () => { learning_resource_id: params.parent, LearningPathRelationshipRequest: params, }), - onSuccess: (response, _vars) => { - queryClient.setQueriesData( - learningResources.featured({}).queryKey, - (old) => updateListParentsOnAdd(response.data, old), - ) - }, onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) + invalidateResourceQueries( + queryClient, + vars.child, + // do NOT skip invalidating the /featured/ lists, + // Changing a learning path might change the members of the featured + // lists. + { skipFeatured: false }, + ) invalidateResourceQueries(queryClient, vars.parent) }, }) @@ -191,18 +192,15 @@ const useLearningpathRelationshipDestroy = () => { id: params.id, learning_resource_id: params.parent, }), - onSuccess: (_response, vars) => { - queryClient.setQueriesData( - learningResources.featured().queryKey, - (old) => - updateListParentsOnDestroy( - { value: vars, type: "learning_path" }, - old, - ), - ) - }, onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) + invalidateResourceQueries( + queryClient, + vars.child, + // do NOT skip invalidating the /featured/ lists, + // Changing a learning path might change the members of the featured + // lists. + { skipFeatured: false }, + ) invalidateResourceQueries(queryClient, vars.parent) }, }) @@ -302,7 +300,15 @@ const useUserListRelationshipCreate = () => { ) }, onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) + invalidateResourceQueries( + queryClient, + vars.child, + // Do NOT invalidate the featured lists. Re-fetching the featured list + // data will cause the order to change, since the /featured API returns + // at random order. + // Instead, `onSuccess` hook will manually update the data. + { skipFeatured: true }, + ) invalidateUserListQueries(queryClient, vars.parent) }, }) @@ -319,12 +325,19 @@ const useUserListRelationshipDestroy = () => { onSuccess: (_response, vars) => { queryClient.setQueriesData( learningResources.featured({}).queryKey, - (old) => - updateListParentsOnDestroy({ value: vars, type: "userlist" }, old), + (old) => updateListParentsOnDestroy(vars, old), ) }, onSettled: (_response, _err, vars) => { - invalidateResourceQueries(queryClient, vars.child, { skipFeatured: true }) + invalidateResourceQueries( + queryClient, + vars.child, + // Do NOT invalidate the featured lists. Re-fetching the featured list + // data will cause the order to change, since the /featured API returns + // at random order. + // Instead, `onSuccess` hook will manually update the data. + { skipFeatured: true }, + ) invalidateUserListQueries(queryClient, vars.parent) }, }) diff --git a/frontends/api/src/hooks/learningResources/keyFactory.ts b/frontends/api/src/hooks/learningResources/keyFactory.ts index 3306023bc2..774eb17f3e 100644 --- a/frontends/api/src/hooks/learningResources/keyFactory.ts +++ b/frontends/api/src/hooks/learningResources/keyFactory.ts @@ -28,8 +28,6 @@ import type { PlatformsApiPlatformsListRequest, FeaturedApiFeaturedListRequest as FeaturedListParams, UserListRelationship, - LearningPathRelationship, - MicroLearningPathRelationship, MicroUserListRelationship, } from "../../generated/v1" import { createQueryKeys } from "@lukemorales/query-key-factory" @@ -228,7 +226,7 @@ const invalidateUserListQueries = ( * did not contain R to begin with, no change is made) */ const updateListParentsOnAdd = ( - relationship: LearningPathRelationship | UserListRelationship, + relationship: UserListRelationship, oldList?: PaginatedLearningResourceList, ) => { if (!oldList) return oldList @@ -244,15 +242,6 @@ const updateListParentsOnAdd = ( } } -type WrappedRelationship = - | { - value?: MicroLearningPathRelationship - type: "learning_path" - } - | { - value?: MicroUserListRelationship - type: "userlist" - } /** * Given * - a list of learning resources L @@ -261,25 +250,20 @@ type WrappedRelationship = * did not contain R to begin with, no change is made) */ const updateListParentsOnDestroy = ( - relationship: WrappedRelationship, + relationship: MicroUserListRelationship, list?: PaginatedLearningResourceList, ) => { if (!list) return list - const { value } = relationship - if (!value) return list - const matchIndex = list.results.findIndex((res) => res.id === value.child) + if (!relationship) return list + const matchIndex = list.results.findIndex( + (res) => res.id === relationship.child, + ) if (matchIndex === -1) return list const updatedResults = [...list.results] const newResource = { ...updatedResults[matchIndex] } - if (relationship.type === "learning_path") { - newResource.learning_path_parents = - newResource.learning_path_parents?.filter((m) => m.id !== value.id) ?? - null - } - if (relationship.type === "userlist") { - newResource.user_list_parents = - newResource.user_list_parents?.filter((m) => m.id !== value.id) ?? null - } + newResource.user_list_parents = + newResource.user_list_parents?.filter((m) => m.id !== relationship.id) ?? + null updatedResults[matchIndex] = newResource return { ...list, From 16735b5508483c43953b92979bd697a44752deff Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 28 Jun 2024 15:11:14 -0400 Subject: [PATCH 5/5] position card action buttons a bit better --- .../LearningPathListingPage.tsx | 16 +++--------- .../src/components/Card/ListCard.tsx | 26 +++++++++++++++++-- .../LearningResourceListCard.tsx | 20 ++------------ frontends/ol-components/src/index.ts | 1 + 4 files changed, 31 insertions(+), 32 deletions(-) diff --git a/frontends/mit-open/src/pages/LearningPathListingPage/LearningPathListingPage.tsx b/frontends/mit-open/src/pages/LearningPathListingPage/LearningPathListingPage.tsx index 5802a15c5e..4c2289c71e 100644 --- a/frontends/mit-open/src/pages/LearningPathListingPage/LearningPathListingPage.tsx +++ b/frontends/mit-open/src/pages/LearningPathListingPage/LearningPathListingPage.tsx @@ -2,7 +2,6 @@ import React, { useCallback, useMemo } from "react" import { Button, SimpleMenu, - ActionButton, Grid, LoadingSpinner, BannerPage, @@ -11,7 +10,7 @@ import { Typography, PlainList, LearningResourceListCard, - theme, + ListCardActionButton, } from "ol-components" import type { SimpleMenuItem } from "ol-components" import EditIcon from "@mui/icons-material/Edit" @@ -33,13 +32,6 @@ const ListHeaderGrid = styled(Grid)` margin-bottom: 1rem; ` -const StyledActionButton = styled(ActionButton)` - ${theme.breakpoints.down("md")} { - width: 16px; - height: 16px; - } -` - const EditListMenu: React.FC<{ resource: LearningPathResource }> = ({ resource, }) => { @@ -63,15 +55,15 @@ const EditListMenu: React.FC<{ resource: LearningPathResource }> = ({ return ( - + } items={items} /> diff --git a/frontends/ol-components/src/components/Card/ListCard.tsx b/frontends/ol-components/src/components/Card/ListCard.tsx index cf92ec8736..4650a944fb 100644 --- a/frontends/ol-components/src/components/Card/ListCard.tsx +++ b/frontends/ol-components/src/components/Card/ListCard.tsx @@ -10,6 +10,7 @@ import { theme } from "../ThemeProvider/ThemeProvider" import { Link } from "react-router-dom" import { Wrapper, containerStyles } from "./Card" import { TruncateText } from "../TruncateText/TruncateText" +import { ActionButton, ActionButtonProps } from "../Button/Button" const LinkContainer = styled(Link)` ${containerStyles} @@ -114,6 +115,9 @@ const Bottom = styled.div` } ` +/** + * Slot intended to contain ListCardAction buttons. + */ const Actions = styled.div<{ hasImage?: boolean }>` display: flex; gap: 8px; @@ -121,13 +125,28 @@ const Actions = styled.div<{ hasImage?: boolean }>` bottom: 24px; right: ${({ hasImage }) => (hasImage ? "284px" : "24px")}; ${theme.breakpoints.down("md")} { - bottom: 12px; - right: ${({ hasImage }) => (hasImage ? "124px" : "12px")}; + bottom: 8px; + gap: 4px; + right: ${({ hasImage }) => (hasImage ? "120px" : "8px")}; } background-color: ${theme.custom.colors.white}; ` +const ListCardActionButton = styled(ActionButton)<{ isMobile?: boolean }>( + ({ theme }) => ({ + [theme.breakpoints.down("md")]: { + borderStyle: "none", + width: "24px", + height: "24px", + svg: { + width: "16px", + height: "16px", + }, + }, + }), +) + type CardProps = { children: ReactNode[] | ReactNode className?: string @@ -140,6 +159,7 @@ type Card = FC & { Title: FC<{ children: ReactNode }> Footer: FC<{ children: ReactNode }> Actions: FC<{ children: ReactNode }> + Action: FC } const ListCard: Card = ({ children, className, href }) => { @@ -192,5 +212,7 @@ ListCard.Info = Info ListCard.Title = Title ListCard.Footer = Footer ListCard.Actions = Actions +ListCard.Action = ListCardActionButton export { ListCard } +export { ListCardActionButton } diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 15ac403987..c16a0a09ea 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -17,7 +17,7 @@ import { pluralize, } from "ol-utilities" import { ListCard } from "../Card/ListCard" -import { ActionButton, ActionButtonProps } from "../Button/Button" +import { ActionButtonProps } from "../Button/Button" import { theme } from "../ThemeProvider/ThemeProvider" import { useMuiBreakpointAtLeast } from "../../hooks/useBreakpoint" @@ -356,19 +356,6 @@ interface LearningResourceListCardProps { inLearningPath?: boolean } -const StyledActionButton = styled(ActionButton)<{ isMobile?: boolean }>( - ({ theme }) => ({ - [theme.breakpoints.down("md")]: { - borderStyle: "none", - width: "24px", - height: "24px", - svg: { - width: "16px", - height: "16px", - }, - }, - }), -) const FILLED_PROPS = { variant: "primary" } as const const UNFILLED_PROPS = { color: "secondary", variant: "secondary" } as const const CardActionButton: React.FC< @@ -378,10 +365,9 @@ const CardActionButton: React.FC< } > = ({ filled, isMobile, ...props }) => { return ( - @@ -431,7 +417,6 @@ const LearningResourceListCard: React.FC = ({ {onAddToLearningPathClick && ( onAddToLearningPathClick(event, resource.id)} > @@ -441,7 +426,6 @@ const LearningResourceListCard: React.FC = ({ {onAddToUserListClick && ( onAddToUserListClick(event, resource.id)} > diff --git a/frontends/ol-components/src/index.ts b/frontends/ol-components/src/index.ts index f8107a07b7..a4a8fc45d2 100644 --- a/frontends/ol-components/src/index.ts +++ b/frontends/ol-components/src/index.ts @@ -28,6 +28,7 @@ export { ActionButtonLink, ButtonLink, } from "./components/Button/Button" +export { ListCardActionButton } from "./components/Card/ListCard" export type { ButtonProps,