From a3ebaac0f026411268ca1dd2856b6762acebcc23 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 6 Mar 2019 08:30:18 -0800
Subject: [PATCH] Tweak the visual style of the ">>" / "<<" depth change
 indicators slightly

Summary:
Ref T13249.

  - When a line has only increased in indent depth, don't red-fill highlight the left side of the diff. Since reading a diff //mostly// involves focusing on the right side, indent depth changes are generally visible enough without this extra hint. The extra hint can become distracting in cases where there is a large block of indent depth changes.
  - Move the markers slightly to the left, to align them with the gutter.
  - Make them slightly opaque so they're a little less prominent.

Test Plan: See screenshots.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13249

Differential Revision: https://secure.phabricator.com/D20251
---
 resources/celerity/map.php                    | 12 ++++-----
 .../DifferentialChangesetTwoUpRenderer.php    | 26 +++++++++++++------
 .../differential/changeset-view.css           |  3 +++
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 11e7fe4118..7f9aaa4995 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -11,7 +11,7 @@ return array(
     'conpherence.pkg.js' => '020aebcf',
     'core.pkg.css' => '34ce1741',
     'core.pkg.js' => '2cda17a4',
-    'differential.pkg.css' => 'ab23bd75',
+    'differential.pkg.css' => '1755a478',
     'differential.pkg.js' => '67e02996',
     'diffusion.pkg.css' => '42c75c37',
     'diffusion.pkg.js' => '91192d85',
@@ -61,7 +61,7 @@ return array(
     'rsrc/css/application/dashboard/dashboard.css' => '4267d6c6',
     'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d',
     'rsrc/css/application/differential/add-comment.css' => '7e5900d9',
-    'rsrc/css/application/differential/changeset-view.css' => 'd92bed0d',
+    'rsrc/css/application/differential/changeset-view.css' => '4193eeff',
     'rsrc/css/application/differential/core.css' => '7300a73e',
     'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b',
     'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
@@ -540,7 +540,7 @@ return array(
     'conpherence-thread-manager' => 'aec8e38c',
     'conpherence-transaction-css' => '3a3f5e7e',
     'd3' => 'd67475f5',
-    'differential-changeset-view-css' => 'd92bed0d',
+    'differential-changeset-view-css' => '4193eeff',
     'differential-core-view-css' => '7300a73e',
     'differential-revision-add-comment-css' => '7e5900d9',
     'differential-revision-comment-css' => '7dbc8d1d',
@@ -1220,6 +1220,9 @@ return array(
       'javelin-behavior',
       'javelin-uri',
     ),
+    '4193eeff' => array(
+      'phui-inline-comment-view-css',
+    ),
     '4234f572' => array(
       'syntax-default-css',
     ),
@@ -1997,9 +2000,6 @@ return array(
       'javelin-util',
       'phabricator-shaped-request',
     ),
-    'd92bed0d' => array(
-      'phui-inline-comment-view-css',
-    ),
     'da15d3dc' => array(
       'phui-oi-list-view-css',
     ),
diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
index 7efd29519e..d803e92c6c 100644
--- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
@@ -162,7 +162,20 @@ final class DifferentialChangesetTwoUpRenderer
           } else if (empty($new_lines[$ii])) {
             $o_class = 'old old-full';
           } else {
-            $o_class = 'old';
+            if (isset($depth_only[$ii])) {
+              if ($depth_only[$ii] == '>') {
+                // When a line has depth-only change, we only highlight the
+                // left side of the diff if the depth is decreasing. When the
+                // depth is increasing, the ">>" marker on the right hand side
+                // of the diff generally provides enough visibility on its own.
+
+                $o_class = '';
+              } else {
+                $o_class = 'old';
+              }
+            } else {
+              $o_class = 'old';
+            }
           }
           $o_classes = $o_class;
         }
@@ -200,13 +213,10 @@ final class DifferentialChangesetTwoUpRenderer
           } else if (empty($old_lines[$ii])) {
             $n_class = 'new new-full';
           } else {
-
-            // NOTE: At least for the moment, I'm intentionally clearing the
-            // line highlighting only on the right side of the diff when a
-            // line has only depth changes. When a block depth is decreased,
-            // this gives us a large color block on the left (to make it easy
-            // to see the depth change) but a clean diff on the right (to make
-            // it easy to pick out actual code changes).
+            // When a line has a depth-only change, never highlight it on
+            // the right side. The ">>" marker generally provides enough
+            // visibility on its own for indent depth increases, and the left
+            // side is still highlighted for indent depth decreases.
 
             if (isset($depth_only[$ii])) {
               $n_class = '';
diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css
index 6ed939a2ee..844690abd3 100644
--- a/webroot/rsrc/css/application/differential/changeset-view.css
+++ b/webroot/rsrc/css/application/differential/changeset-view.css
@@ -126,6 +126,9 @@
   background-size: 12px 12px;
   background-repeat: no-repeat;
   background-position: left center;
+  position: relative;
+  left: -8px;
+  opacity: 0.5;
 }
 
 .differential-diff td span.depth-out {