-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
base: master
Are you sure you want to change the base?
5831/feature/unified read button dropdown #9544
Conversation
1bdefa4
to
e2c8bba
Compare
6829c7d
to
d98df44
Compare
.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; | ||
} |
There was a problem hiding this comment.
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!
49bff57
to
5cc732a
Compare
openlibrary/macros/LocateButton.html
Outdated
$ locateUrl = "/books/XXX/-/borrow?action=locate".replace('XXX', edition_key or '') | ||
|
||
<div class="cta-button-group"> | ||
<a class="cta-btn cta-btn--available" href="$locateUrl"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs analytics clicktracking
<a class="cta-btn cta-btn--available" href="$locateUrl"> | |
<a class="cta-btn cta-btn--available" href="$locateUrl" data-ol-link-track="CTAClick|Locate"> |
828eedd
to
d26c347
Compare
for more information, see https://pre-commit.ci
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.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
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
Simplify HTML structure with the removal of superfluous classes and divs from ReadButton.html and LocateButton.html macros. Simplify or remove many CSS styling rules, move away from mixin-focused styling and unnecessary class descriptors. Overhaul rules for dropper menu positioning to attach it more firmly to details element.
…m:SivanC/openlibrary into 5831/feature/unified-read-button-dropdown
for more information, see https://pre-commit.ci
Add search inside functionality and input box styling to the read button. Remove Search inside button from books page read column.
for more information, see https://pre-commit.ci
Change search inside form input element to take up whole box in dropper, add highlight on hover, group search inside-related style rules in dropper menu for clarity.
Remove descending specificity, add comments to break down stylesheet sections, remove overly specific rules
Remove locate button macro and references in other files, since it has been moved to another branch as part of internetarchive#9952
Fix a litany of minor styling issues
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 ingeneric-dropper.less
to facilitate adetails
element-based implementation of a button with dropdown.Testing
WIP
Screenshot
WIP
Stakeholders
@mekarpeles @jimchamp