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

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%",
   },
 }))

frontends/mit-open/src/page-components/SearchSubscriptionToggle/SearchSubscriptionToggle.tsx

@@ -5,14 +5,18 @@ 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"
 import { SourceTypeEnum } from "api"
 
 import { SignupPopover } from "../SignupPopover/SignupPopover"
 
+const StyledButton = styled(Button)({
+  minWidth: "130px",
+})
+
 type SearchSubscriptionToggleProps = {
   searchParams: URLSearchParams
   sourceType: SourceTypeEnum
@@ -54,17 +58,17 @@ const SearchSubscriptionToggle: React.FC<SearchSubscriptionToggleProps> = ({
     return (
       <SimpleMenu
         trigger={
-          <Button variant="primary" endIcon={<RiArrowDownSLine />}>
+          <StyledButton variant="primary" endIcon={<RiArrowDownSLine />}>
             Following
-          </Button>
+          </StyledButton>
         }
         items={unsubscribeItems}
       />
     )
   }
   return (
     <>
-      <Button
+      <StyledButton
         variant="primary"
         disabled={subscriptionCreate.isLoading}
         startIcon={<RiMailLine />}
@@ -79,7 +83,7 @@ const SearchSubscriptionToggle: React.FC<SearchSubscriptionToggleProps> = ({
         }}
       >
         Follow
-      </Button>
+      </StyledButton>
       <SignupPopover anchorEl={buttonEl} onClose={() => setButtonEl(null)} />
     </>
   )

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<Channel>,
   search?: Partial<LearningResourcesSearchResponse>,
@@ -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<HTMLImageElement>("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<HTMLImageElement>("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,
+      )
+    })
+  })
+})