|
|
Chromium Code Reviews|
Created:
3 years, 10 months ago by dpapad Modified:
3 years, 10 months ago CC:
arv+watch_chromium.org, chromium-reviews, dbeam+watch-settings_chromium.org, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMD Settings: Increase content width from 640px to 680px.
BUG=684153, 692916
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2697193005
Cr-Commit-Position: refs/heads/master@{#451190}
Committed: https://chromium.googlesource.com/chromium/src/+/876cafa176fb331cd498c895d6ccbb15f206eb04
Patch Set 1 #Patch Set 2 : Tweak #
Messages
Total messages: 18 (7 generated)
Description was changed from ========== MD Settings: Increase content width from 640px to 680px. BUG=684153,692916 ========== to ========== MD Settings: Increase content width from 640px to 680px. BUG=684153,692916 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dpapad@chromium.org changed reviewers: + scottchen@chromium.org
On 2017/02/16 18:13:52, dpapad wrote: I had made a change to set min-width on a text container to avoid orphaned english words (per Alan's request) as a % width, so it probably needs to be changed with this CL. https://codereview.chromium.org/2656563006/diff2/120001:140001/chrome/browser...
On 2017/02/16 at 19:17:51, scottchen wrote: > On 2017/02/16 18:13:52, dpapad wrote: > > I had made a change to set min-width on a text container to avoid orphaned english words (per Alan's request) as a % width, so it probably needs to be changed with this CL. > https://codereview.chromium.org/2656563006/diff2/120001:140001/chrome/browser... What is an "orphaned english text"?
On 2017/02/16 19:23:33, dpapad wrote: > On 2017/02/16 at 19:17:51, scottchen wrote: > > On 2017/02/16 18:13:52, dpapad wrote: > > > > I had made a change to set min-width on a text container to avoid orphaned > english words (per Alan's request) as a % width, so it probably needs to be > changed with this CL. > > > https://codereview.chromium.org/2656563006/diff2/120001:140001/chrome/browser... > > What is an "orphaned english text"? when something gets bumped to the next line
On 2017/02/16 19:23:33, dpapad wrote: > On 2017/02/16 at 19:17:51, scottchen wrote: > > On 2017/02/16 18:13:52, dpapad wrote: > > > > I had made a change to set min-width on a text container to avoid orphaned > english words (per Alan's request) as a % width, so it probably needs to be > changed with this CL. > > > https://codereview.chromium.org/2656563006/diff2/120001:140001/chrome/browser... > > What is an "orphaned english text"? A line that has less than 3 words. The min width was to push more text to the last line. (Alan's aware that this would only help the English text when he asked me to add that)
Description was changed from ========== MD Settings: Increase content width from 640px to 680px. BUG=684153,692916 ========== to ========== MD Settings: Increase content width from 640px to 680px. BUG=684153,692916 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
On 2017/02/16 at 19:30:15, scottchen wrote: > On 2017/02/16 19:23:33, dpapad wrote: > > On 2017/02/16 at 19:17:51, scottchen wrote: > > > On 2017/02/16 18:13:52, dpapad wrote: > > > > > > I had made a change to set min-width on a text container to avoid orphaned > > english words (per Alan's request) as a % width, so it probably needs to be > > changed with this CL. > > > > > https://codereview.chromium.org/2656563006/diff2/120001:140001/chrome/browser... > > > > What is an "orphaned english text"? > > A line that has less than 3 words. The min width was to push more text to the last line. (Alan's aware that this would only help the English text when he asked me to add that) Tweaked the percentage to have 3 words in the last line, see http://imgur.com/a/KBlhl. I must admit that the concept of a line with less than 3 words being "problematic" sounds pretty arbitrary to me.
On 2017/02/16 19:44:46, dpapad wrote: > On 2017/02/16 at 19:30:15, scottchen wrote: > > On 2017/02/16 19:23:33, dpapad wrote: > > > On 2017/02/16 at 19:17:51, scottchen wrote: > > > > On 2017/02/16 18:13:52, dpapad wrote: > > > > > > > > I had made a change to set min-width on a text container to avoid orphaned > > > english words (per Alan's request) as a % width, so it probably needs to be > > > changed with this CL. > > > > > > > > https://codereview.chromium.org/2656563006/diff2/120001:140001/chrome/browser... > > > > > > What is an "orphaned english text"? > > > > A line that has less than 3 words. The min width was to push more text to the > last line. (Alan's aware that this would only help the English text when he > asked me to add that) > > Tweaked the percentage to have 3 words in the last line, see > http://imgur.com/a/KBlhl. I must admit that the concept of a line with less than > 3 words being "problematic" sounds pretty arbitrary to me. yes, and it doesn't scale well per-language (as scottchen@ and bettes@ acknowledge)
On 2017/02/16 19:54:47, Dan Beam wrote: > On 2017/02/16 19:44:46, dpapad wrote: > > On 2017/02/16 at 19:30:15, scottchen wrote: > > > On 2017/02/16 19:23:33, dpapad wrote: > > > > On 2017/02/16 at 19:17:51, scottchen wrote: > > > > > On 2017/02/16 18:13:52, dpapad wrote: > > > > > > > > > > I had made a change to set min-width on a text container to avoid > orphaned > > > > english words (per Alan's request) as a % width, so it probably needs to > be > > > > changed with this CL. > > > > > > > > > > > > https://codereview.chromium.org/2656563006/diff2/120001:140001/chrome/browser... > > > > > > > > What is an "orphaned english text"? > > > > > > A line that has less than 3 words. The min width was to push more text to > the > > last line. (Alan's aware that this would only help the English text when he > > asked me to add that) > > > > Tweaked the percentage to have 3 words in the last line, see > > http://imgur.com/a/KBlhl. I must admit that the concept of a line with less > than > > 3 words being "problematic" sounds pretty arbitrary to me. > > yes, and it doesn't scale well per-language (as scottchen@ and bettes@ > acknowledge) LGTM, though not a committer.
dbeam@chromium.org changed reviewers: + dbeam@chromium.org
lgtm
The CQ bit was checked by dpapad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487285150393360,
"parent_rev": "19b6d116165378bc06ae204d74b5b1da58caa9ab", "commit_rev":
"876cafa176fb331cd498c895d6ccbb15f206eb04"}
Message was sent while issue was closed.
Description was changed from ========== MD Settings: Increase content width from 640px to 680px. BUG=684153,692916 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Increase content width from 640px to 680px. BUG=684153,692916 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2697193005 Cr-Commit-Position: refs/heads/master@{#451190} Committed: https://chromium.googlesource.com/chromium/src/+/876cafa176fb331cd498c895d6cc... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/876cafa176fb331cd498c895d6cc... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
