Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCC-0898 - Search Results View All Link #56

Merged
merged 5 commits into from
Dec 12, 2023

Conversation

dgcohen
Copy link
Contributor

@dgcohen dgcohen commented Dec 11, 2023

Ticket: https://jira.nypl.org/browse/SCC-3898

This PR renders a View All x Items link that leads to the Bib page when the number of physical items exceeds 3.

Notes:

  1. This should be moved into the conditional that renders the ItemTable when the two branches are merged.
  2. I changed the item location endpoint type from ItemLocationEndpoint to a generic string to avoid having to cast the type for every item fixture. Not sure if there's a more elegant way to handle this in typescript, but it shouldn't make a difference in practice.

Copy link

vercel bot commented Dec 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
research-catalog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 12, 2023 4:51pm

Copy link
Member

@EdwinGuzman EdwinGuzman left a comment

Choose a reason for hiding this comment

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

One update similar to PR #55

{bib.numPhysicalItems > ITEMS_PER_SEARCH_RESULT && (
<CardActions>
<RCLink href={`${bib.url}#items-table`}>
<Text
Copy link
Member

Choose a reason for hiding this comment

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

Similar to the other PR, don't render p elements inside a elements. This Link can also be of type "standalone" which you may need to propagate down from the RCLink.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, just made both links of the "standalone" type.

@dgcohen dgcohen merged commit 424f107 into main Dec 12, 2023
3 checks passed
@dgcohen dgcohen deleted the SCC-3898/search-results-view-all-link branch December 18, 2023 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants