Update file list header to a single row

Uses an enum to describe how many files are expanded: - NONE - SOME - ALL When NONE: - Expand all button is visible - Collapse all button is hidden - View mode/Settings hidden When SOME: - Expand all button is visible - Collapse all button is hidden - View mode/Settings are visible - Selected view mode is highlighted in blue When ALL: - Expand all button is hidden - Collapse all button is visible - View mode/settings same as 'SOME' Change-Id: I9c4f1e302ad94de6034b9cbaffffae649f0bc6b8
2017-10-16 16:10:52 -07:00
parent 40125966e1
commit 75a20316bf
12 changed files with 241 additions and 64 deletions
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -436,6 +436,7 @@ limitations under the License.
            diff-view-mode="{{viewState.diffMode}}"
            patch-num="{{_patchRange.patchNum}}"
            base-patch-num="{{_patchRange.basePatchNum}}"
+            files-expanded="[[_filesExpanded]]"
            revisions="[[_sortedRevisions]]"
            on-open-diff-prefs="_handleOpenDiffPrefs"
            on-open-download-dialog="_handleOpenDownloadDialog"
@@ -455,6 +456,7 @@ limitations under the License.
            diff-view-mode="[[viewState.diffMode]]"
            edit-loaded="[[_editLoaded]]"
            num-files-shown="{{_numFilesShown}}"
+            files-expanded="{{_filesExpanded}}"
            file-list-increment="{{_numFilesShown}}"
            on-files-shown-changed="_setShownFiles"></gr-file-list>
      </section>
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -146,6 +146,7 @@
      // new patches. This is just the initial setting from the change view vs.
      // an update coming from the two way data binding.
      _patchNum: String,
+      _filesExpanded: String,
      _basePatchNum: String,
      _relatedChangesLoading: {
        type: Boolean,
--- a/polygerrit-ui/app/elements/change/gr-file-list-constants.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-constants.html
@@ -0,0 +1,30 @@
+<!--
+Copyright (C) 2017 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<script>
+  (function(window) {
+    'use strict';
+
+    const GrFileListConstants = window.GrFileListConstants || {};
+
+    GrFileListConstants.FilesExpandedState = {
+      ALL: 'all',
+      NONE: 'none',
+      SOME: 'some',
+    };
+
+    window.GrFileListConstants = GrFileListConstants;
+  })(window);
+</script>
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
@@ -24,7 +24,8 @@ limitations under the License.
 <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
 <link rel="import" href="../../shared/gr-select/gr-select.html">
 <link rel="import" href="../../shared/gr-button/gr-button.html">
-
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
+<link rel="import" href="../gr-file-list-constants.html">

 <dom-module id="gr-file-list-header">
  <template>
@@ -43,7 +44,6 @@ limitations under the License.
      }
      .patchInfo-header {
        background-color: #fafafa;
-        border-bottom: 1px solid #ddd;
        border-top: 1px solid #ddd;
        display: flex;
        min-height: 3.2em;
@@ -74,12 +74,6 @@ limitations under the License.
      .mobile {
        display: none;
      }
-      #diffPrefsContainer,
-      #editControlsContainer,
-      .rightControls {
-        align-self: flex-end;
-        margin: auto 0 auto auto;
-      }
      .showOnEdit {
        display: none;
      }
@@ -93,8 +87,8 @@ limitations under the License.
        align-items: center;
        display: flex;
      }
-      #modeSelect {
-        margin-left: .1em;
+      .downloadContainer {
+        margin-right: 1em;
      }
      .fileList-header {
        align-items: center;
@@ -105,12 +99,37 @@ limitations under the License.
        padding: 0 .25em;
      }
      .rightControls {
+        align-self: flex-end;
+        margin: auto 0 auto auto;
        align-items: center;
        display: flex;
        flex-wrap: wrap;
        font-weight: normal;
        justify-content: flex-end;
      }
+      #collapseBtn,
+      .expanded #expandBtn,
+      .fileViewActions{
+        display: none;
+      }
+      .expanded #expandBtn {
+        display: none;
+      }
+      gr-button.selected iron-icon {
+        color: var(--color-link);
+      }
+      .expanded #collapseBtn,
+      .openFile .fileViewActions {
+        align-items: center;
+        display: flex;
+      }
+      .fileViewActions > *:not(:last-child) {
+        margin-right: .6em;
+      }
+      .fileViewActions .separator {
+        height: 1.5em;
+        margin: 0 1em;
+      }
      .separator {
        background-color: rgba(0, 0, 0, .3);
        height: 1.5em;
@@ -162,12 +181,6 @@ limitations under the License.
            <span class="separator"></span>
            <a href$="[[changeUrl]]">Go to latest patch set</a>
          </span>
-          <span class="container downloadContainer desktop">
-            <span class="separator"></span>
-            <gr-button link
-                class="download"
-                on-tap="_handleDownloadTap">Download</gr-button>
-          </span>
          <span class="container descriptionContainer hideOnEdit">
            <span class="separator"></span>
            <gr-editable-label
@@ -180,48 +193,59 @@ limitations under the License.
                on-changed="_handleDescriptionChanged"></gr-editable-label>
          </span>
        </div>
-        <span id="diffPrefsContainer"
-            class="hideOnEdit"
-            hidden$="[[_computePrefsButtonHidden(diffPrefs, loggedIn)]]"
-            hidden>
-          <gr-button link
-              class="prefsButton desktop"
-              on-tap="_handlePrefsTap">Diff Preferences</gr-button>
-        </span>
-        <span id="editControlsContainer" class="showOnEdit">
-          <gr-edit-controls change="[[change]]"></gr-edit-controls>
-        </span>
-      </div>
-    </div>
-    <div class="fileList-header">
-      <div class="rightControls">
-        <template is="dom-if"
-            if="[[_fileListActionsVisible(shownFileCount, _maxFilesForBulkActions)]]">
-          <gr-button
-              id="expandBtn"
-              link
-              on-tap="_expandAllDiffs">Show diffs</gr-button>
-          <span class="separator"></span>
-          <gr-button
-              id="collapseBtn"
-              link
-              on-tap="_collapseAllDiffs">Hide diffs</gr-button>
-        </template>
-        <template is="dom-if"
-            if="[[!_fileListActionsVisible(shownFileCount, _maxFilesForBulkActions)]]">
-          <div class="warning">
-            Bulk actions disabled because there are too many files.
+        <div class$="rightControls [[_computeExpandedClass(filesExpanded)]]">
+          <span class="downloadContainer desktop">
+            <gr-button link
+                class="download"
+                on-tap="_handleDownloadTap">Download</gr-button>
+          </span>
+          <template is="dom-if"
+              if="[[_fileListActionsVisible(shownFileCount, _maxFilesForBulkActions)]]">
+            <gr-button
+                id="expandBtn"
+                link
+                on-tap="_expandAllDiffs">Expand All</gr-button>
+            <gr-button
+                id="collapseBtn"
+                link
+                on-tap="_collapseAllDiffs">Collapse All</gr-button>
+          </template>
+          <template is="dom-if"
+              if="[[!_fileListActionsVisible(shownFileCount, _maxFilesForBulkActions)]]">
+            <div class="warning">
+              Bulk actions disabled because there are too many files.
+            </div>
+          </template>
+          <div class="fileViewActions">
+            <span class="separator"></span>
+            <span>Diff Views:</span>
+            <gr-button
+                id="sideBySideBtn"
+                link
+                has-tooltip
+                title="Side-by-side diff"
+                class$="[[_computeSelectedClass(diffViewMode, _VIEW_MODES.SIDE_BY_SIDE)]]"
+                on-tap="_handleSideBySideTap"><iron-icon icon="gr-icons:side-by-side"></iron-icon></gr-button>
+            <gr-button
+                id="unifiedBtn"
+                link
+                has-tooltip
+                title="Unified dff"
+                class$="[[_computeSelectedClass(diffViewMode, _VIEW_MODES.UNIFIED)]]"
+                on-tap="_handleUnifiedTap"><iron-icon icon="gr-icons:unified"></iron-icon></gr-button>
+            <span id="diffPrefsContainer"
+                class="hideOnEdit"
+                hidden$="[[_computePrefsButtonHidden(diffPrefs, loggedIn)]]"
+                hidden>
+              <gr-button
+                  link
+                  has-tooltip
+                  title="Diff preferences"
+                  class="prefsButton desktop"
+                  on-tap="_handlePrefsTap"><iron-icon icon="gr-icons:settings"></iron-icon></gr-button>
+            </span>
          </div>
-        </template>
-        <span class="separator"></span>
-        <gr-select
-            id="modeSelect"
-            bind-value="{{diffViewMode}}">
-          <select>
-            <option value="SIDE_BY_SIDE">Side By Side</option>
-            <option value="UNIFIED_DIFF">Unified</option>
-          </select>
-        </gr-select>
+        </div>
      </div>
    </div>
    <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.js
@@ -40,6 +40,7 @@
      },
      patchNum: String,
      basePatchNum: String,
+      filesExpanded: String,
      revisions: Array,
      // Caps the number of files that can be shown and have the 'show diffs' /
      // 'hide diffs' buttons still be functional.
@@ -52,6 +53,15 @@
        type: Boolean,
        computed: '_computeDescriptionReadOnly(loggedIn, change, account)',
      },
+      /** @type {?} */
+      _VIEW_MODES: {
+        type: Object,
+        readOnly: true,
+        value: {
+          SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+          UNIFIED: 'UNIFIED_DIFF',
+        },
+      },
    },

    behaviors: [
@@ -59,13 +69,39 @@
    ],

    _expandAllDiffs() {
+      this._expanded = true;
      this.fire('expand-diffs');
    },

    _collapseAllDiffs() {
+      this._expanded = false;
      this.fire('collapse-diffs');
    },

+    _computeSelectedClass(diffViewMode, buttonViewMode) {
+      return buttonViewMode === diffViewMode ? 'selected' : '';
+    },
+
+    _computeExpandedClass(filesExpanded) {
+      const classes = [];
+      if (filesExpanded === GrFileListConstants.FilesExpandedState.ALL) {
+        classes.push('expanded');
+      }
+      if (filesExpanded === GrFileListConstants.FilesExpandedState.SOME ||
+            filesExpanded === GrFileListConstants.FilesExpandedState.ALL) {
+        classes.push('openFile');
+      }
+      return classes.join(' ');
+    },
+
+    _handleSideBySideTap() {
+      this.diffViewMode = this._VIEW_MODES.SIDE_BY_SIDE;
+    },
+
+    _handleUnifiedTap() {
+      this.diffViewMode = this._VIEW_MODES.UNIFIED;
+    },
+
    _computeDescriptionPlaceholder(readOnly) {
      return (readOnly ? 'No' : 'Add') + ' patchset description';
    },
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.html
@@ -191,14 +191,51 @@ limitations under the License.
    });

    test('diff mode selector is set correctly', () => {
-      const select = element.$.modeSelect;
+      const sideBySideBtn = element.$.sideBySideBtn;
+      const unifiedBtn = element.$.unifiedBtn;
      element.diffViewMode = 'SIDE_BY_SIDE';
      flushAsynchronousOperations();
-      assert.equal(select.nativeSelect.value, 'SIDE_BY_SIDE');
-
+      assert.isTrue(sideBySideBtn.classList.contains('selected'));
+      assert.isFalse(unifiedBtn.classList.contains('selected'));
      element.diffViewMode = 'UNIFIED_DIFF';
      flushAsynchronousOperations();
-      assert.equal(select.nativeSelect.value, 'UNIFIED_DIFF');
+      assert.isFalse(sideBySideBtn.classList.contains('selected'));
+      assert.isTrue(unifiedBtn.classList.contains('selected'));
+    });
+
+    test('fileViewActions are properly hidden', () => {
+      const actions = element.$$('.fileViewActions');
+      assert.equal(getComputedStyle(actions).display, 'none');
+      element.filesExpanded = GrFileListConstants.FilesExpandedState.SOME;
+      flushAsynchronousOperations();
+      assert.notEqual(getComputedStyle(actions).display, 'none');
+      element.filesExpanded = GrFileListConstants.FilesExpandedState.ALL;
+      flushAsynchronousOperations();
+      assert.notEqual(getComputedStyle(actions).display, 'none');
+      element.filesExpanded = GrFileListConstants.FilesExpandedState.NONE;
+      flushAsynchronousOperations();
+      assert.equal(getComputedStyle(actions).display, 'none');
+    });
+
+    test('expand/collapse buttons are toggled correctly', () => {
+      element.shownFileCount = 10;
+      flushAsynchronousOperations();
+      const expandBtn = element.$$('#expandBtn');
+      const collapseBtn = element.$$('#collapseBtn');
+      assert.notEqual(getComputedStyle(expandBtn).display, 'none');
+      assert.equal(getComputedStyle(collapseBtn).display, 'none');
+      element.filesExpanded = GrFileListConstants.FilesExpandedState.SOME;
+      flushAsynchronousOperations();
+      assert.notEqual(getComputedStyle(expandBtn).display, 'none');
+      assert.equal(getComputedStyle(collapseBtn).display, 'none');
+      element.filesExpanded = GrFileListConstants.FilesExpandedState.ALL;
+      flushAsynchronousOperations();
+      assert.equal(getComputedStyle(expandBtn).display, 'none');
+      assert.notEqual(getComputedStyle(collapseBtn).display, 'none');
+      element.filesExpanded = GrFileListConstants.FilesExpandedState.NONE;
+      flushAsynchronousOperations();
+      assert.notEqual(getComputedStyle(expandBtn).display, 'none');
+      assert.equal(getComputedStyle(collapseBtn).display, 'none');
    });

    test('navigateToChange called when range select changes', () => {
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -18,6 +18,7 @@ limitations under the License.
 <link rel="import" href="../../../behaviors/async-foreach-behavior/async-foreach-behavior.html">
 <link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
 <link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
+<link rel="import" href="../../../styles/shared-styles.html">
 <link rel="import" href="../../core/gr-navigation/gr-navigation.html">
 <link rel="import" href="../../core/gr-reporting/gr-reporting.html">
 <link rel="import" href="../../diff/gr-comment-api/gr-comment-api.html">
@@ -30,7 +31,7 @@ limitations under the License.
 <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
 <link rel="import" href="../../shared/gr-select/gr-select.html">
 <link rel="import" href="../../shared/gr-tooltip-content/gr-tooltip-content.html">
-<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../gr-file-list-constants.html">

 <dom-module id="gr-file-list">
  <template>
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -61,6 +61,11 @@
        type: Boolean,
        observer: '_editLoadedChanged',
      },
+      filesExpanded: {
+        type: String,
+        value: GrFileListConstants.FilesExpandedState.NONE,
+        notify: true,
+      },
      _files: {
        type: Array,
        observer: '_filesChanged',
@@ -301,6 +306,8 @@
    collapseAllDiffs() {
      this._showInlineDiffs = false;
      this._expandedFilePaths = [];
+      this.filesExpanded = this._computeExpandedFiles(
+          this._expandedFilePaths.length, this._files.length);
      this.$.diffCursor.handleDiffUpdate();
    },

@@ -814,6 +821,15 @@
          detail.path);
    },

+    _computeExpandedFiles(expandedCount, totalCount) {
+      if (expandedCount === 0) {
+        return GrFileListConstants.FilesExpandedState.NONE;
+      } else if (expandedCount === totalCount) {
+        return GrFileListConstants.FilesExpandedState.ALL;
+      }
+      return GrFileListConstants.FilesExpandedState.SOME;
+    },
+
    /**
     * Handle splices to the list of expanded file paths. If there are any new
     * entries in the expanded list, then render each diff corresponding in
@@ -824,6 +840,9 @@
    _expandedPathsChanged(record) {
      if (!record) { return; }

+      this.filesExpanded = this._computeExpandedFiles(
+          this._expandedFilePaths.length, this._files.length);
+
      // Find the paths introduced by the new index splices:
      const newPaths = record.indexSplices
          .map(splice => {
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -697,7 +697,6 @@ limitations under the License.
      assert.isTrue(element._updateDiffPreferences.called);
    });

-
    test('expanded attribute not set on path when not expanded', () => {
      element._files = [
        {__path: '/COMMIT_MSG'},
@@ -788,6 +787,29 @@ limitations under the License.
      element.push('_expandedFilePaths', path);
    });

+    test('filesExpanded value updates to correct enum', () => {
+      element._files = [{__path: 'foo.bar'}, {__path: 'baz.bar'}];
+      flushAsynchronousOperations();
+      assert.equal(element.filesExpanded,
+          GrFileListConstants.FilesExpandedState.NONE);
+      element.push('_expandedFilePaths', 'baz.bar');
+      flushAsynchronousOperations();
+      assert.equal(element.filesExpanded,
+          GrFileListConstants.FilesExpandedState.SOME);
+      element.push('_expandedFilePaths', 'foo.bar');
+      flushAsynchronousOperations();
+      assert.equal(element.filesExpanded,
+          GrFileListConstants.FilesExpandedState.ALL);
+      element.collapseAllDiffs();
+      flushAsynchronousOperations();
+      assert.equal(element.filesExpanded,
+          GrFileListConstants.FilesExpandedState.NONE);
+      element.expandAllDiffs();
+      flushAsynchronousOperations();
+      assert.equal(element.filesExpanded,
+          GrFileListConstants.FilesExpandedState.ALL);
+    });
+
    suite('_handleFileListTap', () => {
      function testForModifier(modifier) {
        const e = {preventDefault() {}};
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -116,7 +116,10 @@ limitations under the License.
        color: #aaa;
      }
    </style>
-    <paper-button raised="[[!link]]" disabled="[[disabled]]" tabindex="-1">
+    <paper-button
+        raised="[[!link]]"
+        disabled="[[disabled]]"
+        tabindex="-1">
      <content></content>
      <i class="downArrow"></i>
    </paper-button>
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -18,6 +18,7 @@
    is: 'gr-button',

    properties: {
+      tooltip: String,
      downArrow: {
        type: Boolean,
        reflectToAttribute: true,
--- a/polygerrit-ui/app/template_test_srcs/template_test.js
+++ b/polygerrit-ui/app/template_test_srcs/template_test.js
@@ -23,6 +23,7 @@ const EXTERN_NAMES = [
  'GrDiffLine',
  'GrDomHooks',
  'GrEtagDecorator',
+  'GrFileListConstants',
  'GrGapiAuth',
  'GrGerritAuth',
  'GrLinkTextParser',