Skip to content

Commit

Permalink
add SignupPopover to learning resource drawer bookmark button (#1565)
Browse files Browse the repository at this point in the history
* move SignupPopover to ol-components, update imports and tests, add to learning resource drawer user list button

* consolidate ResourceCard and ResourceListCard

* instead of hard coded 9999 z index on popover, set z index in the theme to the default mui value for the drawer, then take that value plus one for popover
  • Loading branch information
gumaerc authored Sep 17, 2024
1 parent 80267e7 commit c7ebcf6
Show file tree
Hide file tree
Showing 17 changed files with 166 additions and 150 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
LearningResourceListCard,
LearningResourceListCardCondensed,
} from "ol-components"
import { ResourceListCard } from "@/page-components/ResourceCard/ResourceCard"
import { ResourceCard } from "@/page-components/ResourceCard/ResourceCard"
import { useListItemMove } from "api/hooks/learningResources"

const EmptyMessage = styled.p({
Expand Down Expand Up @@ -153,9 +153,10 @@ const ItemsListing: React.FC<ItemsListingProps> = ({
<StyledPlainList itemSpacing={condensed ? 1 : 2}>
{items.map((item) => (
<li key={item.id}>
<ResourceListCard
<ResourceCard
resource={item.resource}
condensed={condensed}
list
/>
</li>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,27 +105,23 @@ describe("LearningResourceDrawer", () => {
isLearningPathEditor: true,
isAuthenticated: true,
expectAddToLearningPathButton: true,
expectAddToUserListButton: true,
},
{
isLearningPathEditor: false,
isAuthenticated: true,
expectAddToLearningPathButton: false,
expectAddToUserListButton: true,
},
{
isLearningPathEditor: false,
isAuthenticated: false,
expectAddToLearningPathButton: false,
expectAddToUserListButton: false,
},
])(
"Renders info section list buttons correctly",
async ({
isLearningPathEditor,
isAuthenticated,
expectAddToLearningPathButton,
expectAddToUserListButton,
}) => {
const resource = factories.learningResources.resource({
resource_type: ResourceTypeEnum.Course,
Expand Down Expand Up @@ -159,24 +155,14 @@ describe("LearningResourceDrawer", () => {
.closest("section")
invariant(section)

if (!isAuthenticated) {
const buttons = within(section).queryAllByRole("button")
expect(buttons).toHaveLength(0)
return
} else {
const buttons = within(section).getAllByRole("button")
const expectedButtons =
expectAddToLearningPathButton && expectAddToUserListButton ? 2 : 1
expect(buttons).toHaveLength(expectedButtons)
expect(
!!within(section).queryByRole("button", {
name: "Add to Learning Path",
}),
).toBe(expectAddToLearningPathButton)
expect(
!!within(section).queryByRole("button", { name: "Add to User List" }),
).toBe(expectAddToUserListButton)
}
const buttons = within(section).getAllByRole("button")
const expectedButtons = expectAddToLearningPathButton ? 2 : 1
expect(buttons).toHaveLength(expectedButtons)
expect(
!!within(section).queryByRole("button", {
name: "Add to Learning Path",
}),
).toBe(expectAddToLearningPathButton)
},
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
AddToLearningPathDialog,
AddToUserListDialog,
} from "../Dialogs/AddToListDialog"
import * as urls from "@/common/urls"

const RESOURCE_DRAWER_PARAMS = [RESOURCE_DRAWER_QUERY_PARAM] as const

Expand Down Expand Up @@ -64,6 +65,7 @@ const unsafe_html2plaintext = (text: string) => {
const DrawerContent: React.FC<{
resourceId: number
}> = ({ resourceId }) => {
const loc = useLocation()
const resource = useLearningResourcesDetail(Number(resourceId))
const { data: user } = useUserMe()
const handleAddToLearningPathClick: LearningResourceCardProps["onAddToLearningPathClick"] =
Expand Down Expand Up @@ -101,6 +103,10 @@ const DrawerContent: React.FC<{
user={user}
onAddToLearningPathClick={handleAddToLearningPathClick}
onAddToUserListClick={handleAddToUserListClick}
signupUrl={urls.login({
pathname: loc.pathname,
search: loc.search,
})}
/>
</>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
expectProps,
} from "../../test-utils"
import type { User } from "../../test-utils"
import { ResourceCard, ResourceListCard } from "./ResourceCard"
import { ResourceCard } from "./ResourceCard"
import {
AddToLearningPathDialog,
AddToUserListDialog,
Expand Down Expand Up @@ -39,14 +39,14 @@ jest.mock("@ebay/nice-modal-react", () => {

describe.each([
{
CardComponent: ResourceCard,
BaseComponent: LearningResourceCard,
isList: false,
},
{
CardComponent: ResourceListCard,
BaseComponent: LearningResourceListCard,
isList: true,
},
])("$CardComponent", ({ CardComponent, BaseComponent }) => {
])("$CardComponent", ({ BaseComponent, isList }) => {
const makeResource = factories.learningResources.resource
type SetupOptions = {
user?: Partial<User>
Expand All @@ -60,7 +60,7 @@ describe.each([
setMockResponse.get(urls.userMe.get(), {}, { code: 403 })
}
const { view, location } = renderWithProviders(
<CardComponent {...props} resource={resource} />,
<ResourceCard {...props} resource={resource} list={isList} />,
)
return { resource, view, location }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,19 @@ import {
LearningResourceCard,
LearningResourceListCard,
LearningResourceListCardCondensed,
SignupPopover,
} from "ol-components"
import * as NiceModal from "@ebay/nice-modal-react"
import type {
LearningResourceCardProps,
LearningResourceListCardProps,
} from "ol-components"
import type { LearningResourceCardProps } 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"
import { LearningResource } from "api"
import * as urls from "@/common/urls"
import { useLocation } from "react-router"

const useResourceCard = (resource?: LearningResource | null) => {
const getDrawerHref = useResourceDrawerHref()
Expand Down Expand Up @@ -70,60 +69,25 @@ const useResourceCard = (resource?: LearningResource | null) => {
type ResourceCardProps = Omit<
LearningResourceCardProps,
"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<ResourceCardProps> = ({ resource, ...others }) => {
const {
getDrawerHref,
anchorEl,
handleClosePopover,
handleAddToLearningPathClick,
handleAddToUserListClick,
inUserList,
inLearningPath,
} = useResourceCard(resource)
return (
<>
<LearningResourceCard
resource={resource}
href={resource ? getDrawerHref(resource.id) : undefined}
onAddToLearningPathClick={handleAddToLearningPathClick}
onAddToUserListClick={handleAddToUserListClick}
inUserList={inUserList}
inLearningPath={inLearningPath}
{...others}
/>
<SignupPopover anchorEl={anchorEl} onClose={handleClosePopover} />
</>
)
}

type ResourceListCardProps = Omit<
LearningResourceListCardProps,
"href" | "onAddToLearningPathClick" | "onAddToUserListClick"
> & {
condensed?: boolean
list?: boolean
}

/**
* Just like `ol-components/LearningResourceListCard`, but with builtin actions:
* 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 ResourceListCard: React.FC<ResourceListCardProps> = ({
const ResourceCard: React.FC<ResourceCardProps> = ({
resource,
condensed,
...props
list,
...others
}) => {
const loc = useLocation()
const {
getDrawerHref,
anchorEl,
Expand All @@ -133,25 +97,34 @@ const ResourceListCard: React.FC<ResourceListCardProps> = ({
inUserList,
inLearningPath,
} = useResourceCard(resource)

const ListCardComponent = condensed
? LearningResourceListCardCondensed
: LearningResourceListCard
const CardComponent =
list && condensed
? LearningResourceListCardCondensed
: list
? LearningResourceListCard
: LearningResourceCard
return (
<>
<ListCardComponent
<CardComponent
resource={resource}
href={resource ? getDrawerHref(resource.id) : undefined}
onAddToLearningPathClick={handleAddToLearningPathClick}
onAddToUserListClick={handleAddToUserListClick}
inUserList={inUserList}
inLearningPath={inLearningPath}
{...props}
{...others}
/>
<SignupPopover
signupUrl={urls.login({
pathname: loc.pathname,
search: loc.search,
})}
anchorEl={anchorEl}
onClose={handleClosePopover}
/>
<SignupPopover anchorEl={anchorEl} onClose={handleClosePopover} />
</>
)
}

export { ResourceCard, ResourceListCard }
export type { ResourceCardProps, ResourceListCardProps }
export { ResourceCard }
export type { ResourceCardProps }
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import SliderInput from "./SliderInput"

import type { TabConfig } from "./ResourceCategoryTabs"

import { ResourceListCard } from "../ResourceCard/ResourceCard"
import { ResourceCard } from "../ResourceCard/ResourceCard"
import { useSearchParams } from "@mitodl/course-search-utils/react-router"
import { useUserMe } from "api/hooks/user"

Expand Down Expand Up @@ -855,15 +855,15 @@ const SearchDisplay: React.FC<SearchDisplayProps> = ({
.fill(null)
.map((a, index) => (
<li key={index}>
<ResourceListCard isLoading={isLoading} />
<ResourceCard isLoading={isLoading} list />
</li>
))}
</PlainList>
) : data && data.count > 0 ? (
<PlainList itemSpacing={1.5}>
{data.results.map((resource) => (
<li key={resource.id}>
<ResourceListCard resource={resource} />
<ResourceCard resource={resource} list />
</li>
))}
</PlainList>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import {
useSearchSubscriptionDelete,
useSearchSubscriptionList,
} from "api/hooks/searchSubscription"
import { Button, SimpleMenu, styled } from "ol-components"
import { Button, SignupPopover, 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 { SignupPopover } from "../SignupPopover/SignupPopover"
import * as urls from "@/common/urls"
import { useLocation } from "react-router"

const StyledButton = styled(Button)({
minWidth: "130px",
Expand All @@ -26,6 +27,7 @@ const SearchSubscriptionToggle: React.FC<SearchSubscriptionToggleProps> = ({
searchParams,
sourceType,
}) => {
const loc = useLocation()
const [buttonEl, setButtonEl] = useState<null | HTMLElement>(null)

const subscribeParams: Record<string, string[] | string> = useMemo(() => {
Expand Down Expand Up @@ -90,7 +92,14 @@ const SearchSubscriptionToggle: React.FC<SearchSubscriptionToggleProps> = ({
>
Follow
</StyledButton>
<SignupPopover anchorEl={buttonEl} onClose={() => setButtonEl(null)} />
<SignupPopover
signupUrl={urls.login({
pathname: loc.pathname,
search: loc.search,
})}
anchorEl={buttonEl}
onClose={() => setButtonEl(null)}
/>
</>
)
}
Expand Down

This file was deleted.

Loading

0 comments on commit c7ebcf6

Please sign in to comment.