From 5622fffc548dae6a3a8a4298bf8d382713558451 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Mon, 17 Jun 2024 15:40:32 -0400 Subject: [PATCH 1/7] Add popover component --- frontends/ol-components/package.json | 1 + .../components/Popover/Popover.stories.tsx | 99 +++++++++ .../src/components/Popover/Popover.test.tsx | 130 ++++++++++++ .../src/components/Popover/Popover.tsx | 198 ++++++++++++++++++ frontends/ol-components/src/index.ts | 1 + yarn.lock | 1 + 6 files changed, 430 insertions(+) create mode 100644 frontends/ol-components/src/components/Popover/Popover.stories.tsx create mode 100644 frontends/ol-components/src/components/Popover/Popover.test.tsx create mode 100644 frontends/ol-components/src/components/Popover/Popover.tsx diff --git a/frontends/ol-components/package.json b/frontends/ol-components/package.json index 5c911101f0..4e9319a0c2 100644 --- a/frontends/ol-components/package.json +++ b/frontends/ol-components/package.json @@ -12,6 +12,7 @@ "@dnd-kit/utilities": "^3.2.1", "@emotion/react": "^11.11.1", "@emotion/styled": "^11.11.0", + "@mui/base": "5.0.0-beta.40", "@mui/icons-material": "^5.15.15", "@mui/lab": "^5.0.0-alpha.170", "@mui/material": "^5.15.15", diff --git a/frontends/ol-components/src/components/Popover/Popover.stories.tsx b/frontends/ol-components/src/components/Popover/Popover.stories.tsx new file mode 100644 index 0000000000..43e36bcd14 --- /dev/null +++ b/frontends/ol-components/src/components/Popover/Popover.stories.tsx @@ -0,0 +1,99 @@ +/* eslint-disable react-hooks/rules-of-hooks */ +import React from "react" +import type { Meta, StoryObj } from "@storybook/react" +import { Popover } from "./Popover" +import type { PopoverProps } from "./Popover" +import { Button } from "../Button/Button" +import Typography from "@mui/material/Typography" +import styled from "@emotion/styled" + +const ScrollWrapper = styled.div({ + width: "250px", + height: "250px", + overflow: "auto", + border: "1pt solid blue", +}) +const Wrapper = styled.div({ + width: "500px", + height: "500px", + display: "flex", + justifyContent: "center", + alignItems: "center", + border: "1pt solid red", +}) + +const StoryPopover = (args: PopoverProps) => { + const [anchorEl, setAnchorEl] = React.useState(null) + return ( + + setAnchorEl(null)} + /> + + + + ) +} + +const meta: Meta = { + render: (args) => { + return + }, + title: "smoot-design/Popover", + argTypes: { + placement: { + control: { type: "select" }, + options: [undefined, "top", "right", "bottom", "left"], + }, + modal: { + control: { type: "select" }, + options: [undefined, false, true], + }, + children: { + table: { disable: true }, + }, + }, + args: { + children: ( + <> + Popover Content! + + + + ), + }, +} + +export default meta + +type Story = StoryObj + +export const Basic: Story = {} + +export const Scrolling: Story = { + render: (args) => { + return ( + + + + ) + }, +} + +export const Focus: Story = { + render: (args) => { + return ( + <> + + + + ) + }, +} diff --git a/frontends/ol-components/src/components/Popover/Popover.test.tsx b/frontends/ol-components/src/components/Popover/Popover.test.tsx new file mode 100644 index 0000000000..f80b628c36 --- /dev/null +++ b/frontends/ol-components/src/components/Popover/Popover.test.tsx @@ -0,0 +1,130 @@ +import React from "react" +import { + render, + screen, + waitForElementToBeRemoved, +} from "@testing-library/react" +import user from "@testing-library/user-event" +import { Popover } from "./Popover" +import type { PopoverProps } from "./Popover" +import { ThemeProvider } from "../ThemeProvider/ThemeProvider" + +const PopoverTest = (props: Omit) => { + const [anchorEl, setAnchorEl] = React.useState(null) + return ( +
+ + {anchorEl && ( + +

Popover content

+ + + + )} + + +
+ ) +} + +test.each([{ modal: true }, {}, {}])( + "Traps focus if modal = $modal", + async ({ modal }) => { + render(, { + wrapper: ThemeProvider, + }) + + const popover = screen.getByTestId("popover") + expect(popover.contains(document.activeElement)).toBe(true) + + await user.tab() + expect(document.activeElement).toBe( + screen.getByRole("button", { name: "Button 1" }), + ) + + await user.tab() + expect(document.activeElement).toBe( + screen.getByRole("button", { name: "Button 2" }), + ) + + await user.tab() + expect(document.activeElement).toBe( + screen.getByRole("button", { name: "Button 1" }), + ) + }, +) + +test("Popover does not trap focus if modal=false", async () => { + render(, { + wrapper: ThemeProvider, + }) + + expect(document.activeElement).toBe(document.body) + await user.tab() + expect(document.activeElement).toBe( + screen.getByRole("button", { name: "Anchor" }), + ) + + await user.tab() + expect(document.activeElement).toBe( + screen.getByRole("button", { name: "Button 1" }), + ) + await user.tab() + expect(document.activeElement).toBe( + screen.getByRole("button", { name: "Button 2" }), + ) + await user.tab() + expect(document.activeElement).toBe( + screen.getByRole("button", { name: "Other Button" }), + ) +}) + +test.each([ + { role: "dialog", modal: true }, + { role: "dialog" }, + { role: "tooltip", modal: false }, +])("popover role is $role if $modal", ({ modal, role }) => { + render(, { + wrapper: ThemeProvider, + }) + const popover = screen.getByTestId("popover") + const el = screen.getByRole(role) + expect(popover).toBe(el) +}) + +test("Calls onClose when escape is pressed", async () => { + const onClose = jest.fn() + render(, { + wrapper: ThemeProvider, + }) + + await user.type(document.body, "{esc}") + expect(onClose).toHaveBeenCalled() +}) + +test("Calls onClose when clicking outside popover", async () => { + const onClose = jest.fn() + render(, { + wrapper: ThemeProvider, + }) + + const popover = screen.getByTestId("popover") + expect(popover).toBeVisible() + + await user.click(document.body) + expect(onClose).toHaveBeenCalled() +}) + +test("Popover is open/closed based on 'open'", async () => { + const onClose = jest.fn() + const { rerender } = render(, { + wrapper: ThemeProvider, + }) + + const popover = screen.getByTestId("popover") + expect(popover).toBeVisible() + + rerender() + + await waitForElementToBeRemoved(popover) +}) diff --git a/frontends/ol-components/src/components/Popover/Popover.tsx b/frontends/ol-components/src/components/Popover/Popover.tsx new file mode 100644 index 0000000000..f99c63615c --- /dev/null +++ b/frontends/ol-components/src/components/Popover/Popover.tsx @@ -0,0 +1,198 @@ +import * as React from "react" +import styled from "@emotion/styled" +import MuiPopper from "@mui/material/Popper" +import type { PopperProps } from "@mui/material/Popper" +import Fade from "@mui/material/Fade" +import { FocusTrap } from "@mui/base/FocusTrap" +import ClickAwayListener from "@mui/material/ClickAwayListener" + +const StyledPopper = styled(MuiPopper)(({ theme }) => ({ + zIndex: 1, + "& > div": { + position: "relative", + }, + '&[data-popper-placement*="bottom"]': { + "& > div": { + marginTop: 2, + }, + "& .MuiPopper-arrow": { + top: 0, + marginTop: "-0.9em", + width: "3em", + height: "1em", + "&::before": { + borderWidth: "0 1em 1em 1em", + borderColor: `transparent transparent ${theme.palette.background.paper} transparent`, + }, + }, + }, + '&[data-popper-placement*="top"]': { + "& > div": { + marginBottom: 2, + }, + "& .MuiPopper-arrow": { + bottom: 0, + marginBottom: "-0.9em", + width: "3em", + height: "1em", + "&::before": { + borderWidth: "1em 1em 0 1em", + borderColor: `${theme.palette.background.paper} transparent transparent transparent`, + }, + }, + }, + '&[data-popper-placement*="right"]': { + "& > div": { + marginLeft: 2, + }, + "& .MuiPopper-arrow": { + left: 0, + marginLeft: "-0.9em", + height: "3em", + width: "1em", + "&::before": { + borderWidth: "1em 1em 1em 0", + borderColor: `transparent ${theme.palette.background.paper} transparent transparent`, + }, + }, + }, + '&[data-popper-placement*="left"]': { + "& > div": { + marginRight: 2, + }, + "& .MuiPopper-arrow": { + right: 0, + marginRight: "-0.9em", + height: "3em", + width: "1em", + "&::before": { + borderWidth: "1em 0 1em 1em", + borderColor: `transparent transparent transparent ${theme.palette.background.paper}`, + }, + }, + }, +})) + +const getModifiers = ( + arrowEl: HTMLElement | null, +): PopperProps["modifiers"] => [ + { + name: "flip", + enabled: true, + }, + { + name: "preventOverflow", + enabled: true, + }, + { + name: "arrow", + enabled: !!arrowEl, + options: { + element: arrowEl, + }, + }, + { + name: "offset", + options: { + offset: [0, 10], + }, + }, + { + name: "preventOverflow", + enabled: true, + options: { + altAxis: true, + altBoundary: true, + tether: true, + padding: 8, + }, + }, +] + +const Arrow = styled("div")({ + position: "absolute", + fontSize: 7, + width: "3em", + height: "3em", + "&::before": { + content: '""', + margin: "auto", + display: "block", + width: 0, + height: 0, + borderStyle: "solid", + }, +}) + +const Content = styled.div(({ theme }) => ({ + padding: "16px", + backgroundColor: theme.custom.colors.white, + boxShadow: + "0px 2px 4px 0px rgba(37, 38, 43, 0.10), 0px 6px 24px 0px rgba(37, 38, 43, 0.24)", +})) + +type PopoverProps = Pick & { + children?: React.ReactNode + /** + * Called when + * - clicking outside the popover + * - pressing "Escape" + */ + onClose: () => void + /** + * If true (default: `true`), traps focus. + * + * Note: "Escape" closes the popover. + */ + modal?: boolean +} +const Popover: React.FC = ({ + children, + onClose, + modal = true, + open, + ...props +}) => { + const [arrowRef, setArrowRef] = React.useState(null) + const modifiers = getModifiers(arrowRef) + const content = ( +
+ + + + {children} + + +
+ ) + return ( + { + if (event.key === "Escape") { + onClose() + } + }} + > + {({ TransitionProps }) => ( + + {modal ? ( +
+ {content} +
+ ) : ( + content + )} + + )} + + ) +} + +export { Popover } +export type { PopoverProps } diff --git a/frontends/ol-components/src/index.ts b/frontends/ol-components/src/index.ts index ee117c05d4..9e74e84efe 100644 --- a/frontends/ol-components/src/index.ts +++ b/frontends/ol-components/src/index.ts @@ -179,6 +179,7 @@ export * from "./components/TruncateText/TruncateText" export * from "./components/ThemeProvider/ThemeProvider" export * from "./components/RadioChoiceField/RadioChoiceField" export * from "./components/ChoiceBox/ChoiceBox" +export * from "./components/Popover/Popover" export * from "./constants/imgConfigs" diff --git a/yarn.lock b/yarn.lock index 331ee8b5dd..fc80aef174 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19079,6 +19079,7 @@ __metadata: "@emotion/react": "npm:^11.11.1" "@emotion/styled": "npm:^11.11.0" "@faker-js/faker": "npm:^8.0.0" + "@mui/base": "npm:5.0.0-beta.40" "@mui/icons-material": "npm:^5.15.15" "@mui/lab": "npm:^5.0.0-alpha.170" "@mui/material": "npm:^5.15.15" From 9084abf44e61f7a2c52e7c853428db05e4d6c2b2 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Mon, 17 Jun 2024 17:06:18 -0400 Subject: [PATCH 2/7] use signup popover on field page --- frontends/api/src/test-utils/urls.ts | 3 +- .../SearchSubscriptionToggle.test.tsx | 123 ++++++++++++++++++ .../SearchSubscriptionToggle.tsx | 52 +++++--- .../SignupPopover/SignupPopover.test.tsx | 15 +++ .../SignupPopover/SignupPopover.tsx | 41 ++++++ .../src/pages/FieldPage/FieldPage.test.tsx | 54 +++----- 6 files changed, 229 insertions(+), 59 deletions(-) create mode 100644 frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.test.tsx create mode 100644 frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx create mode 100644 frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx diff --git a/frontends/api/src/test-utils/urls.ts b/frontends/api/src/test-utils/urls.ts index cff99f290c..efea70bc49 100644 --- a/frontends/api/src/test-utils/urls.ts +++ b/frontends/api/src/test-utils/urls.ts @@ -148,7 +148,8 @@ const userSubscription = { `${API_BASE_URL}/api/v1/learning_resources_user_subscription/check/${query(params)}`, delete: (id: number) => `${API_BASE_URL}/api/v1/learning_resources_user_subscription/${id}/unsubscribe/`, - post: () => "/api/v1/learning_resources_user_subscription/subscribe/", + post: () => + `${API_BASE_URL}/api/v1/learning_resources_user_subscription/subscribe/`, } const fields = { diff --git a/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.test.tsx b/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.test.tsx new file mode 100644 index 0000000000..f4519e2db2 --- /dev/null +++ b/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.test.tsx @@ -0,0 +1,123 @@ +import React from "react" +import { renderWithProviders, screen, user, within } from "@/test-utils" +import { SearchSubscriptionToggle } from "./SearchSubscriptionToggle" +import { urls, setMockResponse, factories, makeRequest } from "api/test-utils" +import { SourceTypeEnum } from "api" +import type { LearningResourcesUserSubscriptionApiLearningResourcesUserSubscriptionCheckListRequest as CheckSubscriptionRequest } from "api" + +type SetupApisOptions = { + isAuthenticated?: boolean + isSubscribed?: boolean + subscriptionRequest?: CheckSubscriptionRequest +} +const setupApis = ({ + isAuthenticated = false, + isSubscribed = false, + subscriptionRequest = {}, +}: SetupApisOptions = {}) => { + setMockResponse.get(urls.userMe.get(), { + is_authenticated: isAuthenticated, + }) + + const subscribeResponse = isSubscribed + ? factories.percolateQueries.percolateQueryList({ count: 1 }).results + : factories.percolateQueries.percolateQueryList({ count: 0 }).results + setMockResponse.get( + `${urls.userSubscription.check(subscriptionRequest)}`, + subscribeResponse, + ) + + const subscribeUrl = urls.userSubscription.post() + setMockResponse.post(subscribeUrl, subscribeResponse) + + const unsubscribeUrl = urls.userSubscription.delete(subscribeResponse[0]?.id) + setMockResponse.delete(unsubscribeUrl, subscribeResponse[0]) + return { + subscribeUrl, + unsubscribeUrl, + } +} + +test("Shows subscription popover if user is NOT authenticated", async () => { + // Don't set up all the APIs... We don't want to call the others for unauthenticated users. + setMockResponse.get(urls.userMe.get(), {}, { code: 403 }) + renderWithProviders( + , + ) + const subscribeButton = await screen.findByRole("button", { + name: "Follow", + }) + await user.click(subscribeButton) + const popover = await screen.findByRole("dialog") + within(popover).getByRole("link", { name: "Sign Up" }) +}) + +test.each(Object.values(SourceTypeEnum))( + "Allows subscribing if authenticated and NOT subscribed (source_type=%s)", + async (sourceType) => { + const { subscribeUrl } = setupApis({ + isAuthenticated: true, + isSubscribed: false, + subscriptionRequest: { + source_type: sourceType, + offered_by: ["ocw"], + }, + }) + renderWithProviders( + , + ) + const subscribeButton = await screen.findByRole("button", { + name: "Follow", + }) + await user.click(subscribeButton) + expect(makeRequest).toHaveBeenCalledWith("post", subscribeUrl, { + source_type: sourceType, + offered_by: ["ocw"], + }) + }, +) + +test.each(Object.values(SourceTypeEnum))( + "Allows unsubscribing if authenticated and subscribed (source_type=%s)", + async (sourceType) => { + const { unsubscribeUrl } = setupApis({ + isAuthenticated: true, + isSubscribed: true, + subscriptionRequest: { + source_type: sourceType, + offered_by: ["ocw"], + }, + }) + + renderWithProviders( + , + ) + + const subscribedButton = await screen.findByRole("button", { + name: "Following", + }) + + await user.click(subscribedButton) + + const menu = screen.getByRole("menu") + const unsubscribeButton = within(menu).getByRole("menuitem", { + name: "Unfollow", + }) + await user.click(unsubscribeButton) + + expect(makeRequest).toHaveBeenCalledWith( + "delete", + unsubscribeUrl, + undefined, + ) + }, +) diff --git a/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx b/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx index ef46c83708..4b49f5fca8 100644 --- a/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx +++ b/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx @@ -11,14 +11,18 @@ import ExpandMoreSharpIcon from "@mui/icons-material/ExpandMoreSharp" import { useUserMe } from "api/hooks/user" import { SourceTypeEnum } from "api" import MailOutlineIcon from "@mui/icons-material/MailOutline" +import { SignupPopover } from "../SignupPopover/SignupPopover" -const SearchSubscriptionToggle = ({ - searchParams, - sourceType, -}: { +type SearchSubscriptionToggleProps = { searchParams: URLSearchParams sourceType: SourceTypeEnum +} + +const SearchSubscriptionToggle: React.FC = ({ + searchParams, + sourceType, }) => { + const [buttonEl, setButtonEl] = React.useState(null) const subscribeParams: Record = useMemo(() => { return { source_type: sourceType, ...getSearchParamMap(searchParams) } }, [searchParams, sourceType]) @@ -27,7 +31,7 @@ const SearchSubscriptionToggle = ({ const subscriptionDelete = useSearchSubscriptionDelete() const subscriptionCreate = useSearchSubscriptionCreate() const subscriptionList = useSearchSubscriptionList(subscribeParams, { - enabled: user?.is_authenticated, + enabled: !!user?.is_authenticated, }) const unsubscribe = subscriptionDelete.mutate @@ -44,14 +48,14 @@ const SearchSubscriptionToggle = ({ ] }, [unsubscribe, subscriptionId]) - if (subscriptionList.isLoading) return null - if (!user?.is_authenticated) return null + if (user?.is_authenticated && subscriptionList.isLoading) return null + if (!user) return null if (isSubscribed) { return ( }> - Follow + Following } items={unsubscribeItems} @@ -59,19 +63,27 @@ const SearchSubscriptionToggle = ({ ) } return ( - + <> + + setButtonEl(null)} /> + ) } export { SearchSubscriptionToggle } +export type { SearchSubscriptionToggleProps } diff --git a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx new file mode 100644 index 0000000000..5c085dd689 --- /dev/null +++ b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx @@ -0,0 +1,15 @@ +import React from "react" +import { SignupPopover } from "./SignupPopover" +import { renderWithProviders, screen, within } from "@/test-utils" +import invariant from "tiny-invariant" +import * as urls from "@/common/urls" + +test("SignupPopover shows link to sign up", async () => { + renderWithProviders( + , + ) + const dialog = screen.getByRole("dialog") + const link = within(dialog).getByRole("link") + invariant(link instanceof HTMLAnchorElement) + expect(link.href).toMatch(urls.login()) +}) diff --git a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx new file mode 100644 index 0000000000..def7f7dd92 --- /dev/null +++ b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx @@ -0,0 +1,41 @@ +import React from "react" +import { Popover, Typography, styled, ButtonLink } from "ol-components" +import type { PopoverProps } from "ol-components" +import * as urls from "@/common/urls" + +const StyledPopover = styled(Popover)({ + width: "300px", + maxWidth: "100vw", +}) +const HeaderText = styled(Typography)(({ theme }) => ({ + color: theme.custom.colors.darkGray2, + marginBottom: "8px", +})) +const BodyText = styled(Typography)(({ theme }) => ({ + color: theme.custom.colors.silverGrayDark, + marginBottom: "16px", +})) + +const Footer = styled.div({ + display: "flex", + justifyContent: "end", +}) + +type SignupPopoverProps = Pick +const SignupPopover: React.FC = (props) => { + return ( + + Join MIT Open for free + + As a member, get personalized recommendations, curate learning lists, + and follow your areas of interests. + +
+ Sign Up +
+ + ) +} + +export { SignupPopover } +export type { SignupPopoverProps } diff --git a/frontends/mit-open/src/pages/FieldPage/FieldPage.test.tsx b/frontends/mit-open/src/pages/FieldPage/FieldPage.test.tsx index feda034f4f..36332396ed 100644 --- a/frontends/mit-open/src/pages/FieldPage/FieldPage.test.tsx +++ b/frontends/mit-open/src/pages/FieldPage/FieldPage.test.tsx @@ -7,7 +7,6 @@ import { screen, setMockResponse, within, - user, act, waitFor, } from "../../test-utils" @@ -25,12 +24,11 @@ const mockedFieldSearch = jest.mocked(FieldSearch) const setupApis = ( fieldPatch?: Partial, search?: Partial, - userIsAuthenticated?: boolean, - userIsSubscribed?: boolean, + { isSubscribed = false, isAuthenticated = false } = {}, ) => { const field = factories.fields.field(fieldPatch) setMockResponse.get(urls.userMe.get(), { - is_authenticated: userIsAuthenticated, + is_authenticated: isAuthenticated, }) setMockResponse.get( urls.fields.details(field.channel_type, field.name), @@ -51,7 +49,7 @@ const setupApis = ( subscribeParams[key] = value.split(",") } subscribeParams["source_type"] = "channel_subscription_type" - const subscribeResponse = userIsSubscribed + const subscribeResponse = isSubscribed ? factories.percolateQueries.percolateQueryList({ count: 1 }).results : factories.percolateQueries.percolateQueryList({ count: 0 }).results if (fieldPatch?.search_filter) { @@ -59,13 +57,6 @@ const setupApis = ( `${urls.userSubscription.check(subscribeParams)}`, subscribeResponse, ) - setMockResponse.post(`${urls.userSubscription.post()}`, subscribeResponse) - } - if (userIsSubscribed === true) { - setMockResponse.delete( - urls.userSubscription.delete(subscribeResponse[0]?.id), - subscribeResponse[0], - ) } setMockResponse.get( @@ -187,30 +178,17 @@ describe("FieldPage", () => { }) }) - it("Displays the unsubscribe toggle if the user is authenticated and subscribed", async () => { - const { field } = setupApis({ search_filter: "q=ocw" }, {}, true, true) - renderTestApp({ url: `/c/${field.channel_type}/${field.name}` }) - await screen.findByText(field.title) - const subscribedButton = await screen.findByText("Follow") - assertInstanceOf(subscribedButton, HTMLButtonElement) - user.click(subscribedButton) - const unsubscribeButton = await screen.findByText("Unfollow") - assertInstanceOf(unsubscribeButton, HTMLLIElement) - }) - - it("Displays the subscribe toggle if the user is authenticated but not subscribed", async () => { - const { field } = setupApis({ search_filter: "q=ocw" }, {}, true, false) - renderTestApp({ url: `/c/${field.channel_type}/${field.name}` }) - await screen.findByText(field.title) - const subscribeButton = await screen.findByText("Follow") - assertInstanceOf(subscribeButton, HTMLButtonElement) - }) - it("Hides the subscribe toggle if the user is not authenticated", async () => { - const { field } = setupApis({ search_filter: "q=ocw" }, {}, false, false) - renderTestApp({ url: `/c/${field.channel_type}/${field.name}` }) - await screen.findByText(field.title) - await waitFor(() => { - expect(screen.queryByText("Follow")).not.toBeInTheDocument() - }) - }) + it.each([{ isSubscribed: false }, { isSubscribed: true }])( + "Displays the subscribe toggle for authenticated and unauthenticated users", + async ({ isSubscribed }) => { + const { field } = setupApis( + { search_filter: "q=ocw" }, + {}, + { isSubscribed }, + ) + renderTestApp({ url: `/c/${field.channel_type}/${field.name}` }) + const subscribedButton = await screen.findByText("Follow") + expect(subscribedButton).toBeVisible() + }, + ) }) From a5b4cf180235aa1f723d7b35ad220561868f67d4 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 18 Jun 2024 16:01:02 -0400 Subject: [PATCH 3/7] use popover for resource cards --- .../LearningResourceDrawer.tsx | 13 +- .../ResourceCard/ResourceCard.tsx | 118 ++++++++++++++++++ .../ResourceCarousel.test.tsx | 13 +- .../ResourceCarousel/ResourceCarousel.tsx | 31 +---- .../SearchDisplay/SearchDisplay.tsx | 35 +----- .../SignupPopover/SignupPopover.tsx | 5 +- .../LearningResourceCard.test.tsx | 16 ++- .../LearningResourceCard.tsx | 10 +- .../LearningResourceListCard.test.tsx | 14 ++- .../LearningResourceListCard.tsx | 10 +- .../src/components/Popover/Popover.test.tsx | 7 +- .../src/components/Popover/Popover.tsx | 11 -- 12 files changed, 192 insertions(+), 91 deletions(-) create mode 100644 frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx diff --git a/frontends/mit-open/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx b/frontends/mit-open/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx index a4cd859725..b1684c032b 100644 --- a/frontends/mit-open/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx +++ b/frontends/mit-open/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from "react" +import React, { useEffect, useCallback } from "react" import { RoutedDrawer, LearningResourceExpanded, @@ -96,10 +96,13 @@ const useOpenLearningResourceDrawer = () => { const useResourceDrawerHref = () => { const [search] = useSearchParams() - return (id: number) => { - search.set(RESOURCE_DRAWER_QUERY_PARAM, id.toString()) - return `?${search.toString()}` - } + return useCallback( + (id: number) => { + search.set(RESOURCE_DRAWER_QUERY_PARAM, id.toString()) + return `?${search.toString()}` + }, + [search], + ) } export default LearningResourceDrawer diff --git a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx new file mode 100644 index 0000000000..88fba80ac7 --- /dev/null +++ b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx @@ -0,0 +1,118 @@ +import React, { useCallback, useMemo, useState } from "react" +import { LearningResourceCard, LearningResourceListCard } from "ol-components" +import * as NiceModal from "@ebay/nice-modal-react" +import type { + LearningResourceCardProps, + LearningResourceListCardProps, +} from "ol-components" +import { + AddToLearningPathDialog, + AddToUserListDialog, +} from "../Dialogs/AddToListDialog" +import { useResourceDrawerHref } from "../LearningResourceDrawer/LearningResourceDrawer" +import { useUserMe } from "api/hooks/user" +import { SignupPopover } from "../SignupPopover/SignupPopover" + +const useResourceCard = () => { + const getDrawerHref = useResourceDrawerHref() + const { data: user } = useUserMe() + + const [anchorEl, setAnchorEl] = useState(null) + + const handleClosePopover = useCallback(() => { + setAnchorEl(null) + }, []) + const handleAddToLearningPathClick: LearningResourceCardProps["onAddToLearningPathClick"] = + useMemo(() => { + if (user?.is_authenticated && user?.is_learning_path_editor) { + return (event, resourceId: number) => { + NiceModal.show(AddToLearningPathDialog, { resourceId }) + } + } + return null + }, [user]) + + const handleAddToUserListClick: LearningResourceCardProps["onAddToUserListClick"] = + useMemo(() => { + if (!user) { + // user info is still loading + return null + } + if (user.is_authenticated) { + return (event, resourceId: number) => { + NiceModal.show(AddToUserListDialog, { resourceId }) + } + } + return (event) => { + setAnchorEl(event.currentTarget) + } + }, [user]) + + return { + getDrawerHref, + anchorEl, + handleClosePopover, + handleAddToLearningPathClick, + handleAddToUserListClick, + } +} + +type ResourceCardProps = Omit< + LearningResourceCardProps, + "href" | "onAddToLearningPathClick" | "onAddToUserListClick" +> + +const ResourceCard: React.FC = ({ resource, ...others }) => { + const { + getDrawerHref, + anchorEl, + handleClosePopover, + handleAddToLearningPathClick, + handleAddToUserListClick, + } = useResourceCard() + return ( + <> + + + + ) +} + +type ResourceListCardProps = Omit< + LearningResourceListCardProps, + "href" | "onAddToLearningPathClick" | "onAddToUserListClick" +> + +const ResourceListCard: React.FC = ({ + resource, + ...others +}) => { + const { + getDrawerHref, + anchorEl, + handleClosePopover, + handleAddToLearningPathClick, + handleAddToUserListClick, + } = useResourceCard() + return ( + <> + + + + ) +} + +export { ResourceCard, ResourceListCard } +export type { ResourceCardProps, ResourceListCardProps } 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 8dcd4c205a..358f21718d 100644 --- a/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx +++ b/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.test.tsx @@ -11,6 +11,7 @@ import { import { factories, setMockResponse, makeRequest, urls } from "api/test-utils" import { LearningResourceCard } from "ol-components" import { ControlledPromise } from "ol-test-utilities" +import invariant from "tiny-invariant" jest.mock("ol-components", () => { const actual = jest.requireActual("ol-components") @@ -161,9 +162,17 @@ describe("ResourceCarousel", () => { , ) await waitFor(() => { - expect(makeRequest.mock.calls.length > 0).toBe(true) + expect(makeRequest).toHaveBeenCalledWith( + "get", + expect.stringContaining(urls.learningResources.list()), + undefined, + ) }) - const [_method, url] = makeRequest.mock.calls[0] + const [_method, url] = + makeRequest.mock.calls.find(([_method, url]) => { + return url.includes(urls.learningResources.list()) + }) ?? [] + invariant(url) const urlParams = new URLSearchParams(url.split("?")[1]) expect(urlParams.getAll("resource_type")).toEqual(["course", "program"]) expect(urlParams.get("professional")).toEqual("true") diff --git a/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx b/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx index 9e5edb21b7..db7cfc8c6c 100644 --- a/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx +++ b/frontends/mit-open/src/page-components/ResourceCarousel/ResourceCarousel.tsx @@ -1,5 +1,4 @@ import React from "react" -import * as NiceModal from "@ebay/nice-modal-react" import { useFeaturedLearningResourcesList, useLearningResourcesList, @@ -12,7 +11,6 @@ import { TabContext, TabButtonList, styled, - LearningResourceCard, Typography, } from "ol-components" import type { @@ -22,12 +20,7 @@ import type { FeaturedDataSource, } from "./types" import { LearningResource } from "api" -import { useUserMe } from "api/hooks/user" -import { - AddToLearningPathDialog, - AddToUserListDialog, -} from "../Dialogs/AddToListDialog" -import { useResourceDrawerHref } from "../LearningResourceDrawer/LearningResourceDrawer" +import { ResourceCard } from "../ResourceCard/ResourceCard" const StyledCarousel = styled(Carousel)({ /** @@ -261,23 +254,8 @@ const ResourceCarousel: React.FC = ({ className, isLoading, }) => { - const { data: user } = useUserMe() const [tab, setTab] = React.useState("0") const [ref, setRef] = React.useState(null) - const getDrawerHref = useResourceDrawerHref() - - const showAddToLearningPathDialog = - user?.is_authenticated && user?.is_learning_path_editor - ? (resourceId: number) => { - NiceModal.show(AddToLearningPathDialog, { resourceId }) - } - : null - - const showAddToUserListDialog = user?.is_authenticated - ? (resourceId: number) => { - NiceModal.show(AddToUserListDialog, { resourceId }) - } - : null return ( @@ -305,7 +283,7 @@ const ResourceCarousel: React.FC = ({ {isLoading || childrenLoading ? Array.from({ length: 6 }).map((_, index) => ( - = ({ /> )) : resources.map((resource) => ( - ))} diff --git a/frontends/mit-open/src/page-components/SearchDisplay/SearchDisplay.tsx b/frontends/mit-open/src/page-components/SearchDisplay/SearchDisplay.tsx index 0a82c3845a..0dc9f2a402 100644 --- a/frontends/mit-open/src/page-components/SearchDisplay/SearchDisplay.tsx +++ b/frontends/mit-open/src/page-components/SearchDisplay/SearchDisplay.tsx @@ -12,10 +12,8 @@ import { SimpleSelect, truncateText, css, - LearningResourceListCard, Drawer, } from "ol-components" -import * as NiceModal from "@ebay/nice-modal-react" import { RiCloseLine, @@ -44,12 +42,8 @@ import _ from "lodash" import { ResourceTypeTabs } from "./ResourceTypeTabs" import ProfessionalToggle from "./ProfessionalToggle" import type { TabConfig } from "./ResourceTypeTabs" -import { useUserMe } from "api/hooks/user" -import { - AddToLearningPathDialog, - AddToUserListDialog, -} from "../Dialogs/AddToListDialog" -import { useResourceDrawerHref } from "@/page-components/LearningResourceDrawer/LearningResourceDrawer" + +import { ResourceListCard } from "../ResourceCard/ResourceCard" export const StyledDropdown = styled(SimpleSelect)` margin-left: 8px; @@ -509,22 +503,6 @@ const SearchDisplay: React.FC = ({ { keepPreviousData: true }, ) - const { data: user } = useUserMe() - - const getDrawerHref = useResourceDrawerHref() - - const showAddToLearningPathDialog = - user?.is_authenticated && user?.is_learning_path_editor - ? (resourceId: number) => { - NiceModal.show(AddToLearningPathDialog, { resourceId }) - } - : null - - const showAddToUserListDialog = user?.is_authenticated - ? (resourceId: number) => { - NiceModal.show(AddToUserListDialog, { resourceId }) - } - : null const [mobileDrawerOpen, setMobileDrawerOpen] = React.useState(false) const toggleMobileDrawer = (newOpen: boolean) => () => { @@ -647,7 +625,7 @@ const SearchDisplay: React.FC = ({ .fill(null) .map((a, index) => (
  • - +
    • ))} @@ -655,12 +633,7 @@ const SearchDisplay: React.FC = ({ {data.results.map((resource) => (
  • - +
    • ))}     diff --git a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx index def7f7dd92..cda49ecfb0 100644 --- a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx +++ b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx @@ -21,7 +21,10 @@ const Footer = styled.div({ justifyContent: "end", }) -type SignupPopoverProps = Pick +type SignupPopoverProps = Pick< + PopoverProps, + "anchorEl" | "onClose" | "placement" +> const SignupPopover: React.FC = (props) => { return ( diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.test.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.test.tsx index faafe7b105..3173d53d03 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.test.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.test.tsx @@ -108,13 +108,25 @@ describe("Learning Resource Card", () => { const addToLearningPathButton = screen.getByLabelText( "Add to Learning Path", ) + await addToLearningPathButton.click() const addToUserListButton = screen.getByLabelText("Add to User List") + await addToUserListButton.click() - expect(onAddToLearningPathClick).toHaveBeenCalledWith(resource.id) - expect(onAddToUserListClick).toHaveBeenCalledWith(resource.id) + expect(onAddToLearningPathClick).toHaveBeenCalledWith( + expect.objectContaining({ + target: expect.any(HTMLElement), + }), + resource.id, + ) + expect(onAddToUserListClick).toHaveBeenCalledWith( + expect.objectContaining({ + target: expect.any(HTMLElement), + }), + resource.id, + ) }) test("Displays certificate badge", () => { diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx index 2580d476e0..b37d9dbf79 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceCard.tsx @@ -36,7 +36,10 @@ const getEmbedlyUrl = (resource: LearningResource, size: Size) => { }) } -type ResourceIdCallback = (resourceId: number) => void +type ResourceIdCallback = ( + event: React.MouseEvent, + resourceId: number, +) => void const Info = ({ resource }: { resource: LearningResource }) => { return ( @@ -169,7 +172,7 @@ const LearningResourceCard: React.FC = ({ color="secondary" size="small" aria-label="Add to Learning Path" - onClick={() => onAddToLearningPathClick(resource.id)} + onClick={(event) => onAddToLearningPathClick(event, resource.id)} > @@ -181,7 +184,7 @@ const LearningResourceCard: React.FC = ({ color="secondary" size="small" aria-label="Add to User List" - onClick={() => onAddToUserListClick(resource.id)} + onClick={(event) => onAddToUserListClick(event, resource.id)} > @@ -195,3 +198,4 @@ const LearningResourceCard: React.FC = ({ } export { LearningResourceCard } +export type { LearningResourceCardProps } diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx index 3ccac0b182..b453a156be 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx @@ -113,8 +113,18 @@ describe("Learning Resource List Card", () => { const addToUserListButton = screen.getByLabelText("Add to User List") await addToUserListButton.click() - expect(onAddToLearningPathClick).toHaveBeenCalledWith(resource.id) - expect(onAddToUserListClick).toHaveBeenCalledWith(resource.id) + expect(onAddToLearningPathClick).toHaveBeenCalledWith( + expect.objectContaining({ + target: expect.any(HTMLElement), + }), + resource.id, + ) + expect(onAddToUserListClick).toHaveBeenCalledWith( + expect.objectContaining({ + target: expect.any(HTMLElement), + }), + resource.id, + ) }) test("Displays certificate badge", () => { diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index bafa231907..4907c4ebb1 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -85,7 +85,10 @@ const StyledActionButton = styled(ActionButton)<{ edge: string }>` : ""} ` -type ResourceIdCallback = (resourceId: number) => void +type ResourceIdCallback = ( + event: React.MouseEvent, + resourceId: number, +) => void const getEmbedlyUrl = (url: string, isMobile: boolean) => { return embedlyCroppedImage(url, { @@ -396,7 +399,7 @@ const LearningResourceListCard: React.FC = ({ color="secondary" size="small" aria-label="Add to Learning Path" - onClick={() => onAddToLearningPathClick(resource.id)} + onClick={(event) => onAddToLearningPathClick(event, resource.id)} > @@ -408,7 +411,7 @@ const LearningResourceListCard: React.FC = ({ color="secondary" size="small" aria-label="Add to User List" - onClick={() => onAddToUserListClick(resource.id)} + onClick={(event) => onAddToUserListClick(event, resource.id)} > @@ -427,3 +430,4 @@ const LearningResourceListCard: React.FC = ({ } export { LearningResourceListCard } +export type { LearningResourceListCardProps } diff --git a/frontends/ol-components/src/components/Popover/Popover.test.tsx b/frontends/ol-components/src/components/Popover/Popover.test.tsx index f80b628c36..ac666acfee 100644 --- a/frontends/ol-components/src/components/Popover/Popover.test.tsx +++ b/frontends/ol-components/src/components/Popover/Popover.test.tsx @@ -67,15 +67,16 @@ test("Popover does not trap focus if modal=false", async () => { await user.tab() expect(document.activeElement).toBe( - screen.getByRole("button", { name: "Button 1" }), + screen.getByRole("button", { name: "Other Button" }), ) + await user.tab() expect(document.activeElement).toBe( - screen.getByRole("button", { name: "Button 2" }), + screen.getByRole("button", { name: "Button 1" }), ) await user.tab() expect(document.activeElement).toBe( - screen.getByRole("button", { name: "Other Button" }), + screen.getByRole("button", { name: "Button 2" }), ) }) diff --git a/frontends/ol-components/src/components/Popover/Popover.tsx b/frontends/ol-components/src/components/Popover/Popover.tsx index f99c63615c..c464af4d32 100644 --- a/frontends/ol-components/src/components/Popover/Popover.tsx +++ b/frontends/ol-components/src/components/Popover/Popover.tsx @@ -97,16 +97,6 @@ const getModifiers = ( offset: [0, 10], }, }, - { - name: "preventOverflow", - enabled: true, - options: { - altAxis: true, - altBoundary: true, - tether: true, - padding: 8, - }, - }, ] const Arrow = styled("div")({ @@ -169,7 +159,6 @@ const Popover: React.FC = ({ Date: Tue, 18 Jun 2024 16:21:42 -0400 Subject: [PATCH 4/7] add a test for resource card --- .../ResourceCard/ResourceCard.test.tsx | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx diff --git a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx new file mode 100644 index 0000000000..4163040d6c --- /dev/null +++ b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.test.tsx @@ -0,0 +1,138 @@ +import React from "react" +import * as NiceModal from "@ebay/nice-modal-react" +import { renderWithProviders, user, screen } from "../../test-utils" +import type { User } from "../../test-utils" +import { ResourceCard, ResourceListCard } from "./ResourceCard" +import { + AddToLearningPathDialog, + AddToUserListDialog, +} 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 invariant from "tiny-invariant" + +jest.mock("@ebay/nice-modal-react", () => { + const actual = jest.requireActual("@ebay/nice-modal-react") + return { + __esModule: true, + ...actual, + show: jest.fn(), + } +}) + +describe.each([ + { + CardComponent: ResourceCard, + }, + { + CardComponent: ResourceListCard, + }, +])("$CardComponent", ({ CardComponent }) => { + const makeResource = factories.learningResources.resource + type SetupOptions = { + user?: Partial + props?: Partial + } + const setup = ({ user, props = {} }: SetupOptions = {}) => { + const { resource = makeResource() } = props + if (user?.is_authenticated) { + setMockResponse.get(urls.userMe.get(), user) + } else { + setMockResponse.get(urls.userMe.get(), {}, { code: 403 }) + } + const { view, location } = renderWithProviders( + , + ) + return { resource, view, location } + } + + const labels = { + addToLearningPaths: "Add to Learning Path", + addToUserList: "Add to User List", + } + + test("Applies className to the resource card", () => { + const { view } = setup({ user: {}, props: { className: "test-class" } }) + expect(view.container.firstChild).toHaveClass("test-class") + }) + + test.each([ + { + user: { is_authenticated: true, is_learning_path_editor: false }, + expectAddToLearningPathButton: false, + }, + { + user: { is_authenticated: true, is_learning_path_editor: true }, + expectAddToLearningPathButton: true, + }, + { + user: { is_authenticated: false }, + expectAddToLearningPathButton: false, + }, + ])( + "Always shows 'Add to User List' button, but only shows 'Add to Learning Path' button if user is a learning path editor", + async ({ user, expectAddToLearningPathButton }) => { + setup({ user }) + await screen.findByRole("button", { + name: labels.addToUserList, + }) + const addToLearningPathButton = screen.queryByRole("button", { + name: labels.addToLearningPaths, + }) + expect(!!addToLearningPathButton).toBe(expectAddToLearningPathButton) + }, + ) + + test("Clicking add to list button opens AddToListDialog when authenticated", async () => { + const showModal = jest.mocked(NiceModal.show) + + const { resource } = setup({ + user: { is_learning_path_editor: true, is_authenticated: true }, + }) + const addToUserListButton = await screen.findByRole("button", { + name: labels.addToUserList, + }) + const addToLearningPathButton = await screen.findByRole("button", { + name: labels.addToLearningPaths, + }) + + expect(showModal).not.toHaveBeenCalled() + await user.click(addToLearningPathButton) + invariant(resource) + expect(showModal).toHaveBeenLastCalledWith(AddToLearningPathDialog, { + resourceId: resource.id, + }) + await user.click(addToUserListButton) + expect(showModal).toHaveBeenLastCalledWith(AddToUserListDialog, { + resourceId: resource.id, + }) + }) + + test("Clicking 'Add to User List' opens signup popover if not authenticated", async () => { + setup({ + user: { is_authenticated: false }, + }) + const addToUserListButton = await screen.findByRole("button", { + name: labels.addToUserList, + }) + await user.click(addToUserListButton) + const dialog = screen.getByRole("dialog") + expect(dialog).toBeVisible() + expect(dialog).toHaveTextContent("Sign Up") + }) + + test("Clicking card opens resource drawer", async () => { + const { resource, location } = setup({ + user: { is_learning_path_editor: true }, + }) + invariant(resource) + const cardTitle = screen.getByRole("heading", { name: resource.title }) + await user.click(cardTitle) + expect( + new URLSearchParams(location.current.search).get( + RESOURCE_DRAWER_QUERY_PARAM, + ), + ).toBe(String(resource.id)) + }) +}) From 81d089b78b0538f5b031e72cec0c5429b4d44126 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 18 Jun 2024 16:29:32 -0400 Subject: [PATCH 5/7] add two comments --- .../page-components/ResourceCard/ResourceCard.tsx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx index 88fba80ac7..9636f5380f 100644 --- a/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx +++ b/frontends/mit-open/src/page-components/ResourceCard/ResourceCard.tsx @@ -62,6 +62,13 @@ type ResourceCardProps = Omit< "href" | "onAddToLearningPathClick" | "onAddToUserListClick" > +/** + * Just like `ol-components/LearningResourceCard`, but with builtin actions: + * - click opens the Resource Drawer + * - onAddToListClick opens the Add to List modal + * - for unauthenticated users, a popover prompts signup instead. + * - onAddToLearningPathClick opens the Add to Learning Path modal + */ const ResourceCard: React.FC = ({ resource, ...others }) => { const { getDrawerHref, @@ -89,6 +96,13 @@ type ResourceListCardProps = Omit< "href" | "onAddToLearningPathClick" | "onAddToUserListClick" > +/** + * Just like `ol-components/LearningResourceListCard`, but with builtin actions: + * - click opens the Resource Drawer + * - onAddToListClick opens the Add to List modal + * - for unauthenticated users, a popover prompts signup instead. + * - onAddToLearningPathClick opens the Add to Learning Path modal + */ const ResourceListCard: React.FC = ({ resource, ...others From a8b2ddfe1adcd91d89368705f12033a9dc53e76c Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 18 Jun 2024 16:51:13 -0400 Subject: [PATCH 6/7] add a comment, restore preventOverflow --- .../ol-components/src/components/Popover/Popover.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frontends/ol-components/src/components/Popover/Popover.tsx b/frontends/ol-components/src/components/Popover/Popover.tsx index c464af4d32..46b9e003eb 100644 --- a/frontends/ol-components/src/components/Popover/Popover.tsx +++ b/frontends/ol-components/src/components/Popover/Popover.tsx @@ -6,6 +6,10 @@ import Fade from "@mui/material/Fade" import { FocusTrap } from "@mui/base/FocusTrap" import ClickAwayListener from "@mui/material/ClickAwayListener" +/** + * Based on MUI demo: + * https://github.com/mui/material-ui/blob/d3ef60158ba066779102fba775dda6765e2cc0f5/docs/data/material/components/popper/ScrollPlayground.js#L175 + */ const StyledPopper = styled(MuiPopper)(({ theme }) => ({ zIndex: 1, "& > div": { @@ -97,6 +101,13 @@ const getModifiers = ( offset: [0, 10], }, }, + { + name: "preventOverflow", + enabled: true, + options: { + padding: 8, + }, + }, ] const Arrow = styled("div")({ From 11a680b911b5d599b7a4febb7a87f07272b97198 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 20 Jun 2024 10:44:00 -0400 Subject: [PATCH 7/7] use SITE_NAME, fix login redirect --- .../SignupPopover/SignupPopover.test.tsx | 10 +++++++++- .../SignupPopover/SignupPopover.tsx | 15 +++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx index 5c085dd689..f5c6fe4cee 100644 --- a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx +++ b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.test.tsx @@ -7,9 +7,17 @@ import * as urls from "@/common/urls" test("SignupPopover shows link to sign up", async () => { renderWithProviders( , + { + url: "/some-path?dog=woof", + }, ) const dialog = screen.getByRole("dialog") const link = within(dialog).getByRole("link") invariant(link instanceof HTMLAnchorElement) - expect(link.href).toMatch(urls.login()) + expect(link.href).toMatch( + urls.login({ + pathname: "/some-path", + search: "?dog=woof", + }), + ) }) diff --git a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx index cda49ecfb0..131eb603db 100644 --- a/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx +++ b/frontends/mit-open/src/page-components/SignupPopover/SignupPopover.tsx @@ -2,6 +2,7 @@ import React from "react" import { Popover, Typography, styled, ButtonLink } from "ol-components" import type { PopoverProps } from "ol-components" import * as urls from "@/common/urls" +import { useLocation } from "react-router" const StyledPopover = styled(Popover)({ width: "300px", @@ -26,15 +27,25 @@ type SignupPopoverProps = Pick< "anchorEl" | "onClose" | "placement" > const SignupPopover: React.FC = (props) => { + const loc = useLocation() return ( - Join MIT Open for free + + Join {process.env.SITE_NAME} for free. + As a member, get personalized recommendations, curate learning lists, and follow your areas of interests. )