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

Unified "Read" Button Dropper #5831

Open
cdrini opened this issue Nov 8, 2021 · 22 comments · May be fixed by #9544 or #9941
Open

Unified "Read" Button Dropper #5831

cdrini opened this issue Nov 8, 2021 · 22 comments · May be fixed by #9544 or #9941
Assignees
Labels
Affects: UI Issues with the web site's user interface. [managed] Lead: @mekarpeles Issues overseen by Mek (Staff: Program Lead) [managed] Needs: Design Feedback Primary Focus The active main quest of this contributor Priority: 2 Important, as time permits. [managed] Theme: Book Page Issues related to the redesign of the books page Theme: Design Issues related to UI design, branding, etc. [managed] Type: Design Proposal Proposing a design and soliciting feedback + approval Type: Proposal

Comments

@cdrini
Copy link
Collaborator

cdrini commented Nov 8, 2021

We have a lot of read-y actions a user can perform; designing these as a dropdown would allow us to expose these options in more places; eg search results, carousels; without taking up a bunch of space.

Options

Borrow / Read, Listen, Search Inside, Download, Locate (meaning check nearby libraries)

Description, Patron Story

  • What is the design change?

Turns the read button into a drop-down menu with more options

  • Why is this design change necessary?

saves real estate on the page by grouping similar options (downloads, etc)

  • Who is it for?

all patrons and stakeholders of the books page

  • constraints and implementation

Preferably everything is managed using details html element and no js is required, all css.

Relevant Designs

image

Screenshot 2024-05-10 at 8 33 10 AM

https://www.figma.com/file/dYQkIJOJeMo9hx7ewZQH9x/Open-Library%3A-Design-System-%26-Component-Library?type=design&node-id=601-199&mode=design&t=giRw2crBrKKYkWGi-0

Problem Statement

  • Each book has one or more different applicable actions for reading, locating a copy, etc.
    • e.g. Read, Borrow, Listen, Download epub, Search Inside, Check local library, Purchase...
  • In some case, we overload the read button with an 🎧 icon to suggest there's also a listen option. This is a confusing pattern.
  • Trying to represent all a book's options risks taking up space, introducing multiple access patterns, and creating multiple locations where a patron may have to learn to look.

Proposal & Constraints

Actions in dropdown (WIP):

  • Borrow (or) Read (or) Listen (or) Borrow & Listen
  • Download epub
  • Search Inside
  • Purchase
  • Check local library

Additional context

Stakeholders

@mekarpeles @cdrini

@cdrini cdrini added Affects: UI Issues with the web site's user interface. [managed] Theme: Design Issues related to UI design, branding, etc. [managed] Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] labels Nov 8, 2021
@cdrini cdrini modified the milestones: Next (proposed), Active Sprint Nov 8, 2021
@jimchamp jimchamp added Priority: 3 Issues that we can consider at our leisure. [managed] and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] labels Nov 15, 2021
@cdrini cdrini modified the milestones: Sprint 2021-11, Active Sprint Nov 29, 2021
@jimchamp jimchamp added the Theme: Book Page Issues related to the redesign of the books page label Dec 14, 2021
@seabelis
Copy link
Collaborator

Related request from a patron:
"Hi! Please add "Want to read" button to search resulta view so that results don't have to opened separately."

@jimchamp
Copy link
Collaborator

jimchamp commented May 4, 2022

Decisions made by staff about this:

  1. We want to avoid ever having to show "Not in Library" as a call to action for this button.
  2. We want to include the library links in the redesigned "Read" button. Presently, library links are found in the second item in the sidebar (highlighted below):

library_links

@mekarpeles mekarpeles added Priority: 2 Important, as time permits. [managed] and removed Priority: 3 Issues that we can consider at our leisure. [managed] labels Jun 15, 2022
@mekarpeles
Copy link
Member

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Jun 3, 2024
@mekarpeles
Copy link
Member

The LoanStatus macro handles all of the rending of primary action buttons on the website:
https://github.com/internetarchive/openlibrary/blob/master/openlibrary/macros/LoanStatus.html#L43

It makes use of the ReadButton macro, which is the one we want to upgrade:
https://github.com/internetarchive/openlibrary/blob/master/openlibrary/macros/ReadButton.html

@jimchamp jimchamp removed the Needs: Response Issues which require feedback from lead label Jun 4, 2024
@SivanC
Copy link
Contributor

SivanC commented Jul 2, 2024

EDIT: These issues have been solved with help from rebecca-shoptaw!

@mekarpeles @jimchamp Hi, sorry for my relative radio silence recently, just wanted to give you guys an update on my progress in migrating my codepen design to my local environment.

Here's what it looks like:
Closed:
image
Open:
image

Styling issues I'm currently working on:

  1. I tried to read up on specificity and how to override certain style rules in the best way but I'm a little stumped. The two fixes I need to make are to remove the -5px horizontal margin from the summary element (causing the current misalignment) and to set list-style(-type): none for the dropdown content to remove the bullets.

  2. The worst of the issues is the weird layout of the read button with the summary::after pseudo-element. Not sure why it's so wonky when it worked fine in the codepen. I tried putting a variety of display styles on a variety of elements to no avail. You can see that my element inspector shows a strange (to me) occupied space with display: flex enabled for the summary element:
    With display: flex:
    image
    Without flex:
    image

Other issues I can probably solve by myself but are still WIP are coloring the dropdown arrow and adjusting spacing and hover behavior on the list elements in the dropdown. I'll also begin to work on icons, button functionality and the like when these issues are out of the way. I'll make sure to summarize this again on the call tomorrow. Thanks for taking a look!

@rebecca-shoptaw
Copy link
Collaborator

@SivanC Dropping by with some CSS help in case it's still needed!

  1. For list-style troubleshooting, you'll want to make sure you're adding list-style-type: none to whichever element is the ul
  2. I'd recommend adding a flex-grow: 1 to the "Read" part of the button, which makes it automatically take up all the remaining space in the flex container -- if this doesn't work, you can try pairing it with a flex-shrink: 0 and flex-basis: 1 as well! 🙂

@SivanC
Copy link
Contributor

SivanC commented Jul 2, 2024

@rebecca-shoptaw Hi thanks for your tips! Flex grow works :D As far as the list styling the issue is that I can see in my element inspector that my styling is being overwritten by other rules; other than that it's being applied correctly.
image

@rebecca-shoptaw
Copy link
Collaborator

rebecca-shoptaw commented Jul 2, 2024

@SivanC Aha! You want to make sure you're applying it to the full list ul element rather than each li element -- i.e. if .cta-dropper__content-list is the class for your ul you want to give the list-style-type: none to that itself, not to its li children.

Edit: Hmm looks like list-style-type should work in both places, if you could include a quick screenshot of what the HTML looks like here, i.e. what the hierarchy is of classes, etc. that would be very helpful!

If that doesn't work, there are some specificity tricks you can try, but fingers crossed that does the trick!

@SivanC
Copy link
Contributor

SivanC commented Jul 2, 2024

@rebecca-shoptaw Unfortunately the reason I had it on the li elements was because I thought the same initially and applied it to the ul to no effect. cta-dropper__content-list is the class associated with the ul element and while the styling shows up in the inspector the bullet points remain:
image
image

Here's the full HTML right now:

<div class="dropdown-button-group">
  <div class="cta-dropper">
    <details class="cta-dropper__details">
      <summary class="cta-dropper__summary">
        <div class="cta-button-group">
          <a href="$(stream_url)" title="$title" class="cta-btn cta-btn--ia cta-btn--available cta-btn--$(action)"
              $:analytics_attr(analytics_action)
              $if loan:
                data-userid="$(loan['userid'])"
              >$label</a>
        </div>
      </summary>
      <div class="cta-dropper__content">
        <ul class="cta-dropper__content-list">
          <li>Listen</li>
          <li>Search Inside</li>
          <li>Check Nearby Libraries</li>
        </ul>
      </div>
    </details>
  </div>
</div>

@rebecca-shoptaw
Copy link
Collaborator

rebecca-shoptaw commented Jul 2, 2024

@SivanC Thank you!! Ok, it looks like the styling is being inherited because the parent element of all of this has a read-options class, which has its own li rules.

This means you'll want to use selector specificity to overrule the existing li rules in this particular case. It looks like someone else has already encountered a similar issue and fixed it in read-panel.less:

.Tools .booklinks li {
list-style: none;
font-size: 12px;
}

So it seems that the simplest course of action would be to add a few lines like this directly above those:

.read-options .cta-dropper__content-list li {
  list-style: none;
}

Effectively you're saying that while all .read-options li elements usually have a list-style: disc, if they are the child of a .cta-dropper__content-list they have a list-style: none.

There may be a more elegant way to do this, but this should do the trick!

You could also try swapping out the .cta-dropper__content-list for a higher up class, i.e. .cta-dropper, as long as that won't cause problems in other foreseeable cta-droppers!

@SivanC
Copy link
Contributor

SivanC commented Jul 3, 2024

@rebecca-shoptaw I was about to respond that nothing I tried was working ... until! I noticed that both .read-options and .Tools .btn-notice asked for disc style, so I made a rule that covered both of them according to your suggestion:

.read-options .cta-dropper__content-list li, .Tools .btn-notice .cta-dropper__content-list li {
  list-style: none;
}

image

No more bullet points! :D Also worked to overwrite the negative margins causing the button to be misaligned.

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Jul 3, 2024
@jimchamp jimchamp removed the Needs: Response Issues which require feedback from lead label Jul 9, 2024
@SivanC SivanC linked a pull request Jul 9, 2024 that will close this issue
@mekarpeles mekarpeles added Lead: @mekarpeles Issues overseen by Mek (Staff: Program Lead) [managed] and removed Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] labels Jul 12, 2024
@SivanC
Copy link
Contributor

SivanC commented Jul 26, 2024

Update as of 7/26/24 design call, clarifying some questions surrounding dropper content:

  • Search inside will be a input field
  • Clicking “Download” will scroll to / highlight the Download options (ala clicking on ratings under author name)
  • Clicking Download pills will download directly
  • Buy this book should not be in the dropper for now; considering out of scope
  • Should still show “not in library” + grey for now in this PR
  • In the editions box on a work’s page, read buttons should still have droppers, clicking e.g. download should take you to the download section for that edition (scroll + pulsing border)

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Jul 31, 2024
@cdrini cdrini modified the milestones: Sprint 2024-09, Sprint 2024-08 Aug 2, 2024
@mekarpeles mekarpeles removed the Needs: Response Issues which require feedback from lead label Aug 26, 2024
@mekarpeles mekarpeles linked a pull request Oct 5, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects: UI Issues with the web site's user interface. [managed] Lead: @mekarpeles Issues overseen by Mek (Staff: Program Lead) [managed] Needs: Design Feedback Primary Focus The active main quest of this contributor Priority: 2 Important, as time permits. [managed] Theme: Book Page Issues related to the redesign of the books page Theme: Design Issues related to UI design, branding, etc. [managed] Type: Design Proposal Proposing a design and soliciting feedback + approval Type: Proposal
Projects
None yet
6 participants