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 Links and code section overflow in API Documentation pages #8841

Conversation

rishabhkr-r111
Copy link
Contributor

Closes #8839

Fixed Links and code section overflow in API Documentation pages.

Technical

Added word-wrap: break-word property to div#contentBody to solve the issue.

Testing

Screenshot

Stakeholders

@jimchamp

@codecov-commenter
Copy link

codecov-commenter commented Feb 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 16.02%. Comparing base (c1997f5) to head (911457e).
Report is 32 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8841      +/-   ##
==========================================
- Coverage   16.28%   16.02%   -0.27%     
==========================================
  Files          87       89       +2     
  Lines        4618     4693      +75     
  Branches      805      818      +13     
==========================================
  Hits          752      752              
- Misses       3371     3434      +63     
- Partials      495      507      +12     

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

Copy link
Member

@mekarpeles mekarpeles left a comment

Choose a reason for hiding this comment

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

lgtm

@mekarpeles mekarpeles added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Feb 26, 2024
@mekarpeles mekarpeles self-assigned this Feb 26, 2024
@@ -15,6 +15,9 @@ div#contentBody {
img {
max-width: 100%;
}
p {
Copy link
Member

Choose a reason for hiding this comment

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

This may apply to more cases than we want, a better solution may be to do overflow-x: auto on the specific elements causing the issue, e.g. pre tags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mekarpeles this issue effects links, code and text as well (which I noticed later) present in the page. The text present in the page is only wrapped around p tag, so applying word-wrap: break-word property will solve the overflow issue for links, code and text as well.
using overflow-x: auto will introduce a scroll bar for the elements that are overflowing, a behavior that we may not want.

Copy link
Collaborator

@cdrini cdrini Mar 1, 2024

Choose a reason for hiding this comment

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

For code blocks (<pre>), the scroll bar is preferable to wrapping 👍 Adding a rule specifically for anchors to allow word-wrap: break-word seems reasonable! Those two rules should handle all the cases. Although it is technically possible to have text that is very long, it doesn't really happen often in reality.

@mekarpeles mekarpeles assigned scottbarnes and unassigned mekarpeles Feb 26, 2024
@mekarpeles mekarpeles added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] and removed On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing labels Feb 26, 2024
@rishabhkr-r111
Copy link
Contributor Author

As mentioned by @cdrini updated the <pre> tag to include a scroll bar, and for anchors used word-wrap: break-word.

@scottbarnes scottbarnes added On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing and removed Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] labels Mar 10, 2024
Copy link
Collaborator

@scottbarnes scottbarnes left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @rishabhkr-r111.

@scottbarnes scottbarnes merged commit b247a17 into internetarchive:master Mar 10, 2024
3 checks passed
Achorn pushed a commit to Achorn/openlibrary that referenced this pull request Mar 14, 2024
…ernetarchive#8841)

* Fixed text overflow in API Documentation pages by adding word-warp
property

* updated pre tag to fix overflow issue in code section
merwhite11 pushed a commit to merwhite11/openlibrary that referenced this pull request Mar 27, 2024
…ernetarchive#8841)

* Fixed text overflow in API Documentation pages by adding word-warp
property

* updated pre tag to fix overflow issue in code section
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Links and code section overflow in API Documentation pages
6 participants