Skip to content

Force breaks on extremely long words in comments - #7163

Merged
jancborchardt merged 1 commit into
masterfrom
force-breaks-on-extremely-long-words-in-comments
Nov 17, 2017
Merged

Force breaks on extremely long words in comments#7163
jancborchardt merged 1 commit into
masterfrom
force-breaks-on-extremely-long-words-in-comments

Conversation

@danxuliu

Copy link
Copy Markdown
Member

If not broken extremely long words overflow their container; I have added the break rule to the whole comment, so it will be used too in author names (even if it is highly unlikely that any author name would be long enough to overflow...).

Before:
before-looong

After:
after-looong

@danxuliudanxuliu added 3. to review Waiting for reviews bug design Design, UI, UX, etc. labels Nov 13, 2017
@danxuliudanxuliu added this to the Nextcloud 13 milestone Nov 13, 2017
@codecov

codecovBot commented Nov 13, 2017

Copy link
Copy Markdown

Codecov Report

Merging #7163 into master will decrease coverage by 0.08%.
The diff coverage is n/a.

@@ Coverage Diff @@## master #7163 +/- ##
============================================
- Coverage 50.84% 50.75% -0.09% + Complexity 24545 24479 -66 
============================================
Files 1585 1581 -4 Lines 93796 93578 -218 Branches 1354 1353 -1 ============================================
- Hits 47687 47492 -195 + Misses 46109 46086 -23
Impacted FilesCoverage ΔComplexity Δ
lib/private/Template/JSResourceLocator.php0% <0%> (-50.91%)22% <0%> (-1%)
lib/private/Files/ObjectStore/S3ObjectTrait.php54.71% <0%> (-3.18%)11% <0%> (-2%)
lib/private/Template/ResourceLocator.php67.74% <0%> (-1.62%)25% <0%> (ø)
apps/dav/lib/CalDAV/Schedule/IMipPlugin.php72.02% <0%> (-0.98%)54% <0%> (ø)
apps/dav/lib/CalDAV/CalendarObject.php90.69% <0%> (-0.97%)20% <0%> (-6%)
lib/private/User/Database.php70.86% <0%> (-0.46%)45% <0%> (ø)
...b/private/Files/ObjectStore/ObjectStoreStorage.php66.66% <0%> (-0.28%)74% <0%> (ø)
apps/dav/composer/composer/autoload_static.php0% <0%> (ø)1% <0%> (ø)⬇️
lib/private/Template/CSSResourceLocator.php0% <0%> (ø)26% <0%> (ø)⬇️
core/templates/login.php0% <0%> (ø)0% <0%> (ø)⬇️
... and 12 more

@jancborchardt

Copy link
Copy Markdown
Member

break-all breaks all words, hence this is needed:

 -ms-word-break: break-all; word-break: break-all; word-break: break-word;

@danxuliu

Copy link
Copy Markdown
MemberAuthor

@jancborchardt Are -ms-word-break: break-all and word-break: break-word really needed?

According to Can I Useword-break: break-all is supported by Internet Explorer, so I guess that -ms-word-break: break-all would not be needed.

Can I Use also says that word-break: break-word is an unofficial value that WebKit browsers treat like word-wrap: break-word, which produces a less pleasant (in my opinion) break, as long words are moved to their own line. See:
word-wrap: break-all:
looong-break-all

word-wrap: break-word:
looong-break-word

@jancborchardt

Copy link
Copy Markdown
Member

@danxuliu try out with a longer sentence which has one really long word but a lot of regular, non-overflowing ones in it. With break-all, the regular ones will also be broken. (Yeah it's a mess)

@danxuliu

Copy link
Copy Markdown
MemberAuthor

@jancborchardt

try out with a longer sentence which has one really long word but a lot of regular, non-overflowing ones in it. With break-all, the regular ones will also be broken. (Yeah it's a mess)

Ugh, you are totally right.

Should we use word-wrap: break-word; overflow-wrap: break-word; (it seems that Internet Explorer and Edge require the use of the legacy name) or -ms-word-break: break-all; word-break: break-all; word-break: break-word;?

@jancborchardt

Copy link
Copy Markdown
Member

@danxuliu

danxuliu commented Nov 15, 2017

Copy link
Copy Markdown
MemberAuthor

@jancborchardt

I think we need both

Mmm, I have tested it in Firefox and Chromium and it seems that, in practice, word-wrap/overflow-wrap is ignored when word-break is set. So given this summary I would go with word-wrap/overflow-wrap, but you decide :-)

@jancborchardt

Copy link
Copy Markdown
Member

@danxuliu do you have a solution which both:

  1. Does not cut off words which do not need to be cut
  2. Cuts words which are in total wider than the textbox

If so, we should use that. :) If not we need to decide what is more important to us. (I would say not messing with regular words is more important.)

If not broken extremely long words overflow their container.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu
danxuliuforce-pushed the force-breaks-on-extremely-long-words-in-comments branch from 12b1be5 to 5d85af7CompareNovember 17, 2017 14:12
@danxuliu

Copy link
Copy Markdown
MemberAuthor

@danxuliu do you have a solution which both:

Me? I am just a CSS wannabe :-P You are the master here ;-)

Anyway, based on all of the above (and for latin-based scripts; I do not know about others, sorry :-( ), the summary is:

  • word-break: break-all is supported in all browsers, and it breaks everything when the end of the line is reached. Due to this any word, no matter its length, is broken at arbitrary points (syllables are not respected).

  • word-break: break-word is WebKit only, and it is virtually the same as using word-wrap: break-word; overflow-wrap: break-word;.

  • word-wrap: break-word; overflow-wrap: break-word; is supported in all browsers, and it breaks words only when they are wider than its container.

  1. Does not cut off words which do not need to be cut
  2. Cuts words which are in total wider than the textbox

If so, we should use that. :) If not we need to decide what is more important to us. (I would say not messing with regular words is more important.)

I totally agree with not messing with regular words. Therefore I guess that the winner is word-wrap: break-word; overflow-wrap: break-word; ;-) I have amended the original commit to use that instead of word-break: break-all.

The only value that I can see in word-break: break-all is that words nicely fill the remaining space in the previous line, while word-wrap/overflow-wrap: break-word always move words that do not fit in the remaining space to a new line, no matter its length, and then breaks them arbitrarily if they are wider than the container. But, as agreed, not messing with regular words is more important ;-)

@jancborchardtjancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems good - we can always enhance as we go forward. Especially as I hope CSS will improve here :D

@jancborchardt
jancborchardt merged commit 059be95 into masterNov 17, 2017
@jancborchardt
jancborchardt deleted the force-breaks-on-extremely-long-words-in-comments branch November 17, 2017 18:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbugdesignDesign, UI, UX, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danxuliu@jancborchardt@blizzz