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

Enforce a readable line length for Read More #3442

Conversation

heyhippari
Copy link
Contributor

@heyhippari heyhippari commented Dec 6, 2024

Changed

  • Changed: Changed the maximum line length of text inside of expandable blocks of text to a readable value

Before:
image

After (Ignore the slight differences in metadata, my prod server has Kavita+ and Komf alimenting it):
image

Review cards:
image

Usually, for longer texts, readability suffers with lines over 75 characters on desktop (On mobile, that's around 50 characters).

To make sure descriptions stay readable, the contents of the read-more component are now capped to a line length of 75 characters on larger breakpoints and 50 characters on smaller breakpoints.

I'm keeping this PR as a draft for now, because I need to adjust the cutoff limits for the various uses of the component. I'm generally trying to keep the number of lines similar to the current one, to not break design. It sacrifices the amount of content that's readable without expanding, but I feel like preserving alignment is much better than having 5+ lines of text shown on load.

Edit: Forgot a source for the length thing: https://baymard.com/blog/line-length-readability

Edit 2: This is now ready for review.

One thing I noticed while adjusting the cutoff, there's often an overflow on the review cards when reviews have line returns. I lowered the cutoff a bit (The max-width doesn't do anything for these, so I can omit this if you prefer), which fixes the overflow in some cases, but there should probably be some server-size cleanup of empty lines and such (Especially since we'll likely want to let clients decide how to display them, including the distance between paragraphs, so returning straight paragraphs separated by one \n seems like the best solution). In any case, that's for another PR some day, but I figured I'd raise it while I'm seeing it.

@heyhippari heyhippari force-pushed the refactor/read-more-readability branch from 03daf4d to cdd60aa Compare December 7, 2024 14:48
@heyhippari heyhippari force-pushed the refactor/read-more-readability branch from cdd60aa to 7f26096 Compare December 7, 2024 14:49
@heyhippari heyhippari marked this pull request as ready for review December 7, 2024 14:55
@majora2007
Copy link
Member

I don't disagree with you. I had increased this as people with 4K monitors were complaining about not eating enough space.

@majora2007 majora2007 changed the base branch from develop to feature/pr-flush December 8, 2024 16:03
@majora2007 majora2007 merged commit 2091b35 into Kareadita:feature/pr-flush Dec 8, 2024
@majora2007
Copy link
Member

This broke the read-more in changelog component. can you please raise a fix for it @heyhippari ?

image

@majora2007
Copy link
Member

I ended up reworking the UX here, but overall we need an ability to conditionally apply your new css so it can work as it was for different situations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants