Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there is a lot of redundancy between ResourceCard and ResourceListCard that could potentially be abstracted/consolidated.

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