-
Notifications
You must be signed in to change notification settings - Fork 61
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 text wrapping on program summary and preview view. #1580
Conversation
@@ -223,7 +223,8 @@ private ContainerTag renderQuestionSummary( | |||
Styles.P_2, | |||
Styles.PT_4, | |||
Styles.BORDER_B, | |||
Styles.BORDER_GRAY_300); | |||
Styles.BORDER_GRAY_300) | |||
.attr("style", "word-break:break-word"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like an important improvement!
Please use the tailwind style system whenever possible. In this case I believe you can add this constant to the class list to get the same result https://github.com/seattle-uat/civiform/blob/main/universal-application-tool-0.0.1/app/views/style/Styles.java#L2526
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tailwind docs: https://tailwindcss.com/docs/word-break
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We tried this tailwind property already, noted it in the PR description above:
Tailwind's break-words property does not actually behave as expected and does not solve the issue (looks like this property is not maintained by tailwind anymore). It does not properly wrap the long word text -- The URL link still overflows off the container. Seems like others experienced this problem: tailwindlabs/tailwindcss#835
Break-words from tailwind adds this css:
{overflow-wrap: break-word;}
The custom css we use adds
{word-break: break-word}
Seems like older versions of tailwind worked properly for this use case, but the newer versions are causing this issue for folks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! So sorry for not reading the description properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries! Thanks for the review :D
Description
Bug addressed: Text does not wrap for long words, like copy/pasted URLs, in the application summary rows. We added custom CSS to break long words in order to wrap text.
There is a similar tailwind.css property - https://tailwindcss.com/docs/word-break#break-words. However, the tailwind css does not actually behave as expected and does not solve the issue (looks like this property is not maintained by tailwind anymore).
Adding Bion as a reviewer here to confirm that adding custom css in this way is safe and to recommend any alternatives.
Before:
After:
Checklist
Issue(s)
Fixes #1567