SCC-3776 - Item Table Component #50

dgcohen · 2023-11-30T19:31:11Z

This PR adds the ItemTableData model and the following components:

ItemTable
RequestButtons
ItemAvailability (formerly InformationLinks)

It also includes tests and some small miscellaneous changes to Item model, SearchResultsBib model, and one renaming change (pascal-casing the itemFilterData class).

NB:
There are some further optimizations to be made, but I will not let these hold up the PR. I added comments where relevant.

…/item-table-component

EdwinGuzman

The update makes sense to me. There are some minor comments but I know this will go through another iteration.

Some things that stick out that we have talked about:

Use the the "text" variant for the button that looks like a link for now. I think we may need to reword it and isolate it to better appear as a button. This is for the "contact a librarian" button.
Right now, Buttons styled as link do have the underlined as hover and it is expected.

Something I noticed after we chatted:

the mobile view has many horizontal lines that I will talk to @bigfishdesign13 about. It makes the mobile view confusing (to me at least)

A few things that need to be added that can be another ticket/PR:

the "View all X items > " link when there are more than 3 items in a search result
the eResource links that should appear as "Available Online" (https://www.nypl.org/research/research-catalog/search?q=national%20geographic)

EdwinGuzman · 2023-12-04T15:13:42Z

src/components/RCLink/RCLink.tsx

@@ -8,6 +11,8 @@ interface RCLinkProps {
  children: ReactNode
  className?: string
  color?: string
+  type?: LinkTypes
+  disabled?: boolean


This should be isDisabled. Right now, the disabled version is too light and not the right disabled and I think it's because the DS prop is isDisabled.

For comparison:

EdwinGuzman · 2023-12-04T15:14:03Z

src/components/ItemTable/RequestButtons.tsx

+        <RCLink
+          href={item.aeonUrl}
+          type="buttonSecondary"
+          disabled={!item.isAvailable}


These should be isDisabled.

EdwinGuzman · 2023-12-04T15:24:34Z

src/components/ItemTable/ItemAvailability.test.tsx

+  it("renders the correct text when item is available, has an aeon url, and has a location endpoint", async () => {
+    const item = new Item(itemPhysicallyRequestable, parentBib)
+    render(<ItemAvailability item={item} />)
+    screen.getByText(/Available by appointment/)


These would break if they're not in the document, but for completeness I think it should have the expect(...).toBeInDocument() wrapper.

EdwinGuzman · 2023-12-04T15:24:51Z

src/components/ItemTable/ItemAvailability.test.tsx

+    const item = new Item(itemUnavailable, parentBib)
+    render(<ItemAvailability item={item} />)
+    screen.getByText(/Not available/)
+    screen.getByText(/- Please/)


The above comment applies to all of these.

src/models/SearchResultsBib.ts

EdwinGuzman · 2023-12-04T16:30:21Z

src/components/RCLink/RCLink.tsx

-import { Link as DSLink } from "@nypl/design-system-react-components"
+import {
+  Link as DSLink,
+  type LinkTypes,


@EdwinGuzman take a closer look.

It was pointed out that LinkProps needs to be exported.

EdwinGuzman · 2023-12-04T16:33:14Z

src/components/ItemTable/ItemAvailability.tsx

+        {" - Please "}
+        <Button
+          id="contact-librarian"
+          buttonType="link"


Try buttonType="text".

Does this still need to be addressed?

EdwinGuzman · 2023-12-04T17:30:28Z

src/components/SearchResult/SearchResult.tsx

@@ -29,12 +46,19 @@ const SearchResult = ({ bib }: SearchResultProps) => {
        <RCLink href={`${PATHS.BIB}/${bib.id}`}>{bib.title}</RCLink>
      </CardHeading>
      <CardContent>
-        <Box sx={{ p: { display: "inline", marginRight: "s" } }}>
+        <Box sx={{ p: { display: "inline", marginRight: "s" } }} mb="m">
          {bib.materialType && <Text>{bib.materialType}</Text>}


Taking a better/closer look, I think these should be spans.

bigfishdesign13 · 2023-12-04T21:52:02Z

the mobile view has many horizontal lines that I will talk to @bigfishdesign13 about. It makes the mobile view confusing (to me at least)

@EdwinGuzman Those lines are there to provide delineation between table rows. But yes, that treatment isn't working well here. Perhaps we could tie that style to the showRowDividers prop. MLN is the only NYPL app that is using the Table component and the horizonal lines in the mobile view don't look great on that site either.

…rch-catalog into SCC-3776/item-table-component

charmingduchess · 2023-12-11T18:44:53Z

ENVIRONMENTVARS.md

+| Variable                                 | Type   | Value Example                        | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    |
+| ---------------------------------------- | ------ | ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `NYPL_HEADER_URL`                        | string | "https://ds-header.nypl.org"         | The base URL of the NYPL environment-specific header and footer scripts.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
+| `NEXT_PUBLIC_CLOSED_LOCATIONS`           | string | "all;Library of the Performing Arts" | A semicolon-delimited list of strings. Include quotes around the string. All locations beginning with any string in this list will be removed from the list of request options in the `ElectronicDelivery`, `HoldRequest`, and `ItemTableRow` components. Currently used physical locations: `Schwarzman;Science;Library for the Performing Arts;Schomburg`. To close all locations, add `all`. This will also remove EDD as a request option, the 'Request' buttons, and also disable the hold request/edd forms. If `all` is not present, EDD and 'Request' buttons will still be available. |


What does NEXT_PUBLIC indicate?

That prefix allows env variables to be used on the client side.

charmingduchess · 2023-12-11T18:49:21Z

__test__/fixtures/itemFixtures.ts

@@ -16,6 +18,7 @@ export const itemPhysicallyRequestable = {
      prefLabel: "book, limited circ, MaRLI",
    },
  ],
+  dueDate: ["September 3rd"],


I'm not sure it makes a difference to the code, but I'm pretty sure this would be formatted YYYY-MM-DD

src/components/ItemTable/ItemAvailability.tsx

EdwinGuzman

The update looks good but there are a few things to cover before moving on.

There are general spacing issues that I think will be picked up during VQA. I think now is a good time to share the Vercel Preview link in the Jira ticket with Apoorva and Clare for their VQA and accessibilty review.
The electronic resource link seems better because the link text is actual text and not the URL. Do verify that this is what we want (also note there could be some space above the "Available Online" text):

The "Not available - Please contact a librarian for assistance" text is not displaying. There also seems to be more spacing between elements and it could be slightly reduced. It also appears that the buttons that should be disabled are not. The right side is from this PR and the search term is "National Geographic":

EdwinGuzman

Approving knowing there's stylistic updates todo later.

dgcohen and others added 30 commits November 15, 2023 10:42

Initialize item table component

53583a5

Restore getActivePage

494a832

Refactor item table headings getter function

eb90f4b

Initialize ItemTableData model

529ae04

Initialize item table data refactor

b124ffc

Remove length check on volume

6776e03

Merge branch 'main' of github.com:NYPL/research-catalog into SCC-3776…

7e4dd90

…/item-table-component

Remove getActivePage util

05b2cd1

Render table content

ee5c1fc

Render table content

1f4806c

Use tableLayout key in css prop

50a7e4a

Initialize request buttons

723fd04

Add closed locations env variable

5ed422f

Create separate item table per item on search results

e5046b6

Format environment variables readme

68cbb69

Tweak table styles

527f32b

Tweak table styles on mobile

c66f51a

Render request link data

20d8bbf

Render request link data

9ecf936

Increase bottom margin on buttons

cc19610

Remove onClick handler comments

e155254

Fix isRecap attribute name

e488321

Add bottom margin to information links

77d2419

Start off site logic

16485d0

Add not available case to information links

02ac042

Rename information links to ItemAvailability

dc145c0

Fix issues with DS text components

ee31a18

Add TODO comments

fd05fe0

Refactor closed locations logic

d369f98

Fix broken SearchResult test

36d0a09

Add tests for request buttons

f87badb

vercel bot deployed to Preview December 1, 2023 20:04 View deployment

dgcohen marked this pull request as ready for review December 1, 2023 20:07

dgcohen requested review from EdwinGuzman and charmingduchess December 1, 2023 20:07

EdwinGuzman requested changes Dec 4, 2023

View reviewed changes

EdwinGuzman added the Changes Requested label Dec 4, 2023

dgcohen added 2 commits December 7, 2023 15:57

Use buttondisabled as type when item is not available

d10de6d

Remove isDisabled link prop from RCLink

a5ae2bb

vercel bot deployed to Preview December 7, 2023 20:59 View deployment

dgcohen added 2 commits December 8, 2023 10:22

Fix dueDate type

27b56f4

Merge branch 'SCC-3776/item-table-component' of github.com:NYPL/resea…

deaa85f

…rch-catalog into SCC-3776/item-table-component

vercel bot deployed to Preview December 8, 2023 15:24 View deployment

charmingduchess reviewed Dec 11, 2023

View reviewed changes

src/components/ItemTable/ItemAvailability.tsx Show resolved Hide resolved

dgcohen added 2 commits December 12, 2023 14:13

Merge in main and resolve conflicts

08ef887

Create showViewAllItems link helper

624db28

vercel bot deployed to Preview December 12, 2023 19:18 View deployment

Change date format in fixture

417f23a

vercel bot deployed to Preview December 12, 2023 19:30 View deployment

Use string for search params issuance

903a570

vercel bot deployed to Preview December 12, 2023 20:06 View deployment

dgcohen requested review from EdwinGuzman and charmingduchess December 12, 2023 20:07

EdwinGuzman reviewed Dec 13, 2023

View reviewed changes

EdwinGuzman approved these changes Dec 21, 2023

View reviewed changes

dgcohen merged commit 8e584fc into main Dec 21, 2023
3 checks passed

dgcohen deleted the SCC-3776/item-table-component branch December 21, 2023 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SCC-3776 - Item Table Component #50

SCC-3776 - Item Table Component #50

dgcohen commented Nov 30, 2023 •

edited

Loading

EdwinGuzman left a comment

EdwinGuzman Dec 4, 2023

EdwinGuzman Dec 4, 2023

EdwinGuzman Dec 4, 2023

EdwinGuzman Dec 4, 2023

EdwinGuzman Dec 4, 2023

EdwinGuzman Dec 4, 2023

EdwinGuzman Dec 4, 2023

EdwinGuzman Dec 13, 2023

EdwinGuzman Dec 4, 2023

bigfishdesign13 commented Dec 4, 2023

charmingduchess Dec 11, 2023

dgcohen Dec 12, 2023

charmingduchess Dec 11, 2023

EdwinGuzman left a comment

EdwinGuzman left a comment

SCC-3776 - Item Table Component #50

SCC-3776 - Item Table Component #50

Conversation

dgcohen commented Nov 30, 2023 • edited Loading

EdwinGuzman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bigfishdesign13 commented Dec 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EdwinGuzman left a comment

Choose a reason for hiding this comment

EdwinGuzman left a comment

Choose a reason for hiding this comment

dgcohen commented Nov 30, 2023 •

edited

Loading