From a7ffb08b0985898a184d2b854cf1ed305d3b0e4a Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 17 Jul 2024 12:58:58 -0400 Subject: [PATCH] Unit Detail Banner Updates (#1272) * adjust comment, show more icons * 8px icon-text gap * make button wider * unit page header styling * fix tests * fix image heights --- .../ChannelDetails/ChannelDetails.tsx | 1 + .../SearchSubscriptionToggle.tsx | 14 +- .../pages/ChannelPage/ChannelPage.test.tsx | 143 +++++++------ .../pages/ChannelPage/UnitChannelTemplate.tsx | 193 +++++++++--------- .../src/components/Banner/Banner.tsx | 28 +-- .../src/components/Button/Button.stories.tsx | 69 +++---- .../src/components/Button/Button.tsx | 17 +- frontends/ol-components/src/index.ts | 7 +- 8 files changed, 236 insertions(+), 236 deletions(-) diff --git a/frontends/mit-open/src/page-components/ChannelDetails/ChannelDetails.tsx b/frontends/mit-open/src/page-components/ChannelDetails/ChannelDetails.tsx index 27dee8fa8e..407a888409 100644 --- a/frontends/mit-open/src/page-components/ChannelDetails/ChannelDetails.tsx +++ b/frontends/mit-open/src/page-components/ChannelDetails/ChannelDetails.tsx @@ -152,6 +152,7 @@ const ChannelDetailsCard = styled(Box)(({ theme }) => ({ }, [theme.breakpoints.down("md")]: { padding: "16px", + marginTop: "16px", width: "100%", }, })) diff --git a/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx b/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx index 1415155a97..192449bc09 100644 --- a/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx +++ b/frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx @@ -5,7 +5,7 @@ import { useSearchSubscriptionDelete, useSearchSubscriptionList, } from "api/hooks/searchSubscription" -import { Button, SimpleMenu } 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" @@ -13,6 +13,10 @@ import { SourceTypeEnum } from "api" import { SignupPopover } from "../SignupPopover/SignupPopover" +const StyledButton = styled(Button)({ + minWidth: "130px", +}) + type SearchSubscriptionToggleProps = { searchParams: URLSearchParams sourceType: SourceTypeEnum @@ -54,9 +58,9 @@ const SearchSubscriptionToggle: React.FC = ({ return ( }> + }> Following - + } items={unsubscribeItems} /> @@ -64,7 +68,7 @@ const SearchSubscriptionToggle: React.FC = ({ } return ( <> - + setButtonEl(null)} /> ) diff --git a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx index 842d586e2d..176ebbac08 100644 --- a/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx +++ b/frontends/mit-open/src/pages/ChannelPage/ChannelPage.test.tsx @@ -1,6 +1,5 @@ -import { assertInstanceOf } from "ol-utilities" import { urls, factories, makeRequest } from "api/test-utils" -import type { Channel } from "api/v0" +import { ChannelTypeEnum, type Channel } from "api/v0" import type { LearningResourcesSearchResponse } from "api" import { renderTestApp, @@ -20,6 +19,15 @@ jest.mock("./ChannelSearch", () => { }) const mockedChannelSearch = jest.mocked(ChannelSearch) +const someAncestor = (el: HTMLElement, cb: (el: HTMLElement) => boolean) => { + let ancestor = el.parentElement + while (ancestor) { + if (cb(ancestor)) return true + ancestor = ancestor.parentElement + } + return false +} + const setupApis = ( channelPatch?: Partial, search?: Partial, @@ -100,73 +108,35 @@ const setupApis = ( } describe("ChannelPage", () => { - it("Displays the channel title, banner, and avatar", async () => { - const { channel } = setupApis({ - search_filter: "offered_by=ocw", - channel_type: "unit", - }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - const findTitle = (titles: HTMLElement[]) => { - return titles[ - titles.findIndex( - (title: HTMLElement) => - title.textContent === channel.title || - title.textContent === channel.configuration.heading, - ) - ] - } - await waitFor(() => { - const titles = screen.getAllByRole("heading") - const title = findTitle(titles) - expect(title).toBeInTheDocument() - }) - const titles = screen.getAllByRole("heading") - const title = findTitle(titles) - expect(title).toBeInTheDocument() - const header = title.closest("header") - assertInstanceOf(header, HTMLElement) - const images = within(header).getAllByRole("img") as HTMLImageElement[] - const headerStyles = getComputedStyle(header) - if (channel.channel_type !== "unit") { - /* - * unit channels are filtered out from this assertion - * because they wrap the background image in a linear-gradient, - * which causes react testing library to not render the background-image - * property at all - */ - expect(headerStyles.backgroundImage).toContain( - channel.configuration.banner_background, - ) - } - expect(images[0].src).toContain(channel.configuration.logo) - }) - it("Displays a featured carousel if the channel type is 'unit'", async () => { - const { channel } = setupApis({ - search_filter: "offered_by=ocw", - channel_type: "unit", - }) - - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - await screen.findAllByText(channel.title) - const carousel = await screen.findByText("Featured Courses") - expect(carousel).toBeInTheDocument() + it.each( + Object.values(ChannelTypeEnum).filter((v) => v !== ChannelTypeEnum.Unit), + )( + "Displays the title, background, and avatar (channelType: %s)", + async (channelType) => { + const { channel } = setupApis({ + search_filter: "offered_by=ocw", + channel_type: channelType, + }) - await waitFor(() => { - expect(makeRequest).toHaveBeenCalledWith( - "get", - urls.learningResources.featured({ limit: 12, offered_by: ["ocw"] }), - undefined, + renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + const title = await screen.findByRole("heading", { name: channel.title }) + // Banner background image + expect( + someAncestor(title, (el) => + window + .getComputedStyle(el) + .backgroundImage.includes(channel.configuration.banner_background), + ), + ).toBe(true) + // logo + const images = screen.getAllByRole("img") + const logos = images.filter((img) => + img.src.includes(channel.configuration.logo), ) - }) + expect(logos.length).toBe(1) + }, + ) - await waitFor(() => { - expect(makeRequest).toHaveBeenCalledWith( - "get", - urls.learningResources.featured({ limit: 12 }), - undefined, - ) - }) - }) it("Does not display a featured carousel if the channel type is not 'unit'", async () => { const { channel } = setupApis({ search_filter: "topic=physics", @@ -239,3 +209,44 @@ describe("ChannelPage", () => { }, ) }) + +describe("Unit Channel Pages", () => { + it("Displays the channel title, banner, and avatar", async () => { + const { channel } = setupApis({ + search_filter: "offered_by=ocw", + channel_type: "unit", + }) + renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + + const title = await screen.findByRole("heading", { name: channel.title }) + const image = within(title).getByRole("img") + expect(image.src).toContain(channel.configuration.logo) + }) + it("Displays a featured carousel if the channel type is 'unit'", async () => { + const { channel } = setupApis({ + search_filter: "offered_by=ocw", + channel_type: "unit", + }) + + renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + await screen.findAllByText(channel.title) + const carousel = await screen.findByText("Featured Courses") + expect(carousel).toBeInTheDocument() + + await waitFor(() => { + expect(makeRequest).toHaveBeenCalledWith( + "get", + urls.learningResources.featured({ limit: 12, offered_by: ["ocw"] }), + undefined, + ) + }) + + await waitFor(() => { + expect(makeRequest).toHaveBeenCalledWith( + "get", + urls.learningResources.featured({ limit: 12 }), + undefined, + ) + }) + }) +}) diff --git a/frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx b/frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx index 7e200a0dd9..9f11cddc8a 100644 --- a/frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx +++ b/frontends/mit-open/src/pages/ChannelPage/UnitChannelTemplate.tsx @@ -1,11 +1,18 @@ import React, { useMemo } from "react" -import { styled, Container, Box, Breadcrumbs, Banner } from "ol-components" +import { + styled, + Container, + Box, + Breadcrumbs, + Stack, + BannerBackground, + Typography, +} from "ol-components" import { MetaTags } from "ol-utilities" import { SearchSubscriptionToggle } from "@/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle" import { ChannelDetails } from "@/page-components/ChannelDetails/ChannelDetails" import { useChannelDetail } from "api/hooks/channels" import ChannelMenu from "@/components/ChannelMenu/ChannelMenu" -import ChannelAvatar from "@/components/ChannelAvatar/ChannelAvatar" import ResourceCarousel, { ResourceCarouselProps, } from "@/page-components/ResourceCarousel/ResourceCarousel" @@ -15,6 +22,13 @@ import { HOME as HOME_URL, UNITS as UNITS_URL } from "../../common/urls" import { ChannelTypeEnum } from "api/v0" import { ChannelControls, UNITS_LABEL } from "./ChannelPageTemplate" +const StyledBannerBackground = styled(BannerBackground)(({ theme }) => ({ + padding: "48px 0 64px 0", + [theme.breakpoints.down("md")]: { + padding: "32px 0 16px 0", + }, +})) + const FeaturedCoursesCarousel = styled(ResourceCarousel)(({ theme }) => ({ margin: "80px 0", [theme.breakpoints.down("sm")]: { @@ -23,6 +37,37 @@ const FeaturedCoursesCarousel = styled(ResourceCarousel)(({ theme }) => ({ }, })) +const UnitLogo = styled.img(({ theme }) => ({ + filter: "saturate(0%) invert(100%)", + maxWidth: "100%", + width: "auto", + height: "50px", + [theme.breakpoints.down("md")]: { + height: "40px", + }, +})) + +const BannerContent = styled.div(({ theme }) => ({ + display: "flex", + flexDirection: "row", + alignItems: "center", + justifyContent: "space-between", + [theme.breakpoints.down("md")]: { + flexDirection: "column", + }, +})) + +const ChannelHeader = styled.h1({ + margin: 0, + display: "flex", + flexGrow: 1, + flexShrink: 0, +}) + +const HEADER_STYLES = { + width: { md: "80%", sm: "100%" }, +} + interface UnitChannelTemplateProps { children: React.ReactNode name: string @@ -57,23 +102,18 @@ const UnitChannelTemplate: React.FC = ({ }, ] - const headerStyles = { - width: { md: "80%", sm: "100%" }, - my: 1, - } - return ( <> - + = ({ ]} current={channel.data?.title} /> - } - avatar={ - displayConfiguration?.logo && channel.data ? ( - ({ - flexGrow: 1, - flexShrink: 0, - order: 1, - py: "24px", - - [theme.breakpoints.down("md")]: { - py: 0, - pb: "8px", - }, - [theme.breakpoints.down("sm")]: { - pt: "32px", + + + + {channel.data ? ( + + ) : null} + + + + {displayConfiguration?.heading} + + + {displayConfiguration?.sub_heading} + + + - - - ) : null - } - header={displayConfiguration?.heading} - headerTypography={{ xs: "h5", md: "h4" }} - headerStyles={headerStyles} - subheader={displayConfiguration?.sub_heading} - subheaderStyles={headerStyles} - subheaderTypography={{ xs: "body2", md: "body1" }} - extraHeader={ - - - {channel.data?.search_filter ? ( - - ) : null} - {channel.data?.is_moderator ? ( - - ) : null} - - - } - extraRight={ - channel.data && ( - - - - ) - } - /> + flexShrink: 1, + }} + > + + {channel.data?.search_filter ? ( + + ) : null} + {channel.data?.is_moderator ? ( + + ) : null} + + + + {channel.data ? : null} + + + ( - ({ - theme, - backgroundUrl, - backgroundSize = "cover", - backgroundDim = 0, - containerPadding = "48px 0 48px 0", - }) => ({ +const BannerBackground = styled.div( + ({ theme, backgroundUrl, backgroundSize = "cover", backgroundDim = 0 }) => ({ backgroundAttachment: "fixed", backgroundImage: backgroundDim ? `linear-gradient(rgba(0 0 0 / ${backgroundDim}%), rgba(0 0 0 / ${backgroundDim}%)), url('${backgroundUrl}')` @@ -35,7 +28,7 @@ const BannerWrapper = styled.header( backgroundSize: backgroundSize, backgroundRepeat: "no-repeat", color: theme.custom.colors.white, - padding: containerPadding, + padding: "48px 0 48px 0", [theme.breakpoints.down("sm")]: { padding: "32px 0 32px 0", }, @@ -65,11 +58,10 @@ const RightContainer = styled.div(({ theme }) => ({ }, })) -type BannerProps = BannerWrapperProps & { +type BannerProps = BannerBackgroundProps & { backgroundUrl: string backgroundSize?: string backgroundDim?: number - containerPadding?: string navText: React.ReactNode avatar?: React.ReactNode header: React.ReactNode @@ -90,7 +82,6 @@ const Banner = ({ backgroundUrl, backgroundSize = "cover", backgroundDim = 0, - containerPadding = "48px 0 48px 0", navText, avatar, header, @@ -105,11 +96,10 @@ const Banner = ({ const defaultHeaderTypography = { xs: "h2", md: "h1" } const defaultSubHeaderTypography = { xs: "body2", md: "body1" } return ( - {navText} @@ -135,9 +125,9 @@ const Banner = ({ {extraRight} - + ) } -export { Banner } -export type { BannerProps } +export { Banner, BannerBackground } +export type { BannerProps, BannerBackgroundProps } diff --git a/frontends/ol-components/src/components/Button/Button.stories.tsx b/frontends/ol-components/src/components/Button/Button.stories.tsx index da21b11e90..9475b92d28 100644 --- a/frontends/ol-components/src/components/Button/Button.stories.tsx +++ b/frontends/ol-components/src/components/Button/Button.stories.tsx @@ -8,18 +8,22 @@ import { RiArrowRightLine, RiDeleteBinLine, RiEditLine, + RiTestTubeLine, + RiMailLine, } from "@remixicon/react" import { withRouter } from "storybook-addon-react-router-v6" import { fn } from "@storybook/test" import { capitalize } from "ol-utilities" -const icons = { +const ICONS = { None: undefined, ArrowForwardIcon: , ArrowBackIcon: , DeleteIcon: , EditIcon: , + TestTubeIcon: , + MailIcon: , } const meta: Meta = { @@ -46,12 +50,12 @@ const meta: Meta = { control: { type: "select" }, }, startIcon: { - options: Object.keys(icons), - mapping: icons, + options: Object.keys(ICONS), + mapping: ICONS, }, endIcon: { - options: Object.keys(icons), - mapping: icons, + options: Object.keys(ICONS), + mapping: ICONS, }, }, args: { @@ -184,19 +188,12 @@ export const EdgeStory: Story = { export const WithIconStory: Story = { render: (args) => ( - - - - - + + {Object.entries(ICONS).map(([key, icon]) => ( + + ))} ), } @@ -225,8 +222,16 @@ const EDGES = ["rounded", "circular", "none"] as const const VARIANTS = ["primary", "secondary", "tertiary", "text"] as const const EXTRA_PROPS = [ {}, - { startIcon: }, - { endIcon: }, + /** + * Show RiTestTubeLine because it is a fairly thin icon + */ + { startIcon: }, + /** + * Show RiTestTubeLine because it is a fairly thick icon + */ + { startIcon: }, + { endIcon: }, + { endIcon: }, ] export const LinkStory: Story = { @@ -302,24 +307,6 @@ export const WrappingButtonShowcase: Story = { }, } -const ICONS = [ - { - component: , - key: "back", - }, - { - component: , - key: "delete", - }, - { - component: , - key: "edit", - }, - { - component: , - key: "forward", - }, -] export const ActionButtonsShowcase: Story = { render: (args) => ( <> @@ -339,15 +326,15 @@ export const ActionButtonsShowcase: Story = { {SIZES.map((size) => ( - {ICONS.map((icon) => ( + {Object.entries(ICONS).map(([key, icon]) => ( - {icon.component} + {icon} ))} diff --git a/frontends/ol-components/src/components/Button/Button.tsx b/frontends/ol-components/src/components/Button/Button.tsx index a4e3f7f124..1fd5275b64 100644 --- a/frontends/ol-components/src/components/Button/Button.tsx +++ b/frontends/ol-components/src/components/Button/Button.tsx @@ -195,14 +195,6 @@ const ButtonStyled = styled.button((props) => { ] }) -/** - * Typically, SVG Icons are structured as ` `; the svg is - * 24x24px by default, but the path may be smaller. I.e., there is generally - * whitespace around the path. - * - * The negative margin below accounts (approximately) for this whitespace. - * The whitespace varies by icon, so it's not perfect. - */ const IconContainer = styled.span<{ side: "start" | "end"; size: ButtonSize }>( ({ size, side }) => [ { @@ -211,11 +203,16 @@ const IconContainer = styled.span<{ side: "start" | "end"; size: ButtonSize }>( alignItems: "center", }, side === "start" && { + /** + * The negative margin is to counteract the padding on the button itself. + * Without icons, the left space is 24/16/12 px. + * With icons, the left space is 20/12/8 px. + */ marginLeft: "-4px", - marginRight: "4px", + marginRight: "8px", }, side === "end" && { - marginLeft: "4px", + marginLeft: "8px", marginRight: "-4px", }, { diff --git a/frontends/ol-components/src/index.ts b/frontends/ol-components/src/index.ts index 0772333673..f6d2b2d87e 100644 --- a/frontends/ol-components/src/index.ts +++ b/frontends/ol-components/src/index.ts @@ -19,8 +19,11 @@ export type { BadgeProps } from "@mui/material/Badge" export { default as AppBar } from "@mui/material/AppBar" export type { AppBarProps } from "@mui/material/AppBar" -export { Banner } from "./components/Banner/Banner" -export type { BannerProps } from "./components/Banner/Banner" +export { Banner, BannerBackground } from "./components/Banner/Banner" +export type { + BannerProps, + BannerBackgroundProps, +} from "./components/Banner/Banner" export { Button,