From 681fa61719c2ea28661670fa4d59278c972b5480 Mon Sep 17 00:00:00 2001
From: Ben Rohlfs <brohlfs@google.com>
Date: Tue, 21 Jan 2020 12:28:10 +0100
Subject: [PATCH] Fix box-shadow of collapsed commit message

overflow:hidden unfortunately cuts off the box-shadow, so we have to
move this css rule down into gr-editable-content.

Bug: Issue 12200
Change-Id: Icd81669d21c5e5115be3f98bbc1b0dc2bcb80057
---
 .../change/gr-change-view/gr-change-view.html | 11 +++----
 .../change/gr-change-view/gr-change-view.js   | 32 +++++++++++--------
 .../gr-change-view/gr-change-view_test.html   |  8 +++--
 .../gr-editable-content.html                  |  4 +++
 4 files changed, 32 insertions(+), 23 deletions(-)

diff --git 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
index 471560dc5f..a44a7d2e0a 100644
--- 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
@@ -190,10 +190,6 @@ limitations under the License.
         height: 0;
         margin-bottom: var(--spacing-l);
       }
-      #commitMessage.collapsed {
-        max-height: 36em;
-        overflow: hidden;
-      }
       #relatedChanges.collapsed {
         margin-bottom: var(--spacing-l);
         max-height: var(--relation-chain-max-height, 2em);
@@ -449,12 +445,13 @@ limitations under the License.
               </div>
               <div
                   id="commitMessage"
-                  class$="commitMessage [[_computeCommitClass(_commitCollapsed, _latestCommitMessage)]]">
+                  class="commitMessage">
                 <gr-editable-content id="commitMessageEditor"
                     editing="[[_editingCommitMessage]]"
                     content="{{_latestCommitMessage}}"
                     storage-key="[[_computeCommitMessageKey(_change._number, _change.current_revision)]]"
-                    remove-zero-width-space>
+                    remove-zero-width-space
+                    collapsed$="[[_computeCommitMessageCollapsed(_commitCollapsed, _commitCollapsible)]]">
                   <gr-linked-text pre
                       content="[[_latestCommitMessage]]"
                       config="[[_projectConfig.commentlinks]]"
@@ -477,7 +474,7 @@ limitations under the License.
               <div
                   id="commitCollapseToggle"
                   class="collapseToggleContainer"
-                  hidden$="[[_computeCommitToggleHidden(_latestCommitMessage)]]">
+                  hidden$="[[!_commitCollapsible]]">
                 <gr-button
                     link
                     id="commitCollapseToggleButton"
diff --git 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
index 058bce85bf..5b0eb34c98 100644
--- 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
@@ -184,7 +184,8 @@
         _hideEditCommitMessage: {
           type: Boolean,
           computed: '_computeHideEditCommitMessage(_loggedIn, ' +
-            '_editingCommitMessage, _change, _editMode)',
+              '_editingCommitMessage, _change, _editMode, _commitCollapsed, ' +
+              '_commitCollapsible)',
         },
         _diffAgainst: String,
         /** @type {?string} */
@@ -243,10 +244,16 @@
           computed:
           '_computeChangeStatusChips(_change, _mergeable, _submitEnabled)',
         },
+        /** If false, then the "Show more" button was used to expand. */
         _commitCollapsed: {
           type: Boolean,
           value: true,
         },
+        /** Is the "Show more/less" button visible? */
+        _commitCollapsible: {
+          type: Boolean,
+          computed: '_computeCommitCollapsible(_latestCommitMessage)',
+        },
         _relatedChangesCollapsed: {
           type: Boolean,
           value: true,
@@ -543,10 +550,12 @@
       return this.changeStatuses(change, options);
     }
 
-    _computeHideEditCommitMessage(loggedIn, editing, change, editMode) {
+    _computeHideEditCommitMessage(
+        loggedIn, editing, change, editMode, collapsed, collapsible) {
       if (!loggedIn || editing ||
           (change && change.status === this.ChangeStatus.MERGED) ||
-          editMode) {
+          editMode ||
+          (collapsed && collapsible)) {
         return true;
       }
 
@@ -1583,9 +1592,8 @@
       return 'Change ' + changeNum;
     }
 
-    _computeCommitClass(collapsed, commitMessage) {
-      if (this._computeCommitToggleHidden(commitMessage)) { return ''; }
-      return collapsed ? 'collapsed' : '';
+    _computeCommitMessageCollapsed(collapsed, collapsible) {
+      return collapsible && collapsed;
     }
 
     _computeRelatedChangesClass(collapsed) {
@@ -1622,9 +1630,9 @@
       }
     }
 
-    _computeCommitToggleHidden(commitMessage) {
-      if (!commitMessage) { return true; }
-      return commitMessage.split('\n').length < MIN_LINES_FOR_COMMIT_COLLAPSE;
+    _computeCommitCollapsible(commitMessage) {
+      if (!commitMessage) { return false; }
+      return commitMessage.split('\n').length >= MIN_LINES_FOR_COMMIT_COLLAPSE;
     }
 
     _getOffsetHeight(element) {
@@ -1652,8 +1660,6 @@
       // bottom margin.
       const EXTRA_HEIGHT = 30;
       let newHeight;
-      const hasCommitToggle =
-          !this._computeCommitToggleHidden(this._latestCommitMessage);
 
       if (window.matchMedia(`(max-width: ${BREAKPOINT_RELATED_SMALL})`)
           .matches) {
@@ -1672,7 +1678,7 @@
             MINIMUM_RELATED_MAX_HEIGHT);
         newHeight = medRelatedHeight;
       } else {
-        if (hasCommitToggle) {
+        if (this._commitCollapsible) {
           // Make sure the content is lined up if both areas have buttons. If
           // the commit message is not collapsed, instead use the change info
           // height.
@@ -1695,7 +1701,7 @@
       stylesToUpdate['--relation-chain-max-height'] = newHeight + 'px';
 
       // Update the max-height of the relation chain to this new height.
-      if (hasCommitToggle) {
+      if (this._commitCollapsible) {
         stylesToUpdate['--related-change-btn-top-padding'] = remainder + 'px';
       }
 
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 9c23d0ec8f..1ef79cf567 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1238,14 +1238,16 @@ limitations under the License.
       });
 
       test('commitCollapseToggle functions', () => {
-        element._latestCommitMessage = _.times(31, String).join('\n');
+        element._latestCommitMessage = _.times(35, String).join('\n');
         assert.isTrue(element._commitCollapsed);
+        assert.isTrue(element._commitCollapsible);
         assert.isTrue(
-            element.$.commitMessage.classList.contains('collapsed'));
+            element.$.commitMessageEditor.hasAttribute('collapsed'));
         MockInteractions.tap(element.$.commitCollapseToggleButton);
         assert.isFalse(element._commitCollapsed);
+        assert.isTrue(element._commitCollapsible);
         assert.isFalse(
-            element.$.commitMessage.classList.contains('collapsed'));
+            element.$.commitMessageEditor.hasAttribute('collapsed'));
       });
     });
 
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html
index 69c280b43e..627f9482cf 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.html
@@ -38,6 +38,10 @@ limitations under the License.
         box-shadow: var(--elevation-level-1);
         padding: var(--spacing-m);
       }
+      :host([collapsed]) .viewer {
+        max-height: 36em;
+        overflow: hidden;
+      }
       .editor iron-autogrow-textarea {
         background-color: var(--view-background-color);
         width: 100%;