Skip to content

Commit 9fedd34

Browse files
sophiebitsepriestley
sophiebits
authored and
epriestley
committedApr 3, 2014
Break long words in differential two-up view
Summary: This should prevent long lines from making the code width different between files, which can be annoying. (And of course, it stops long lines from making a giant scrollbar too.) Test Plan: Loaded this diff in Chrome, Firefox, IE9, and IE8: {F137505} (That's a screenshot from Chrome, but it looks about the same in the other browsers.) Reviewers: chad, #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, Korvin, chad Maniphest Tasks: T2004 Differential Revision: https://secure.phabricator.com/D8686
1 parent 85394a9 commit 9fedd34

File tree

6 files changed

+74
-56
lines changed

6 files changed

+74
-56
lines changed
 

‎resources/celerity/map.php

+11-11
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
'names' =>
99
array(
1010
'core.pkg.css' => 'fb144113',
11-
'core.pkg.js' => 'e39d336b',
11+
'core.pkg.js' => '57d0e1c4',
1212
'darkconsole.pkg.js' => 'ca8671ce',
13-
'differential.pkg.css' => 'cc216438',
13+
'differential.pkg.css' => '331c38d9',
1414
'differential.pkg.js' => '11a5b750',
1515
'diffusion.pkg.css' => '3783278d',
1616
'diffusion.pkg.js' => '5b4010f4',
@@ -55,7 +55,7 @@
5555
'rsrc/css/application/countdown/timer.css' => '86b7b0a0',
5656
'rsrc/css/application/diff/inline-comment-summary.css' => '14a91639',
5757
'rsrc/css/application/differential/add-comment.css' => 'c478bcaa',
58-
'rsrc/css/application/differential/changeset-view.css' => 'd1951f43',
58+
'rsrc/css/application/differential/changeset-view.css' => 'df93fa84',
5959
'rsrc/css/application/differential/core.css' => '7ac3cabc',
6060
'rsrc/css/application/differential/results-table.css' => '239924f9',
6161
'rsrc/css/application/differential/revision-comment.css' => '48186045',
@@ -463,7 +463,7 @@
463463
'rsrc/js/core/behavior-line-linker.js' => 'bc778103',
464464
'rsrc/js/core/behavior-more.js' => '9b9197be',
465465
'rsrc/js/core/behavior-object-selector.js' => 'b4eef37b',
466-
'rsrc/js/core/behavior-oncopy.js' => 'c3e218fe',
466+
'rsrc/js/core/behavior-oncopy.js' => 'e69e4400',
467467
'rsrc/js/core/behavior-phabricator-nav.js' => 'b5842a5e',
468468
'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'c021950a',
469469
'rsrc/js/core/behavior-refresh-csrf.js' => 'c4b31646',
@@ -505,7 +505,7 @@
505505
'conpherence-notification-css' => '403cf598',
506506
'conpherence-update-css' => '1099a660',
507507
'conpherence-widget-pane-css' => '87b12e0c',
508-
'differential-changeset-view-css' => 'd1951f43',
508+
'differential-changeset-view-css' => 'df93fa84',
509509
'differential-core-view-css' => '7ac3cabc',
510510
'differential-inline-comment-editor' => 'f2441746',
511511
'differential-results-table-css' => '239924f9',
@@ -596,7 +596,7 @@
596596
'javelin-behavior-phabricator-nav' => 'b5842a5e',
597597
'javelin-behavior-phabricator-notification-example' => 'c51a6616',
598598
'javelin-behavior-phabricator-object-selector' => 'b4eef37b',
599-
'javelin-behavior-phabricator-oncopy' => 'c3e218fe',
599+
'javelin-behavior-phabricator-oncopy' => 'e69e4400',
600600
'javelin-behavior-phabricator-remarkup-assist' => 'c021950a',
601601
'javelin-behavior-phabricator-reveal-content' => '8f24abfc',
602602
'javelin-behavior-phabricator-search-typeahead' => 'f6b56f7a',
@@ -1634,11 +1634,6 @@
16341634
2 => 'javelin-util',
16351635
3 => 'javelin-magical-init',
16361636
),
1637-
'c3e218fe' =>
1638-
array(
1639-
0 => 'javelin-behavior',
1640-
1 => 'javelin-dom',
1641-
),
16421637
'c4b31646' =>
16431638
array(
16441639
0 => 'javelin-request',
@@ -1814,6 +1809,11 @@
18141809
2 => 'javelin-view-visitor',
18151810
3 => 'javelin-util',
18161811
),
1812+
'e69e4400' =>
1813+
array(
1814+
0 => 'javelin-behavior',
1815+
1 => 'javelin-dom',
1816+
),
18171817
'e7c21fb3' =>
18181818
array(
18191819
0 => 'javelin-dom',

‎src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php

+38-21
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@ public function isOneUpRenderer() {
77
return false;
88
}
99

10+
private function renderColgroup() {
11+
return phutil_tag('colgroup', array(), array(
12+
phutil_tag('col', array('class' => 'num')),
13+
phutil_tag('col', array('class' => 'left')),
14+
phutil_tag('col', array('class' => 'num')),
15+
phutil_tag('col', array('class' => 'copy')),
16+
phutil_tag('col', array('class' => 'right')),
17+
phutil_tag('col', array('class' => 'cov')),
18+
));
19+
}
20+
1021
public function renderTextChange(
1122
$range_start,
1223
$range_len,
@@ -31,6 +42,8 @@ public function renderTextChange(
3142
}
3243

3344
$html = array();
45+
$html[] = $this->renderColgroup();
46+
3447
$old_lines = $this->getOldLines();
3548
$new_lines = $this->getNewLines();
3649
$gaps = $this->getGaps();
@@ -184,22 +197,23 @@ public function renderTextChange(
184197
}
185198

186199
$o_num = null;
187-
$o_classes = 'left';
200+
$o_classes = '';
188201
$o_text = null;
189202
if (isset($old_lines[$ii])) {
190203
$o_num = $old_lines[$ii]['line'];
191204
$o_text = isset($old_render[$ii]) ? $old_render[$ii] : null;
192205
if ($old_lines[$ii]['type']) {
193206
if ($old_lines[$ii]['type'] == '\\') {
194207
$o_text = $old_lines[$ii]['text'];
195-
$o_classes .= ' comment';
208+
$o_class = 'comment';
196209
} else if ($original_left && !isset($highlight_old[$o_num])) {
197-
$o_classes .= ' old-rebase';
210+
$o_class = 'old-rebase';
198211
} else if (empty($new_lines[$ii])) {
199-
$o_classes .= ' old old-full';
212+
$o_class = 'old old-full';
200213
} else {
201-
$o_classes .= ' old';
214+
$o_class = 'old';
202215
}
216+
$o_classes = $o_class;
203217
}
204218
}
205219

@@ -265,7 +279,6 @@ public function renderTextChange(
265279
}
266280
}
267281
}
268-
$n_classes .= ' right'.$n_colspan;
269282

270283
if (isset($hunk_starts[$o_num])) {
271284
$html[] = $context_not_available;
@@ -283,10 +296,11 @@ public function renderTextChange(
283296
$n_id = null;
284297
}
285298

286-
// NOTE: This is a unicode zero-width space, which we use as a hint
287-
// when intercepting 'copy' events to make sure sensible text ends
288-
// up on the clipboard. See the 'phabricator-oncopy' behavior.
289-
$zero_space = "\xE2\x80\x8B";
299+
// NOTE: This is a unicode 'word joiner' (essentially a non-breaking
300+
// zero-width space), which we use as a hint when intercepting 'copy'
301+
// events to make sure sensible text ends up on the clipboard.
302+
// See the 'phabricator-oncopy' behavior.
303+
$zero_space = "\xE2\x81\xA0";
290304

291305
// NOTE: The Javascript is sensitive to whitespace changes in this
292306
// block!
@@ -326,9 +340,9 @@ public function renderTextChange(
326340
}
327341
$html[] = phutil_tag('tr', array('class' => 'inline'), array(
328342
phutil_tag('th', array()),
329-
phutil_tag('td', array('class' => 'left'), $comment_html),
343+
phutil_tag('td', array(), $comment_html),
330344
phutil_tag('th', array()),
331-
phutil_tag('td', array('colspan' => 3, 'class' => 'right3'), $new),
345+
phutil_tag('td', array('colspan' => 3), $new),
332346
));
333347
}
334348
}
@@ -338,11 +352,11 @@ public function renderTextChange(
338352
$on_right = true);
339353
$html[] = phutil_tag('tr', array('class' => 'inline'), array(
340354
phutil_tag('th', array()),
341-
phutil_tag('td', array('class' => 'left')),
355+
phutil_tag('td', array()),
342356
phutil_tag('th', array()),
343357
phutil_tag(
344358
'td',
345-
array('colspan' => 3, 'class' => 'right3'),
359+
array('colspan' => 3),
346360
$comment_html),
347361
));
348362
}
@@ -391,9 +405,9 @@ public function renderFileChange($old_file = null,
391405
$comment_html = $this->renderInlineComment($comment, $on_right = false);
392406
$html_old[] = phutil_tag('tr', array('class' => 'inline'), array(
393407
phutil_tag('th', array()),
394-
phutil_tag('td', array('class' => 'left'), $comment_html),
408+
phutil_tag('td', array(), $comment_html),
395409
phutil_tag('th', array()),
396-
phutil_tag('td', array('colspan' => 3, 'class' => 'right3')),
410+
phutil_tag('td', array('colspan' => 3)),
397411
));
398412
}
399413
}
@@ -402,11 +416,11 @@ public function renderFileChange($old_file = null,
402416
$comment_html = $this->renderInlineComment($comment, $on_right = true);
403417
$html_new[] = phutil_tag('tr', array('class' => 'inline'), array(
404418
phutil_tag('th', array()),
405-
phutil_tag('td', array('class' => 'left')),
419+
phutil_tag('td', array()),
406420
phutil_tag('th', array()),
407421
phutil_tag(
408422
'td',
409-
array('colspan' => 3, 'class' => 'right3'),
423+
array('colspan' => 3),
410424
$comment_html),
411425
));
412426
}
@@ -424,12 +438,15 @@ public function renderFileChange($old_file = null,
424438
$th_new = phutil_tag('th', array('id' => "C{$id}OL1"), 1);
425439
}
426440

427-
$output = hsprintf(
441+
$output = array();
442+
$output[] = $this->renderColgroup();
443+
444+
$output[] = hsprintf(
428445
'<tr class="differential-image-diff">'.
429446
'%s'.
430-
'<td class="left differential-old-image">%s</td>'.
447+
'<td class="differential-old-image">%s</td>'.
431448
'%s'.
432-
'<td class="right3 differential-new-image" colspan="3">%s</td>'.
449+
'<td class="differential-new-image" colspan="3">%s</td>'.
433450
'</tr>'.
434451
'%s'.
435452
'%s',

‎src/applications/diffusion/controller/DiffusionBrowseFileController.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ private function buildDisplayRows(
789789
$anchor_text,
790790

791791
// NOTE: See phabricator-oncopy behavior.
792-
"\xE2\x80\x8B",
792+
"\xE2\x81\xA0",
793793

794794
// TODO: [HTML] Not ideal.
795795
phutil_safe_html(str_replace("\t", ' ', $line['data'])),

‎src/view/layout/PhabricatorSourceCodeView.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public function render() {
6161
pht('...'));
6262
} else {
6363
$content_number = $line_number;
64-
$content_line = hsprintf("\xE2\x80\x8B%s", $line);
64+
// NOTE: See phabricator-oncopy behavior.
65+
$content_line = hsprintf("\xE2\x81\xA0%s", $line);
6566
}
6667

6768
$row_attributes = array();

‎webroot/rsrc/css/application/differential/changeset-view.css

+21-21
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,30 @@
1616
.differential-diff {
1717
background: #fff;
1818
width: 100%;
19+
min-width: 780px;
1920
border-top: 1px solid {$lightblueborder};
2021
border-bottom: 1px solid {$lightblueborder};
22+
table-layout: fixed;
23+
}
24+
25+
.differential-diff col.num {
26+
width: 45px;
27+
}
28+
29+
.differential-diff col.left,
30+
.differential-diff col.right {
31+
width: 49.25%;
32+
}
33+
34+
.differential-diff col.copy {
35+
width: 0.5%;
36+
}
37+
38+
.differential-diff col.cov {
39+
width: 1%;
2140
}
2241

2342
.differential-diff td {
24-
min-width: 320px;
2543
/*
2644
Disable ligatures in Firefox. Firefox 3 has fancypants ligature support, but
2745
it gets applied to monospaced fonts, which sucks because it means that the
@@ -35,16 +53,14 @@
3553
letter-spacing: 0.0083334px;
3654
vertical-align: top;
3755
white-space: pre-wrap;
56+
word-wrap: break-word;
3857
padding: 0 8px 1px;
3958
line-height: 16px;
4059
}
4160

4261
.differential-diff th {
4362
text-align: right;
4463
padding: 2px 6px 0px 0px;
45-
width: 4%;
46-
min-width: 45px;
47-
max-width: 4%;
4864
vertical-align: top;
4965
background: {$lightbluebackground};
5066
color: {$bluetext};
@@ -59,21 +75,6 @@
5975
-ms-user-select: none;
6076
user-select: none;
6177
}
62-
.differential-diff td.left {
63-
width: 45%;
64-
}
65-
66-
.differential-diff td.right,
67-
.differential-diff td.right1 {
68-
width: 43.5%;
69-
}
70-
71-
.differential-diff td.right2 {
72-
width: 44.5%;
73-
}
74-
.differential-diff td.right3 {
75-
width: 45%;
76-
}
7778

7879
.differential-changeset-immutable .differential-diff th {
7980
cursor: auto;
@@ -126,8 +127,6 @@
126127
}
127128

128129
.differential-diff td.cov {
129-
min-width: 1%;
130-
width: 1%;
131130
padding: 0;
132131
}
133132

@@ -252,6 +251,7 @@ td.cov-X {
252251
width: 100%;
253252
-moz-box-sizing: border-box;
254253
-webkit-box-sizing: border-box;
254+
box-sizing: border-box;
255255
overflow: hidden;
256256
white-space: normal;
257257
}

‎webroot/rsrc/js/core/behavior-oncopy.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717
JX.behavior('phabricator-oncopy', function() {
1818

19-
var zws = "\u200B"; // Unicode Zero-Width Space
19+
var zws = "\u2060"; // Unicode Word Joiner (Non-Breaking Zero-Width Space)
2020

2121
JX.enableDispatch(document.body, 'copy');
2222
JX.Stratcom.listen(

0 commit comments

Comments
 (0)
Failed to load comments.