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

Fix incorrect width computation for text that can't wrap #57

Merged
merged 1 commit into from
May 9, 2023
Merged

Fix incorrect width computation for text that can't wrap #57

merged 1 commit into from
May 9, 2023

Conversation

TomerAberbach
Copy link
Contributor

Some reproductions of the issue: https://codesandbox.io/s/react-wrap-balancer-bug-ubbizi

The problem is that if the text can't wrap, the container's height will never change as we shrink the wrapper. So we keep shrinking the wrapper, never observe a height change, and end up with a width lower than the minimum required to not overflow the container.

I solved it by making sure lower is no less than the minimum width necessary to fit the contents (see scrollWidth documentation).

Ideally you wouldn't give react-wrap-balancer text that can't wrap in the first place, but sometimes you're getting arbitrary titles from somewhere and you don't know if they are single words or have non-breaking spaces so we should probably handle this case in the package.

@vercel
Copy link

vercel bot commented May 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-wrap-balancer ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 1, 2023 2:47am

Copy link
Owner

@shuding shuding left a comment

Choose a reason for hiding this comment

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

Great catch, and nice solution! Thanks!

@shuding shuding merged commit 141d1fa into shuding:main May 9, 2023
1 check passed
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.

None yet

2 participants