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

Fixed Read More Button Display Logic to Avoid Unnecessary Display #9959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CodeMaverick2
Copy link
Contributor


Description

This PR addresses an issue where the "Read more" button was displayed even when it extended the section beyond its fully expanded content. This led to a confusing user experience, as the "Read more" button was not serving its intended purpose. The following changes have been made to resolve this issue:

Closes #9329

Changes

  1. Capture "Read More" Button Height:

    • The height of the "Read more" button is now captured during initialization to ensure accurate layout calculations.
  2. Updated Condition in reset Method:

    • The logic determining whether the "Read more" button should be displayed has been updated to account for the button's height. This ensures the button is only shown when necessary.

Impact

  • Improved User Experience: The "Read more" button is now displayed only when needed, reducing unnecessary clutter and confusion.
  • Correct Layout Behavior: The layout now behaves as expected, with the "Read more" button enhancing usability instead of detracting from it.

Testing

To test visit a link like https://testing.openlibrary.org/books/OL8978501M/Robin_Hood

Screenshots

Screenshot from 2024-07-25 18-43-42

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 17.16%. Comparing base (ce16a79) to head (55082ce).
Report is 484 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9959      +/-   ##
==========================================
+ Coverage   16.06%   17.16%   +1.10%     
==========================================
  Files          90       89       -1     
  Lines        4769     4754      -15     
  Branches      832      832              
==========================================
+ Hits          766      816      +50     
+ Misses       3480     3428      -52     
+ Partials      523      510      -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CodeMaverick2
Copy link
Contributor Author

CodeMaverick2 commented Oct 26, 2024

@RayBB This issue has been resolved and is ready for merge. When convenient, could you please add the hacktoberfest-accepted label? Thank you!

@RayBB
Copy link
Collaborator

RayBB commented Oct 29, 2024

@CodeMaverick2 please do not keep opening new PRs for the same issue. When feedback is given address it in that PR.
That being said, have you addressed all of @cdrini 's feedback in #9871 ?

Please note that the team is still working on getting Open Library Fully operational so may not be able to review this soon.

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.

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