Swap remove/add delta counts

https://imgur.com/a/jyRdz3u

This puts removed on the left and added on the right. This matches the
diff and aligns with the natural negative-->positive axis direction.

One drawback is it puts removed line count first which is overall less
important than the added count.

Release-Notes: skip
Change-Id: I97c8303e7c1911fc9a59e78ffc2ca388b8f35d9e

2 files changed, 89 insertions, 53 deletions

diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 97b8de3913..d6e733609d 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -128,7 +128,7 @@ interface SizeBarLayout {
   maxDeleted: number;
   maxAdditionWidth: number;
   maxDeletionWidth: number;
-  deletionOffset: number;
+  additionOffset: number;
 }
 
 function createDefaultSizeBarLayout(): SizeBarLayout {
@@ -139,7 +139,7 @@ function createDefaultSizeBarLayout(): SizeBarLayout {
     maxDeleted: 0,
     maxAdditionWidth: 0,
     maxDeletionWidth: 0,
-    deletionOffset: 0,
+    additionOffset: 0,
   };
 }
 
@@ -463,13 +463,13 @@ export class GrFileList extends LitElement {
         }
         .added {
           color: var(--positive-green-text-color);
-        }
-        .removed {
-          color: var(--negative-red-text-color);
           text-align: left;
           min-width: 4em;
           padding-left: var(--spacing-s);
         }
+        .removed {
+          color: var(--negative-red-text-color);
+        }
         .drafts {
           color: var(--error-foreground);
           font-weight: var(--font-weight-bold);
@@ -1321,18 +1321,18 @@ export class GrFileList extends LitElement {
       <div class=${this.computeSizeBarsClass(file.__path)} aria-hidden="true">
         <svg width="61" height="8">
           <rect
-            x=${this.computeBarAdditionX(file, sizeBarLayout)}
+            x=${this.computeBarDeletionX(file, sizeBarLayout)}
             y="0"
             height="8"
-            fill="var(--positive-green-text-color)"
-            width=${this.computeBarAdditionWidth(file, sizeBarLayout)}
+            fill="var(--negative-red-text-color)"
+            width=${this.computeBarDeletionWidth(file, sizeBarLayout)}
           ></rect>
           <rect
-            x=${this.computeBarDeletionX(sizeBarLayout)}
+            x=${this.computeBarAdditionX(sizeBarLayout)}
             y="0"
             height="8"
-            fill="var(--negative-red-text-color)"
-            width=${this.computeBarDeletionWidth(file, sizeBarLayout)}
+            fill="var(--positive-green-text-color)"
+            width=${this.computeBarAdditionWidth(file, sizeBarLayout)}
           ></rect>
         </svg>
       </div>
@@ -1348,20 +1348,20 @@ export class GrFileList extends LitElement {
         -->
       <div class=${this.computeClass('', file.__path)}>
         <span
-          class="added"
+          class="removed"
           tabindex="0"
-          aria-label=${`${file.lines_inserted} added`}
+          aria-label=${`${file.lines_deleted} removed`}
           ?hidden=${file.binary}
         >
-          +${file.lines_inserted}
+          -${file.lines_deleted}
         </span>
         <span
-          class="removed"
+          class="added"
           tabindex="0"
-          aria-label=${`${file.lines_deleted} removed`}
+          aria-label=${`${file.lines_inserted} added`}
           ?hidden=${file.binary}
         >
-          -${file.lines_deleted}
+          +${file.lines_inserted}
         </span>
         <span
           class=${ifDefined(this.computeBinaryClass(file.size_delta))}
@@ -1542,18 +1542,18 @@ export class GrFileList extends LitElement {
         <div class="total-stats">
           <div>
             <span
-              class="added"
+              class="removed"
               tabindex="0"
-              aria-label="Total ${patchChange.inserted} lines added"
+              aria-label="Total ${patchChange.deleted} lines removed"
             >
-              +${patchChange.inserted}
+              -${patchChange.deleted}
             </span>
             <span
-              class="removed"
+              class="added"
               tabindex="0"
-              aria-label="Total ${patchChange.deleted} lines removed"
+              aria-label="Total ${patchChange.inserted} lines added"
             >
-              -${patchChange.deleted}
+              +${patchChange.inserted}
             </span>
           </div>
         </div>
@@ -1586,23 +1586,23 @@ export class GrFileList extends LitElement {
       <div class="row totalChanges">
         <div class="total-stats">
           <span
-            class="added"
-            aria-label="Total bytes inserted: ${deltaInserted}"
+            class="removed"
+            aria-label="Total bytes removed: ${deltaDeleted}"
           >
-            ${deltaInserted}
+            ${deltaDeleted}
             ${this.formatPercentage(
               patchChange.total_size,
-              patchChange.size_delta_inserted
+              patchChange.size_delta_deleted
             )}
           </span>
           <span
-            class="removed"
-            aria-label="Total bytes removed: ${deltaDeleted}"
+            class="added"
+            aria-label="Total bytes inserted: ${deltaInserted}"
           >
-            ${deltaDeleted}
+            ${deltaInserted}
             ${this.formatPercentage(
               patchChange.total_size,
-              patchChange.size_delta_deleted
+              patchChange.size_delta_inserted
             )}
           </span>
         </div>
@@ -2470,7 +2470,7 @@ export class GrFileList extends LitElement {
         (SIZE_BAR_MAX_WIDTH - SIZE_BAR_GAP_WIDTH) * ratio;
       stats.maxDeletionWidth =
         SIZE_BAR_MAX_WIDTH - SIZE_BAR_GAP_WIDTH - stats.maxAdditionWidth;
-      stats.deletionOffset = stats.maxAdditionWidth + SIZE_BAR_GAP_WIDTH;
+      stats.additionOffset = stats.maxDeletionWidth + SIZE_BAR_GAP_WIDTH;
     }
     return stats;
   }
@@ -2498,9 +2498,8 @@ export class GrFileList extends LitElement {
    * Get the x-offset of the addition bar for a file.
    * Private but used in tests.
    */
-  computeBarAdditionX(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
-    if (!file || !stats) return;
-    return stats.maxAdditionWidth - this.computeBarAdditionWidth(file, stats);
+  computeBarAdditionX(stats: SizeBarLayout) {
+    return stats.additionOffset;
   }
 
   /**
@@ -2524,9 +2523,11 @@ export class GrFileList extends LitElement {
 
   /**
    * Get the x-offset of the deletion bar for a file.
+   * Private but used in tests.
    */
-  private computeBarDeletionX(stats: SizeBarLayout) {
-    return stats.deletionOffset;
+  computeBarDeletionX(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
+    if (!file || !stats) return;
+    return stats.maxDeletionWidth - this.computeBarDeletionWidth(file, stats);
   }
 
   // Private but used in tests.
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index ab9d038fc8..b2cd4308a2 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -76,7 +76,6 @@ function createFiles(
 
 suite('gr-file-list tests', () => {
   let element: GrFileList;
-
   let saveStub: sinon.SinonStub;
 
   suite('basic tests', async () => {
@@ -106,11 +105,9 @@ suite('gr-file-list tests', () => {
       element.numFilesShown = 200;
       element.basePatchNum = PARENT;
       element.patchNum = 2 as RevisionPatchSetNum;
-      saveStub = sinon
-        .stub(element, '_saveReviewedState')
-        .callsFake(() => Promise.resolve());
-      await element.updateComplete;
+      saveStub = sinon.stub(element, '_saveReviewedState').resolves();
       element.showSizeBars = true;
+      await element.updateComplete;
       // Wait for expandedFilesChanged to complete.
       await waitEventLoop();
     });
@@ -207,14 +204,16 @@ suite('gr-file-list tests', () => {
             </div>
           </div>
           <div class="desktop" role="gridcell">
-            <div aria-hidden="true" class="sizeBars"></div>
+            <div aria-hidden="true" class="sizeBars">
+              <svg><!-- contents asserted separately below --></svg>
+            </div>
           </div>
           <div class="stats" role="gridcell">
             <div>
-              <span aria-label="9 added" class="added" tabindex="0"> +9 </span>
               <span aria-label="0 removed" class="removed" tabindex="0">
                 -0
               </span>
+              <span aria-label="9 added" class="added" tabindex="0"> +9 </span>
               <span hidden=""> +/-0 B </span>
             </div>
           </div>
@@ -262,6 +261,31 @@ suite('gr-file-list tests', () => {
           </div>
         </div>`
       );
+      // <svg> and contents are ignored by assert.dom.equal() above, so we need
+      // a separate assert.lightDom.equal() for it here.
+      const sizeBarsSVG = queryAndAssert<SVGSVGElement>(
+        element,
+        '.sizeBars > svg'
+      );
+      assert.lightDom.equal(
+        sizeBarsSVG,
+        /* HTML */ `
+          <rect
+            height="8"
+            width="0"
+            x="0"
+            y="0"
+            fill="var(--negative-red-text-color)"
+          ></rect>
+          <rect
+            height="8"
+            width="60"
+            x="1"
+            y="0"
+            fill="var(--positive-green-text-color)"
+          ></rect>
+        `
+      );
     });
 
     test('renders file paths', async () => {
@@ -1763,7 +1787,7 @@ suite('gr-file-list tests', () => {
         maxDeleted: 0,
         maxAdditionWidth: 0,
         maxDeletionWidth: 0,
-        deletionOffset: 0,
+        additionOffset: 0,
       };
 
       element.files = [];
@@ -1811,7 +1835,7 @@ suite('gr-file-list tests', () => {
         maxDeleted: 0,
         maxAdditionWidth: 60,
         maxDeletionWidth: 0,
-        deletionOffset: 60,
+        additionOffset: 60,
       };
 
       // Uses half the space when file is half the largest addition and there
@@ -1839,22 +1863,33 @@ suite('gr-file-list tests', () => {
       assert.equal(element.computeBarAdditionWidth(file, stats), 1.5);
     });
 
-    test('_computeBarAdditionX', () => {
+    test('computeBarDeletionX', () => {
       const file = {
         __path: 'foo/bar.baz',
-        lines_inserted: 5,
-        lines_deleted: 0,
+        lines_inserted: 0,
+        lines_deleted: 5,
         size: 0,
         size_delta: 0,
       };
       const stats = {
+        maxInserted: 0,
+        maxDeleted: 10,
+        maxAdditionWidth: 0,
+        maxDeletionWidth: 60,
+        additionOffset: 60,
+      };
+      assert.equal(element.computeBarDeletionX(file, stats), 30);
+    });
+
+    test('computeBarAdditionX', () => {
+      const stats = {
         maxInserted: 10,
         maxDeleted: 0,
         maxAdditionWidth: 60,
         maxDeletionWidth: 0,
-        deletionOffset: 60,
+        additionOffset: 60,
       };
-      assert.equal(element.computeBarAdditionX(file, stats), 30);
+      assert.equal(element.computeBarAdditionX(stats), 60);
     });
 
     test('computeBarDeletionWidth', () => {
@@ -1870,7 +1905,7 @@ suite('gr-file-list tests', () => {
         maxDeleted: 10,
         maxAdditionWidth: 30,
         maxDeletionWidth: 30,
-        deletionOffset: 31,
+        additionOffset: 31,
       };
 
       // Uses a quarter the space when file is half the largest deletions and
@@ -1898,7 +1933,7 @@ suite('gr-file-list tests', () => {
       assert.equal(element.computeBarDeletionWidth(file, stats), 1.5);
     });
 
-    test('_computeSizeBarsClass', () => {
+    test('computeSizeBarsClass', () => {
       element.showSizeBars = false;
       assert.equal(
         element.computeSizeBarsClass('foo/bar.baz'),