diff --git a/frontends/mit-learn/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx b/frontends/mit-learn/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx index 8dc5f85f96..3dba912931 100644 --- a/frontends/mit-learn/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx +++ b/frontends/mit-learn/src/page-components/LearningResourceDrawer/LearningResourceDrawer.tsx @@ -19,7 +19,7 @@ import { AddToLearningPathDialog, AddToUserListDialog, } from "../Dialogs/AddToListDialog" -import * as urls from "@/common/urls" +import { SignupPopover } from "../SignupPopover/SignupPopover" const RESOURCE_DRAWER_PARAMS = [RESOURCE_DRAWER_QUERY_PARAM] as const @@ -65,8 +65,8 @@ const unsafe_html2plaintext = (text: string) => { const DrawerContent: React.FC<{ resourceId: number }> = ({ resourceId }) => { - const loc = useLocation() const resource = useLearningResourcesDetail(Number(resourceId)) + const [signupEl, setSignupEl] = React.useState(null) const { data: user } = useUserMe() const handleAddToLearningPathClick: LearningResourceCardProps["onAddToLearningPathClick"] = useMemo(() => { @@ -79,12 +79,13 @@ const DrawerContent: React.FC<{ }, [user]) const handleAddToUserListClick: LearningResourceCardProps["onAddToUserListClick"] = useMemo(() => { - if (user?.is_authenticated) { - return (event, resourceId: number) => { - NiceModal.show(AddToUserListDialog, { resourceId }) + return (event, resourceId: number) => { + if (!user?.is_authenticated) { + setSignupEl(event.currentTarget) + return } + NiceModal.show(AddToUserListDialog, { resourceId }) } - return null }, [user]) useCapturePageView(Number(resourceId)) @@ -103,11 +104,8 @@ const DrawerContent: React.FC<{ user={user} onAddToLearningPathClick={handleAddToLearningPathClick} onAddToUserListClick={handleAddToUserListClick} - signupUrl={urls.login({ - pathname: loc.pathname, - search: loc.search, - })} /> + setSignupEl(null)} /> ) } diff --git a/frontends/mit-learn/src/page-components/ResourceCard/ResourceCard.tsx b/frontends/mit-learn/src/page-components/ResourceCard/ResourceCard.tsx index a31ca28c69..09b39f42d4 100644 --- a/frontends/mit-learn/src/page-components/ResourceCard/ResourceCard.tsx +++ b/frontends/mit-learn/src/page-components/ResourceCard/ResourceCard.tsx @@ -3,7 +3,6 @@ import { LearningResourceCard, LearningResourceListCard, LearningResourceListCardCondensed, - SignupPopover, } from "ol-components" import * as NiceModal from "@ebay/nice-modal-react" import type { LearningResourceCardProps } from "ol-components" @@ -14,8 +13,7 @@ import { import { useResourceDrawerHref } from "../LearningResourceDrawer/LearningResourceDrawer" import { useUserMe } from "api/hooks/user" import { LearningResource } from "api" -import * as urls from "@/common/urls" -import { useLocation } from "react-router" +import { SignupPopover } from "../SignupPopover/SignupPopover" const useResourceCard = (resource?: LearningResource | null) => { const getDrawerHref = useResourceDrawerHref() @@ -87,7 +85,6 @@ const ResourceCard: React.FC = ({ list, ...others }) => { - const loc = useLocation() const { getDrawerHref, anchorEl, @@ -114,14 +111,7 @@ const ResourceCard: React.FC = ({ inLearningPath={inLearningPath} {...others} /> - + ) } diff --git a/frontends/mit-learn/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx b/frontends/mit-learn/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx index fe9b3b119f..b80f87e884 100644 --- a/frontends/mit-learn/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx +++ b/frontends/mit-learn/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx @@ -5,13 +5,12 @@ import { useSearchSubscriptionDelete, useSearchSubscriptionList, } from "api/hooks/searchSubscription" -import { Button, SignupPopover, SimpleMenu, styled } from "ol-components" +import { Button, SimpleMenu, styled } from "ol-components" import type { SimpleMenuItem } from "ol-components" import { RiArrowDownSLine, RiMailLine } from "@remixicon/react" import { useUserMe } from "api/hooks/user" import { SourceTypeEnum } from "api" -import * as urls from "@/common/urls" -import { useLocation } from "react-router" +import { SignupPopover } from "../SignupPopover/SignupPopover" const StyledButton = styled(Button)({ minWidth: "130px", @@ -27,7 +26,6 @@ const SearchSubscriptionToggle: React.FC = ({ searchParams, sourceType, }) => { - const loc = useLocation() const [buttonEl, setButtonEl] = useState(null) const subscribeParams: Record = useMemo(() => { @@ -92,14 +90,7 @@ const SearchSubscriptionToggle: React.FC = ({ > Follow - setButtonEl(null)} - /> + setButtonEl(null)} /> ) } diff --git a/frontends/mit-learn/src/page-components/SignupPopover/SignupPopover.test.tsx b/frontends/mit-learn/src/page-components/SignupPopover/SignupPopover.test.tsx new file mode 100644 index 0000000000..f5c6fe4cee --- /dev/null +++ b/frontends/mit-learn/src/page-components/SignupPopover/SignupPopover.test.tsx @@ -0,0 +1,23 @@ +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( + , + { + 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({ + pathname: "/some-path", + search: "?dog=woof", + }), + ) +}) diff --git a/frontends/ol-components/src/components/SignupPopover/SignupPopover.tsx b/frontends/mit-learn/src/page-components/SignupPopover/SignupPopover.tsx similarity index 71% rename from frontends/ol-components/src/components/SignupPopover/SignupPopover.tsx rename to frontends/mit-learn/src/page-components/SignupPopover/SignupPopover.tsx index 5036c91926..64487d1b25 100644 --- a/frontends/ol-components/src/components/SignupPopover/SignupPopover.tsx +++ b/frontends/mit-learn/src/page-components/SignupPopover/SignupPopover.tsx @@ -1,9 +1,8 @@ import React from "react" +import { Popover, Typography, styled, ButtonLink } from "ol-components" import type { PopoverProps } from "ol-components" -import styled from "@emotion/styled" -import { Popover } from "../Popover/Popover" -import Typography from "@mui/material/Typography" -import { ButtonLink } from "../Button/Button" +import * as urls from "@/common/urls" +import { useLocation } from "react-router" const StyledPopover = styled(Popover)({ width: "300px", @@ -26,13 +25,11 @@ const Footer = styled.div({ type SignupPopoverProps = Pick< PopoverProps, "anchorEl" | "onClose" | "placement" -> & { - signupUrl?: string -} +> const SignupPopover: React.FC = (props) => { - const { signupUrl, ...popoverProps } = props + const loc = useLocation() return ( - + Join {APP_SETTINGS.SITE_NAME} for free. @@ -41,7 +38,14 @@ const SignupPopover: React.FC = (props) => { and follow your areas of interest.
- Sign Up + + Sign Up +
) diff --git a/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx b/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx index 2393f428fa..2ebd9264a7 100644 --- a/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx +++ b/frontends/ol-components/src/components/LearningResourceExpanded/InfoSection.tsx @@ -1,4 +1,4 @@ -import React, { useState } from "react" +import React from "react" import styled from "@emotion/styled" import ISO6391 from "iso-639-1" import { @@ -26,7 +26,6 @@ import Typography from "@mui/material/Typography" import type { User } from "api/hooks/user" import { CardActionButton } from "../LearningResourceCard/LearningResourceListCard" import { LearningResourceCardProps } from "../LearningResourceCard/LearningResourceCard" -import { SignupPopover } from "../SignupPopover/SignupPopover" const InfoItems = styled.section` display: flex; @@ -243,17 +242,13 @@ const InfoSection = ({ user, onAddToLearningPathClick, onAddToUserListClick, - signupUrl, }: { resource?: LearningResource run?: LearningResourceRun user?: User onAddToLearningPathClick?: LearningResourceCardProps["onAddToLearningPathClick"] onAddToUserListClick?: LearningResourceCardProps["onAddToUserListClick"] - signupUrl?: string }) => { - const [buttonEl, setButtonEl] = useState(null) - if (!resource) { return null } @@ -294,19 +289,14 @@ const InfoSection = ({ + onClick={ onAddToUserListClick - ? onAddToUserListClick(event, resource.id) - : setButtonEl(event.currentTarget) + ? (event) => onAddToUserListClick?.(event, resource.id) + : undefined } > - setButtonEl(null)} - /> {infoItems.map((props, index) => ( diff --git a/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpanded.tsx b/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpanded.tsx index 17e645ab36..8949a85719 100644 --- a/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpanded.tsx +++ b/frontends/ol-components/src/components/LearningResourceExpanded/LearningResourceExpanded.tsx @@ -144,7 +144,6 @@ type LearningResourceExpandedProps = { imgConfig: EmbedlyConfig onAddToLearningPathClick?: LearningResourceCardProps["onAddToLearningPathClick"] onAddToUserListClick?: LearningResourceCardProps["onAddToUserListClick"] - signupUrl?: string } const ImageSection: React.FC<{ @@ -324,7 +323,6 @@ const LearningResourceExpanded: React.FC = ({ imgConfig, onAddToLearningPathClick, onAddToUserListClick, - signupUrl, }) => { const [selectedRun, setSelectedRun] = useState(resource?.runs?.[0]) @@ -427,7 +425,6 @@ const LearningResourceExpanded: React.FC = ({ user={user} onAddToLearningPathClick={onAddToLearningPathClick} onAddToUserListClick={onAddToUserListClick} - signupUrl={signupUrl} /> ) diff --git a/frontends/ol-components/src/components/Popover/Popover.tsx b/frontends/ol-components/src/components/Popover/Popover.tsx index b1c7de92ae..48a332c227 100644 --- a/frontends/ol-components/src/components/Popover/Popover.tsx +++ b/frontends/ol-components/src/components/Popover/Popover.tsx @@ -5,14 +5,13 @@ 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" -import { MUI_DRAWER_Z_INDEX } from "../../constants/styleOverrides" /** * 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: MUI_DRAWER_Z_INDEX + 1, + zIndex: theme.zIndex.drawer + 1, "& > div": { position: "relative", }, diff --git a/frontends/ol-components/src/components/SignupPopover/SignupPopover.test.tsx b/frontends/ol-components/src/components/SignupPopover/SignupPopover.test.tsx deleted file mode 100644 index ee23a4d921..0000000000 --- a/frontends/ol-components/src/components/SignupPopover/SignupPopover.test.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import React from "react" -import { SignupPopover } from "./SignupPopover" -import invariant from "tiny-invariant" -import { render, screen, within } from "@testing-library/react" -import { ThemeProvider } from "../ThemeProvider/ThemeProvider" -import { BrowserRouter } from "react-router-dom" - -test("SignupPopover shows link to sign up", async () => { - const signupUrl = "https://example.com/signup" - render( - - - - , - - , - ) - const dialog = screen.getByRole("dialog") - const link = within(dialog).getByRole("link") - invariant(link instanceof HTMLAnchorElement) - expect(link.href).toMatch(signupUrl) -}) diff --git a/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx b/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx index 9ab02b196b..f2bb97970d 100644 --- a/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx +++ b/frontends/ol-components/src/components/ThemeProvider/ThemeProvider.tsx @@ -9,7 +9,6 @@ import * as typography from "./typography" import * as buttons from "./buttons" import * as chips from "./chips" import { colors } from "./colors" -import { MUI_DRAWER_Z_INDEX } from "../../constants/styleOverrides" const shadow = { shadowOffsetX: 3, @@ -81,13 +80,6 @@ const themeOptions: ThemeOptions = { styleOverrides: { paper: { borderRadius: "4px" } }, }, MuiChip: chips.chipComponent, - MuiDrawer: { - styleOverrides: { - paper: { - zIndex: MUI_DRAWER_Z_INDEX, - }, - }, - }, }, } diff --git a/frontends/ol-components/src/constants/styleOverrides.ts b/frontends/ol-components/src/constants/styleOverrides.ts deleted file mode 100644 index 25ba13e46f..0000000000 --- a/frontends/ol-components/src/constants/styleOverrides.ts +++ /dev/null @@ -1,3 +0,0 @@ -const MUI_DRAWER_Z_INDEX = 1200 - -export { MUI_DRAWER_Z_INDEX } diff --git a/frontends/ol-components/src/index.ts b/frontends/ol-components/src/index.ts index d71d04ae61..9c988a0768 100644 --- a/frontends/ol-components/src/index.ts +++ b/frontends/ol-components/src/index.ts @@ -199,8 +199,6 @@ export * from "./constants/imgConfigs" export { Input, AdornmentButton } from "./components/Input/Input" export type { InputProps, AdornmentButtonProps } from "./components/Input/Input" -export { SignupPopover } from "./components/SignupPopover/SignupPopover" -export type { SignupPopoverProps } from "./components/SignupPopover/SignupPopover" export { SearchInput } from "./components/SearchInput/SearchInput" export type { SearchInputProps } from "./components/SearchInput/SearchInput" export { TextField } from "./components/TextField/TextField"