From bd5985e67db6a22232a50dff99643c49b06ce079 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 5 Mar 2015 14:11:36 -0800 Subject: [PATCH] Add "Copy" and "Coverage" columns to unified view Summary: These aren't being populated yet; they mostly fix some JS errors with inlines. For example, the inline hover reticle relies on adjusting its width to account for the "copy" column, and failed when the column did not exist. Test Plan: - Hovering inlines in unified now works, mostly. - Interacted with inlines in side-by-side. Reviewers: chad, btrahan Reviewed By: btrahan Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D11985 --- resources/celerity/map.php | 28 ++++++------- .../DifferentialChangesetOneUpRenderer.php | 41 +++++++++++++++---- .../render/DifferentialChangesetRenderer.php | 16 ++++++-- .../differential/changeset-view.css | 4 ++ .../behavior-edit-inline-comments.js | 1 + 5 files changed, 63 insertions(+), 27 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index da15078aec..4bc2103883 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,8 +10,8 @@ return array( 'core.pkg.css' => 'a9770fbb', 'core.pkg.js' => 'a77025a1', 'darkconsole.pkg.js' => '8ab24e01', - 'differential.pkg.css' => 'd8866ed8', - 'differential.pkg.js' => '4db30ad2', + 'differential.pkg.css' => '6641cdd5', + 'differential.pkg.js' => 'e260829c', 'diffusion.pkg.css' => '591664fa', 'diffusion.pkg.js' => 'bfc0737b', 'maniphest.pkg.css' => '68d4dd3d', @@ -55,7 +55,7 @@ return array( 'rsrc/css/application/dashboard/dashboard.css' => '17937d22', 'rsrc/css/application/diff/inline-comment-summary.css' => 'eb5f8e8c', 'rsrc/css/application/differential/add-comment.css' => 'c478bcaa', - 'rsrc/css/application/differential/changeset-view.css' => 'b600950c', + 'rsrc/css/application/differential/changeset-view.css' => 'bad09138', 'rsrc/css/application/differential/core.css' => '7ac3cabc', 'rsrc/css/application/differential/results-table.css' => '181aa9d9', 'rsrc/css/application/differential/revision-comment.css' => '48186045', @@ -367,7 +367,7 @@ return array( 'rsrc/js/application/differential/behavior-comment-preview.js' => '6932def3', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', 'rsrc/js/application/differential/behavior-dropdown-menus.js' => '2035b9cb', - 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '65936067', + 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => 'f159658c', 'rsrc/js/application/differential/behavior-keyboard-nav.js' => '2c426492', 'rsrc/js/application/differential/behavior-populate.js' => '8694b1df', 'rsrc/js/application/differential/behavior-show-field-details.js' => 'bba9eedf', @@ -518,7 +518,7 @@ return array( 'conpherence-notification-css' => '04a6e10a', 'conpherence-update-css' => '1099a660', 'conpherence-widget-pane-css' => '3d575438', - 'differential-changeset-view-css' => 'b600950c', + 'differential-changeset-view-css' => 'bad09138', 'differential-core-view-css' => '7ac3cabc', 'differential-inline-comment-editor' => '6a049cf7', 'differential-results-table-css' => '181aa9d9', @@ -569,7 +569,7 @@ return array( 'javelin-behavior-differential-comment-jump' => '4fdb476d', 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', 'javelin-behavior-differential-dropdown-menus' => '2035b9cb', - 'javelin-behavior-differential-edit-inline-comments' => '65936067', + 'javelin-behavior-differential-edit-inline-comments' => 'f159658c', 'javelin-behavior-differential-feedback-preview' => '6932def3', 'javelin-behavior-differential-keyboard-navigation' => '2c426492', 'javelin-behavior-differential-populate' => '8694b1df', @@ -1224,14 +1224,6 @@ return array( 'javelin-dom', 'javelin-fx', ), - 65936067 => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-util', - 'javelin-vector', - 'differential-inline-comment-editor', - ), '6882e80a' => array( 'javelin-dom', ), @@ -1882,6 +1874,14 @@ return array( 'javelin-install', 'javelin-util', ), + 'f159658c' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-util', + 'javelin-vector', + 'differential-inline-comment-editor', + ), 'f24f3253' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index f9768766a8..29a3654a74 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -19,6 +19,7 @@ final class DifferentialChangesetOneUpRenderer return phutil_tag('colgroup', array(), array( phutil_tag('col', array('class' => 'num')), phutil_tag('col', array('class' => 'num')), + phutil_tag('col', array('class' => 'copy')), phutil_tag('col', array('class' => 'unified')), )); } @@ -32,6 +33,11 @@ final class DifferentialChangesetOneUpRenderer list($left_prefix, $right_prefix) = $this->getLineIDPrefixes(); + $no_copy = phutil_tag('td', array('class' => 'copy')); + $no_coverage = null; + + $column_width = 4; + $out = array(); foreach ($primitives as $p) { $type = $p['type']; @@ -54,7 +60,9 @@ final class DifferentialChangesetOneUpRenderer $out[] = phutil_tag('th', array('id' => $left_id), $p['line']); $out[] = phutil_tag('th', array()); + $out[] = $no_copy; $out[] = phutil_tag('td', array('class' => $class), $p['render']); + $out[] = $no_coverage; } else { if ($p['htype']) { $class = 'right new'; @@ -76,26 +84,41 @@ final class DifferentialChangesetOneUpRenderer } $out[] = phutil_tag('th', array('id' => $right_id), $p['line']); + + $out[] = $no_copy; $out[] = phutil_tag('td', array('class' => $class), $p['render']); + $out[] = $no_coverage; } $out[] = hsprintf(''); break; case 'inline': - $out[] = hsprintf(''); - $out[] = hsprintf(''); - $inline = $this->buildInlineComment( $p['comment'], $p['right']); $inline->setBuildScaffolding(false); - $out[] = $inline->render(); - $out[] = hsprintf(''); + $out[] = phutil_tag( + 'tr', + array(), + array( + phutil_tag('th'), + phutil_tag('th'), + $no_copy, + phutil_tag('td', array(), $inline), + $no_coverage, + )); break; case 'no-context': - $out[] = hsprintf( - '%s', - pht('Context not available.')); + $out[] = phutil_tag( + 'tr', + array(), + phutil_tag( + 'td', + array( + 'class' => 'show-more', + 'colspan' => $column_width, + ), + pht('Context not available.'))); break; case 'context': $top = $p['top']; @@ -112,7 +135,7 @@ final class DifferentialChangesetOneUpRenderer 'td', array( 'class' => 'show-more', - 'colspan' => 3, + 'colspan' => $column_width, ), $links)); break; diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index 0a52ab6e11..38176992ff 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -533,12 +533,20 @@ abstract class DifferentialChangesetRenderer { $new_buf = array(); $out[] = array($primitive); } else if ($type == 'inline') { - $out[] = $old_buf; - $out[] = $new_buf; + + // If this inline is on the left side, put it after the old lines. + if (!$primitive['right']) { + $out[] = $old_buf; + $out[] = array($primitive); + $out[] = $new_buf; + } else { + $out[] = $old_buf; + $out[] = $new_buf; + $out[] = array($primitive); + } + $old_buf = array(); $new_buf = array(); - - $out[] = array($primitive); } else { throw new Exception("Unknown primitive type '{$primitive}'!"); } diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 6559197c74..400f88e578 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -39,6 +39,10 @@ width: 49.25%; } +.differential-diff.diff-1up col.unified { + width: 99.5%; +} + .differential-diff col.copy { width: 0.5%; } diff --git a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js index 6dbc43c2eb..940f1e0b56 100644 --- a/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js +++ b/webroot/rsrc/js/application/differential/behavior-edit-inline-comments.js @@ -188,6 +188,7 @@ JX.behavior('differential-edit-inline-comments', function(config) { var td = JX.DOM.findAbove(comment, 'td'); var th = td.previousSibling; + // TODO: For one-up views, this is incorrect! var new_part = isNewFile(th) ? 'N' : 'O'; var prefix = 'C' + id_part + new_part + 'L';