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

Don't show read more link when not needed #6428

Merged
merged 4 commits into from
Apr 20, 2022

Conversation

tangym27
Copy link
Contributor

Closes #6401

  • Fix such that the "read more/less" button does not appear
  • Button will also appear/disappear when the screen gets resized

Technical

  • Resizing based off of code from SearchBar.js
  • Hardcoded 58 is based off of the height of .restricted-height (made a comment in the code itself)

Testing

(Locally): http://localhost:8080/works/OL13101191W/Alice%27s_Adventures_in_Wonderland

Screenshot

With resizing in action (sorry for low quality):
read_more

Stakeholders

@jimchamp

@mekarpeles
Copy link
Member

@tangym27, this looks like a great direction to me, stellar job on this PR and thank you for including the screencast!

@mekarpeles
Copy link
Member

mekarpeles commented Apr 15, 2022

Tests are failing on coverage (which is OK) we just need to change the coverage % here:
https://github.com/internetarchive/openlibrary/blob/master/package.json#L99-L104

You can see the test coverage errors here to determine how much we should increase the limits by:
https://github.com/internetarchive/openlibrary/runs/6040584094?check_suite_focus=true#step:11:94

@mekarpeles mekarpeles added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 15, 2022
@mekarpeles mekarpeles added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Apr 18, 2022
tangym27 and others added 2 commits April 20, 2022 13:29
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

@cdrini cdrini merged commit dbac857 into internetarchive:master Apr 20, 2022
@cdrini cdrini changed the title fix read more link issue Don't show read more link when not needed Apr 20, 2022
@jimchamp jimchamp removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 1 Do this week, receiving emails, time sensitive, . [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Read more" links sometimes appear when not needed
4 participants