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.

Sign up for GitHub

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

Jump to bottom

"Read more" button should not be added if it takes up as much space as the hidden text #9329

Open
cdrini opened this issue May 23, 2024 · 2 comments · May be fixed by #9871
Open

"Read more" button should not be added if it takes up as much space as the hidden text #9329

cdrini opened this issue May 23, 2024 · 2 comments · May be fixed by #9871
Assignees
@CodeMaverick2
Labels
Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 3 Issues that we can consider at our leisure. [managed] Team: Front-end Issues belonging to the Front-end team [experimental tag] Theme: Design Issues related to UI design, branding, etc. [managed] Type: Bug Something isn't working. [managed]

Comments

@cdrini
Copy link
Collaborator

cdrini commented May 23, 2024 •
edited
Loading

Problem

Evidence / Screenshot

image
image

Relevant URL(s)

E.g. https://openlibrary.org/books/OL8978501M/Robin_Hood

Reproducing the bug

  1. Go to https://openlibrary.org/books/OL8978501M/Robin_Hood
  • Expected behavior: Description not collapsed, since "read more" makes the section actually longer :P
  • Actual behavior: "Read more" shown over description

Context

  • Browser (Chrome, Safari, Firefox, etc): FF
  • OS (Windows, Mac, etc): Win
  • Logged in (Y/N): Y
  • Environment (prod, dev, local): prod

Notes from this Issue's Lead

Proposal & constraints

Perhaps we can update the + 1 here:

openlibrary/openlibrary/plugins/openlibrary/js/readmore.js

Line 58 in 73b70f6

if (this.$content.scrollHeight <= (this.collapsedHeight + 1)) {

To instead be the height of the .read-more__toggle--more button. We might need to capture that height in the constructor though, before the read more button is potentially hidden!

Related files

https://github.com/internetarchive/openlibrary/blob/73b70f6b997c401caf2fe8078a6296c8f3167ee5/openlibrary/plugins/openlibrary/js/readmore.js

Stakeholders

@RayBB

Note: Before making a new branch or updating an existing one, please ensure your branch is up to date.
@cdrini cdrini added Type: Bug Something isn't working. [managed] Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead Priority: 3 Issues that we can consider at our leisure. [managed] Theme: Design Issues related to UI design, branding, etc. [managed] Team: Front-end Issues belonging to the Front-end team [experimental tag] Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels May 23, 2024
@CodeMaverick2
Copy link
Contributor

@RayBB I would like to work on this issue

@RayBB
Copy link
Collaborator

RayBB commented Jul 16, 2024

@CodeMaverick2 assigned to you :)

@CodeMaverick2 CodeMaverick2 mentioned this issue Jul 25, 2024
Closed
@mishalsheza mishalsheza mentioned this issue Aug 12, 2024
Closed
@RayBB RayBB added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Sep 11, 2024
@CodeMaverick2 CodeMaverick2 linked a pull request Sep 11, 2024 that will close this issue
Open
@RayBB RayBB added Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. and removed Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] labels Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@CodeMaverick2 CodeMaverick2

Labels
Lead: @RayBB Issues overseen by Ray (Onboarding & Documentation Lead) [manages] Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 3 Issues that we can consider at our leisure. [managed] Team: Front-end Issues belonging to the Front-end team [experimental tag] Theme: Design Issues related to UI design, branding, etc. [managed] Type: Bug Something isn't working. [managed]
Projects
None yet
Milestone
No milestone
3 participants
@RayBB @cdrini @CodeMaverick2