-
Notifications
You must be signed in to change notification settings - Fork 0
Make availability statements into links #343
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
Conversation
** Why are these changes being introduced: We want the "availability statements" which we generate in our search results to be links to the full record in Primo. These links should match the links already present in the title elements. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/use-318 ** How does this address that need: This replicates the link logic from the title element in _result_primo in the availability span. As a result of this, we also need to tweak the styling of visited links, because visited links in the result block (which includes this link) are defined to be white text (on a white background, because these links are not buttons). ** Document any side effects to this change: I have two questions about this work, which I'd like review to address: 1. Should we create a helper method to handle the link-building logic for the title and availability text? Any thoughts about where this helper lives, as we currently have helpers in both record and search? 2. The styles I'm working with come from two locations in the results partial stylesheet. I've chosen to make this change around the other link rules, but there is also an availability-specific short batch of rules around line 334 if that makes more sense.
Pull Request Test Coverage Report for Build 21181155442Details
💛 - Coveralls |
|
@djanelle-mit I'm tagging you for review here because I'm specifically interested in your thoughts about the CSS strategy I'm using here. If you'd like, I'm happy to create a ticket for you to deal with any styling tweaks after this, but if it's faster for me just to make a few tweaks I'm also happy to take direction. |
|
@matt-bernhardt Hmm... I think this might be worth a quick refactor now that we have a clear picture of what could go in that links area between LibKey, availability, etc. Why don't we create a ticket for that and I can pick it up. What's there now is fine to roll out as-is. I need some time to think about how to treat these "tertiary" links... should they have the same blue underline styles that our main links do, or should they be subtle and only obviously a link on hover? Should there be some lighter weight but still noticeable link affordance? |
|
The style refactoring work has been ticketed as USE-339 (please double-check this, as the ticket was initially created under the TSS space for unknown reasons - I've tried to move it) |
Yep, I'm seeing it as I'd expect. |
This wraps our current availability statement in the same logic that we use to build the links for record titles - going to full record pages in Primo. As a side effect, we need to overwrite some CSS rules that try and turn this statement white when it's visited.
I have two questions for reviewers:
results-getblock (which includes the availability statement) - I chose to put the rules here in the link-focused area, but am open to feedback about other ways to approach it.Ticket: https://mitlibraries.atlassian.net/browse/use-318
Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
E.g., if the PR includes updated dependencies and/or data
migration, or how to confirm the feature is working.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing