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

5831/feature/unified read button dropdown #9544

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

SivanC
Copy link
Contributor

@SivanC SivanC commented Jul 9, 2024

Opening draft PR after suggested during today's community call (July 9 2024).

Closes #5831

Feature/refactor: Condenses several "read-y" actions (Read/Borrow/Listen, Search Inside, Download, Check Nearby Libraries) into a dropdown.

Technical

Complete overhaul of macros/ReadButton.html and new CSS in generic-dropper.less to facilitate a details element-based implementation of a button with dropdown.

Testing

WIP

Screenshot

WIP

Stakeholders

@mekarpeles @jimchamp

@SivanC SivanC force-pushed the 5831/feature/unified-read-button-dropdown branch from 1bdefa4 to e2c8bba Compare August 7, 2024 18:18
@SivanC SivanC force-pushed the 5831/feature/unified-read-button-dropdown branch from 6829c7d to d98df44 Compare August 20, 2024 06:28
Comment on lines 202 to 219
.read-options .cta-dropper__button .cta-btn, .Tools .btn-notice .cta-dropper__button .cta-btn {
margin-bottom: 0;
border-radius: 5px 2px 2px 5px;
text-align: center;
}
.read-options .cta-dropper__content-list .cta-btn--w-icon, .Tools .btn-notice .cta-dropper__content-list .cta-btn--w-icon {
margin: 0;
}
.read-options .cta-dropper__summary, .Tools .btn-notice .cta-dropper__summary {
margin: 0;
}
.read-options .cta-dropper__content ul, .Tools .btn-notice .cta-dropper__content ul {
margin-left: 5px;
}
.read-options .cta-dropper__content-list li, .Tools .btn-notice .cta-dropper__content-list li {
list-style: none;
margin: 0;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mekarpeles Here's where the bot dings me for exceeding 0,3,0 specificity. Here is also some conversation about it in the issue which lead me to this route. My CSS is not polished obviously and I was planning to go back through and streamline a bit anyway so no rush to dig into it right now if you have other things to do!

@SivanC SivanC force-pushed the 5831/feature/unified-read-button-dropdown branch from 49bff57 to 5cc732a Compare September 25, 2024 20:03
SivanC and others added 25 commits September 25, 2024 13:04
Progress towards reworking the read button, currently with major styling issues and lack of functionality.
Change from cta-button-groupp to cta-button-group
Remove some unnecessary div/span tags to simplify the HTML. Overwrite general CSS styling related to margins and list-style with specific rules.
Add temporary icons to the content buttons, and also replace and improve the dropper arrow.
Move the cta button outside of the summary tag in order to give the summary::after pseudo-element its own container for styling purposes. Has the side effect of moving the dropped content to appear directly below the dropper button, and not align with the left side of the cta button as before.
Change position of dropper content to be aligned with left edge of read button as it was before restructuring of the element hierarchy.
Remove gray color when hovering over dropper content, add dark blue hover color to dropper button; change padding and margin rules to make summary element same size and shape as details element; fix misaligned open dropper arrow.
Add four new icons to static/images/icons (listen, search, download, and map). Add link to read aloud audio to content dropper Listen button.
Re-add coniditonal to check for listen-ability of work. Listen button does not show up in content dropper if there is no read aloud option available.
Clicking the check nearby libraries header in the read button will smoothly scroll the user to the check nearby libraries panel and highlight it with a border animation. See PR 7125 for similar implementation with star ratings.
Remove the scroll to CNL panel and border highlight/pulse after clicking the check nearby libraries button in the dropdown. No longer necessary since the goal of the button is to take the user to a worldcat link for now instead of having multiple options.
Add a Check Nearby Libraries option to the read button dropper if an OCLC or ISBN identifier exists for an edition. Not satisfied with this solution as it adds another argument to LoanStatus.html calls, will probably be changed
Switch from passing an edition to LoanStatus to using its existing doc parameter when attempting to grab OCLC and ISBN identifiers for generating a worldcat url. Wasn't able to test many situations such as multiple editions with different availability of OCLC/ISBN, etc., so will need further testing in a more robust environment.
Slightly change read-panel.less (removing .Tools from a styling rule, removing unused summary style rule) to lower specificity requirements in generic-dropper.less and integrate previously isolated rules into main styling block.
Remove extra line that unintentionally increased specificity
Refactor the Check Nearby Libraries button (and rename it) to use the OLID instead of OCLC/ISBN/title identifier, making it more robust. Now goes to a new LocateButton.html macro instead of WorldcatURL.html as well.
Change variable from olid to edition_key to make its contents more clear
Adjust width of dropper content to account for shortened longest button label. Return 10px bottom margin to Not in Library button that was removed when .Tools was removed from the main read-panel.less styling rules.
Change from using work_key to doc.key in LoanStatus.html to retrieve the correct OLID for searching Worldcat
Replace current 'Not In Library' button with 'Locate' on the search page, which brings the user to the worldcat page for the book. Only applies if the book is not available in Open Library
Remove the worldcat link directly below the Not In Library button in the edition box on the books page, since it is replaced by the locate button. Fix locate button macro complaining about possibility of non-string value for edition key
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.

Unified "Read" Button Dropper
1 participant