Merge pull request #352 from NYPL/main

Merge main into qa
NYPL · Oct 7, 2024 · 014829d · 014829d
2 parents c6e8d57 + 010e96a
commit 014829d
Show file tree

Hide file tree

Showing 10 changed files with 117 additions and 98 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -5,7 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-### Hotfix
+## [1.3.2] 2024-10-7
+
+### Fixed
+
+- Adjust ItemTable styles according to design (SCC-4299)
+- Remove border and padding from eResources link as part of post-Bib page launch VQA (SCC-4299)
+- Remove base url from finding aid urls (SCC-4306)
+- Fix incorrect subject heading urls in Bib Details (SCC-4305)
+
+### [1.3.1] Hotfix
 
 - Fix Table column widths on Search Results after DS 3.4.0 update (SCC-4299)
 

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "research-catalog",
-  "version": "1.2.1",
+  "version": "1.3.2",
   "scripts": {
     "dev": "next dev -p 8080",
     "build": "next build",

diff --git a/src/components/BibPage/BibDetail.tsx b/src/components/BibPage/BibDetail.tsx
@@ -7,7 +7,7 @@ import RCLink from "../Links/RCLink/RCLink"
 import ExternalLink from "../Links/ExternalLink/ExternalLink"
 import type {
   BibDetail,
-  Url,
+  BibDetailURL,
   LinkedBibDetail,
   SubjectHeadingDetail,
   AnyBibDetail,
@@ -77,7 +77,7 @@ const PlainTextElement = (field: BibDetail) => {
 
 const CompoundSubjectHeadingElement = (field: SubjectHeadingDetail) => {
   const subjectHeadingLinksPerSubject = field.value.map(
-    (subjectHeadingUrls: Url[]) => {
+    (subjectHeadingUrls: BibDetailURL[]) => {
       return SingleSubjectHeadingElement(subjectHeadingUrls)
     }
   )
@@ -89,27 +89,30 @@ const CompoundSubjectHeadingElement = (field: SubjectHeadingDetail) => {
   return DetailElement(field.label, values)
 }
 
-const SingleSubjectHeadingElement = (subjectHeadingUrls: Url[]) => {
-  return subjectHeadingUrls.reduce((linksPerSubject, url: Url, index) => {
-    const divider = (
-      // this span will render as > in between the divided subject heading links
-      <span data-testid="divider" key={`divider-${index}`}>
-        {" "}
-        &gt;{" "}
-      </span>
-    )
-    const link = LinkElement(url, "internal")
-    linksPerSubject.push(link)
-    if (!isItTheLastElement(index, subjectHeadingUrls)) {
-      linksPerSubject.push(divider)
-    }
-    return linksPerSubject
-  }, [] as ReactElement[])
+const SingleSubjectHeadingElement = (subjectHeadingUrls: BibDetailURL[]) => {
+  return subjectHeadingUrls.reduce(
+    (linksPerSubject, url: BibDetailURL, index) => {
+      const divider = (
+        // this span will render as > in between the divided subject heading links
+        <span data-testid="divider" key={`divider-${index}`}>
+          {" "}
+          &gt;{" "}
+        </span>
+      )
+      const link = LinkElement(url, "internal")
+      linksPerSubject.push(link)
+      if (!isItTheLastElement(index, subjectHeadingUrls)) {
+        linksPerSubject.push(divider)
+      }
+      return linksPerSubject
+    },
+    [] as ReactElement[]
+  )
 }
 
 const LinkedDetailElement = (field: LinkedBibDetail) => {
   const internalOrExternal = field.link
-  const values = field.value.map((urlInfo: Url, i) => {
+  const values = field.value.map((urlInfo: BibDetailURL, i) => {
     return (
       <li key={`${kebabCase(field.label)}-${i}`}>
         {LinkElement(urlInfo, internalOrExternal)}
@@ -119,17 +122,18 @@ const LinkedDetailElement = (field: LinkedBibDetail) => {
   return DetailElement(field.label, values)
 }
 
-const LinkElement = (url: Url, linkType: string) => {
+const LinkElement = (url: BibDetailURL, linkType: string) => {
   let Link: typeof RCLink | typeof ExternalLink
   if (linkType === "internal") Link = RCLink
   else if (linkType === "external") Link = ExternalLink
   const stringDirection = rtlOrLtr(url.urlLabel)
   return (
     <Link
       dir={stringDirection}
-      href={BASE_URL + url.url}
+      href={url.url}
       key={url.url}
-      includeBaseUrl={false}
+      // external link does not include this prop
+      includeBaseUrl={true}
       textDecoration="none"
     >
       {url.urlLabel}

diff --git a/src/components/ItemTable/ItemTable.tsx b/src/components/ItemTable/ItemTable.tsx
@@ -1,8 +1,4 @@
-import {
-  Box,
-  Table,
-  useNYPLBreakpoints,
-} from "@nypl/design-system-react-components"
+import { Box, Table } from "@nypl/design-system-react-components"
 
 import type ItemTableData from "../../models/ItemTableData"
 import StatusLinks from "./StatusLinks"
@@ -17,7 +13,6 @@ interface ItemTableProps {
  */
 const ItemTable = ({ itemTableData }: ItemTableProps) => {
   const { tableHeadings, tableData, items, inSearchResult } = itemTableData
-  const { isLargerThanMobile } = useNYPLBreakpoints()
 
   return (
     // Display as grid to prevent bug where the outer container stretches to the Table's width on mobile
@@ -27,26 +22,26 @@ const ItemTable = ({ itemTableData }: ItemTableProps) => {
           inSearchResult ? " " + styles.inSearchResult : ""
         }`}
         columnHeaders={tableHeadings}
-        // TODO: Review these values with the design team
+        tableTextSize="body2"
         columnStyles={
           inSearchResult
             ? [
-                { width: "33.3%", minWidth: 150, maxWidth: 250 },
-                { width: "33.3%", minWidth: 150, maxWidth: 250 },
-                { width: "33.3%", minWidth: 150, maxWidth: 250 },
+                { width: 272, minWidth: 85 },
+                { width: 272, minWidth: 85 },
+                { width: 272, minWidth: 85 },
               ]
             : [
-                { minWidth: 350, maxwidth: 350 },
-                { minwidth: 150, maxWidth: 200 },
-                { minwidth: 150, maxWidth: 150 },
-                { minwidth: 150, maxWidth: 150 },
-                { minwidth: 200, maxWidth: 250 },
-                { minwidth: 150, maxWidth: 200 },
+                { width: "auto", minWidth: 250 },
+                { width: "14%", minWidth: 100 },
+                { width: "14%", minWidth: 100 },
+                { width: "14%", minWidth: 100 },
+                { width: "14%", minWidth: 100 },
+                { width: "14%", minWidth: 100 },
               ]
         }
         tableData={tableData}
         showRowDividers={!inSearchResult}
-        isScrollable={!isLargerThanMobile}
+        isScrollable={true}
         my={{ base: inSearchResult ? "s" : 0, md: "s" }}
         data-testid={
           !inSearchResult

diff --git a/src/components/SearchResults/ElectronicResourcesLink.tsx b/src/components/SearchResults/ElectronicResourcesLink.tsx
@@ -17,14 +17,7 @@ const ElectronicResourcesLink = ({
   electronicResources,
 }: ElectronicResourcesLinkProps) => {
   return (
-    <Box
-      mt="s"
-      sx={{
-        border: "1px solid var(--nypl-colors-ui-border-default)",
-        padding: "s",
-      }}
-      data-testid="electronic-resources-link"
-    >
+    <Box mt="s" data-testid="electronic-resources-link">
       <Text
         mb="xxs"
         fontSize={{ base: "mobile.body.body1", md: "desktop.body.body1" }}

diff --git a/src/components/SearchResults/SearchResult.tsx b/src/components/SearchResults/SearchResult.tsx
@@ -38,7 +38,9 @@ const SearchResult = ({ bib }: SearchResultProps) => {
       </CardHeading>
       <CardContent>
         <Box
-          sx={{ p: { display: "inline", marginRight: "s", marginBottom: "s" } }}
+          sx={{
+            p: { display: "inline-block", marginRight: "s", marginBottom: "s" },
+          }}
         >
           {bib.materialType && <Text>{bib.materialType}</Text>}
           {bib.publicationStatement && <Text>{bib.publicationStatement}</Text>}

diff --git a/src/models/BibDetails.ts b/src/models/BibDetails.ts
@@ -4,7 +4,7 @@ import type {
   BibDetail,
   FieldMapping,
   AnnotatedMarcField,
-  Url,
+  BibDetailURL,
   SubjectHeadingDetail,
   AnnotatedMarc,
   AnyBibDetail,
@@ -207,7 +207,10 @@ export default class BibDetails {
     }
   }
 
-  buildExternalLinkedDetail(label: string, values: Url[]): LinkedBibDetail {
+  buildExternalLinkedDetail(
+    label: string,
+    values: BibDetailURL[]
+  ): LinkedBibDetail {
     if (!values.length) return null
     return {
       link: "external",
@@ -365,32 +368,16 @@ export default class BibDetails {
 
   buildSubjectHeadings(): SubjectHeadingDetail {
     if (!this.bib.subjectHeadings) return
-    const subjectHeadingsUrls = this.bib.subjectHeadings.map(
-      ({ label, uuid }) => {
-        if (!label || !uuid) return
-        const subject = label.replace(/\.$/, "")
-        // stackedSubjectHeadings: ["a", "a -- b", "a -- b -- c"]
-        const stackedSubjectHeadings =
-          this.constructSubjectHeadingsArray(subject)
-        const shepUrl = `/subject_headings/${uuid}`
-        // splitSubjectHeadings: ["a", "b", "c"]
-        const splitSubjectHeadings = subject.split(" -- ")
+    const subjectHeadingsUrls = this.bib.subjectHeadings.map((heading) =>
+      this.flattenSubjectHeadingUrls(heading)
+    )
 
-        return splitSubjectHeadings.map((heading, index) => {
-          const urlWithLabelParam = `${shepUrl}?label=${encodeURI(
-            stackedSubjectHeadings[index]
-          )}`
-          return {
-            url: urlWithLabelParam,
-            urlLabel: heading,
-          }
-        })
+    return (
+      subjectHeadingsUrls?.length && {
+        label: "Subject",
+        value: subjectHeadingsUrls,
       }
     )
-    return {
-      label: "Subject",
-      value: subjectHeadingsUrls,
-    }
   }
 
   buildSubjectLiterals(): SubjectHeadingDetail {
@@ -400,7 +387,7 @@ export default class BibDetails {
         subject = subject.replace(/\.$/, "")
         // stackedSubjectHeadings: ["a", "a -- b", "a -- b -- c"]
         const stackedSubjectHeadings =
-          this.constructSubjectHeadingsArray(subject)
+          this.constructSubjectLiteralsArray(subject)
         const filterQueryForSubjectHeading = "/search?filters[subjectLiteral]="
         // splitSubjectHeadings: ["a", "b", "c"]
         const splitSubjectHeadings = subject.split(" -- ")
@@ -421,19 +408,49 @@ export default class BibDetails {
     }
   }
 
-  constructSubjectHeadingsArray(subject: string) {
+  constructSubjectLiteralsArray(subject: string) {
     // subject = "Italian food -- Spaghetti"
-    let stackedSubjectHeading = ""
+    let stackedSubjectLiteral = ""
 
     return subject
       .split(" -- ") // ["Italian food", "spaghetti"]
       .map((urlString, index) => {
         const dashDivided = index !== 0 ? " -- " : ""
         // First iteration "Italian food"
         // Second iteration "Italian food -- spaghetti"
-        stackedSubjectHeading = `${stackedSubjectHeading}${dashDivided}${urlString}`
+        stackedSubjectLiteral = `${stackedSubjectLiteral}${dashDivided}${urlString}`
 
-        return stackedSubjectHeading
+        return stackedSubjectLiteral
       })
   }
+
+  /**
+   * Flatten subject headings into a list of objects with a url and a label
+   */
+  flattenSubjectHeadingUrls(heading): BibDetailURL[] | null {
+    if (!heading.label || !heading.uuid) return null
+    const subjectHeadingsArray = []
+
+    // iterate through each nested subject until there's no parent element
+    let currentHeading = heading
+
+    while (currentHeading.parent) {
+      subjectHeadingsArray.unshift(
+        this.getSubjectHeadingUrl(currentHeading.uuid, currentHeading.label)
+      )
+      currentHeading = currentHeading.parent
+    }
+    // add the top level subject heading
+    subjectHeadingsArray.unshift(
+      this.getSubjectHeadingUrl(currentHeading.uuid, currentHeading.label)
+    )
+    return subjectHeadingsArray
+  }
+
+  getSubjectHeadingUrl(uuid: string, label: string): BibDetailURL {
+    return {
+      url: `/subject_headings/${uuid}?label=${encodeURIComponent(label)}`,
+      urlLabel: label.split(" -- ").pop(),
+    }
+  }
 }
diff --git a/src/models/modelTests/BibDetails.test.ts b/src/models/modelTests/BibDetails.test.ts
@@ -63,25 +63,25 @@ describe("Bib model", () => {
         value: [
           [
             {
-              url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze,%20G%C3%A9rard%20de",
+              url: "/subject_headings/cf347108-e1f2-4c0f-808a-ac4ace2f0765?label=Cortanze%2C%20G%C3%A9rard%20de",
               urlLabel: "Cortanze, Gérard de",
             },
             {
-              url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze,%20G%C3%A9rard%20de%20--%20Childhood%20and%20youth",
+              url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze%2C%20G%C3%A9rard%20de%20--%20Childhood%20and%20youth",
               urlLabel: "Childhood and youth",
             },
           ],
           [
             {
-              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors,%20French",
+              url: "/subject_headings/5fd065df-b4e9-48cb-b13c-ea15f36b96b4?label=Authors%2C%20French",
               urlLabel: "Authors, French",
             },
             {
-              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors,%20French%20--%2020th%20century",
+              url: "/subject_headings/e43674a7-5f02-44f1-95cd-dbcc776331b7?label=Authors%2C%20French%20--%2020th%20century",
               urlLabel: "20th century",
             },
             {
-              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors,%20French%20--%2020th%20century%20--%20Biography",
+              url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors%2C%20French%20--%2020th%20century%20--%20Biography",
               urlLabel: "Biography",
             },
           ],

diff --git a/src/types/bibDetailsTypes.ts b/src/types/bibDetailsTypes.ts
@@ -1,7 +1,7 @@
 export type AnyBibDetail = BibDetail | LinkedBibDetail | SubjectHeadingDetail
 
 export interface SubjectHeadingDetail {
-  value: Url[][]
+  value: BibDetailURL[][]
   label: string
 }
 
@@ -13,15 +13,15 @@ export interface BibDetail {
 }
 
 export interface LinkedBibDetail {
-  value: Url[]
+  value: BibDetailURL[]
   // label is the formatted name of the field, such as "Author"
   label: string
   // indicates if a linked bib detail is internal, like a link to a creator
   // literal search, or external, like supplementary content
   link?: "internal" | "external"
 }
 
-export interface Url {
+export interface BibDetailURL {
   url: string
   urlLabel: string
 }