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

Create hook to auto-scale font size to fit into the parent container #2509

Merged
merged 4 commits into from
Feb 8, 2022

Conversation

urbullet
Copy link
Contributor

@urbullet urbullet commented Feb 6, 2022

Created a hook that takes two refs (parent container & text container). And then checks if the text container is overlapping the parent, then some action is required (resizing). And it will resize the font until 6px, as it will be impossible to read after that. Instead, it informs the text container to break anywhere when there is a text overflow, while adding hyphens for more clarity of the word break.

I have tested it with the longest known name (according to google, it is "Hubert Blaine Wolfeschlegelsteinhausenbergerdorff Sr."). And even Mr. Hubert can now see his name displayed properly on our profile page.

I have not added any tests for it, as TDD and css can get really messy; add hooks & refs into the blunder and you have a very hard test.

Closes #1730.

Web frontend checklist

  • Formatted my code with yarn format && yarn lint --fix
  • There are no warnings from yarn lint
  • There are no console warnings when running the app
  • Added any new components to storybook
  • Added tests where relevant
  • All tests pass
  • Clicked around my changes running locally and it works
  • Checked Desktop, Mobile and Tablet screen sizes

@lucaslcode lucaslcode requested a review from a team February 6, 2022 22:12
Copy link
Member

@darrenvong darrenvong left a comment

Choose a reason for hiding this comment

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

This seems a bit overkill to me. Can we not design this such that it flows over to multiple lines with only CSS?

I also have accessibility concerns over this solution - the fact this will keep shrink the user name to 6px just to fit a design is really bad for users with poor eyesight. Realistically, anything below 10px (i.e. the size of our current "caption text" in our theme) is just too small to be easily read, and undermines the fact the user name is meant to be an important text!

Also setting it against absolute pixel value will break things for user who set their default font size to something larger in the browser.

@lucaslcode
Copy link
Member

This is a really cool hook, but I think word-break: break-word (god css wrapping is a mess) might do the trick?

@urbullet
Copy link
Contributor Author

urbullet commented Feb 7, 2022 via email

@lucaslcode
Copy link
Member

just make sure to check caniuse.com - from memory the most "correct" option isn't supported on mobile ios or something (typical 🤣 )

@urbullet
Copy link
Contributor Author

urbullet commented Feb 7, 2022

I've removed the created hook, and replaced it with simple CSS properties word-break: 'break-word'; overflow-wrap: 'break-word';.

This should do the trick, it is supported across all browsers.

@urbullet urbullet self-assigned this Feb 7, 2022
@urbullet urbullet marked this pull request as draft February 7, 2022 20:19
@urbullet urbullet marked this pull request as ready for review February 7, 2022 20:57
@urbullet urbullet requested a review from a team February 7, 2022 20:57
Copy link
Member

@lucaslcode lucaslcode left a comment

Choose a reason for hiding this comment

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

Thanks so much! Feel free to merge this

@urbullet urbullet merged commit 420a1b7 into develop Feb 8, 2022
@urbullet urbullet deleted the urbullet/long-display-name-on-profile-1730 branch February 8, 2022 14:50
@aapeliv aapeliv added release notes: pending Add to stuff that should be included in release notes release notes: done Was mentioned in release notes and removed release notes: pending Add to stuff that should be included in release notes labels Feb 12, 2022
aapeliv pushed a commit that referenced this pull request Apr 20, 2024
…-on-profile-1730

Break down long names to multiple lines in profile page
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: done Was mentioned in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long display name on profile overflows container
4 participants