Show all bar when editing commit message

- Moving show all bar to gr-editable-content - Adding Save, Cancel buttons to show all bar - textarea to block, so no space between textarea and show all bar - Set min-height for editor to 160px same as viewer The code will be cleaned up, when feature flag will be removed. Change-Id: I303e57672c88c29bce757365e2b48722458f7695
2021-02-08 08:53:33 +01:00
parent 41623da45e
commit c24dc5fb1d
5 changed files with 181 additions and 89 deletions
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -367,13 +367,6 @@ export class GrChangeView extends KeyboardShortcutMixin(
  })
  _hideEditCommitMessage?: boolean;

-  @property({
-    type: Boolean,
-    computed:
-      '_computeHideShowAllContainer(_hideEditCommitMessage, _commitCollapsible)',
-  })
-  _hideShowAllContainer = false;
-
  @property({type: String})
  _diffAgainst?: string;

@@ -918,13 +911,6 @@ export class GrChangeView extends KeyboardShortcutMixin(
    return changeStatuses(change, options);
  }

-  _computeHideShowAllContainer(
-    _hideEditCommitMessage?: boolean,
-    _commitCollapsible?: boolean
-  ) {
-    return !_commitCollapsible && _hideEditCommitMessage;
-  }
-
  _computeHideEditCommitMessage(
    loggedIn: boolean,
    editing: boolean,
@@ -2363,6 +2349,9 @@ export class GrChangeView extends KeyboardShortcutMixin(
  }

  _computeCommitMessageCollapsed(collapsed?: boolean, collapsible?: boolean) {
+    if (this._isNewChangeSummaryUiEnabled) {
+      return false;
+    }
    return collapsible && collapsed;
  }

@@ -2371,9 +2360,6 @@ export class GrChangeView extends KeyboardShortcutMixin(
  }

  _computeCollapseText(collapsed: boolean) {
-    if (this._isNewChangeSummaryUiEnabled) {
-      return collapsed ? 'Show all' : 'Show less';
-    }
    // Symbols are up and down triangles.
    return collapsed ? '\u25bc Show more' : '\u25b2 Show less';
  }
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -109,25 +109,6 @@ export const htmlTemplate = html`
      /* Account for border and padding and rounding errors. */
      max-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px);
    }
-    .show-all-container {
-      background-color: var(--view-background-color);
-      display: flex;
-      justify-content: flex-end;
-      margin-bottom: 8px;
-      border-top-width: 1px;
-      border-top-style: solid;
-      border-radius: 0 0 4px 4px;
-      border-color: var(--border-color);
-      box-shadow: var(--elevation-level-1);
-    }
-    .show-all-container .show-all-button {
-      margin-right: auto;
-    }
-    .show-all-container iron-icon {
-      color: inherit;
-      --iron-icon-height: 18px;
-      --iron-icon-width: 18px;
-    }
    .commitMessage gr-linked-text {
      word-break: break-word;
    }
@@ -139,9 +120,6 @@ export const htmlTemplate = html`
    .new-change-summary-true #commitMessageEditor {
      --collapsed-max-height: 300px;
    }
-    .new-change-summary-true gr-linked-text {
-      min-height: 160px;
-    }
    .editCommitMessage {
      margin-top: var(--spacing-l);

@@ -492,9 +470,11 @@ export const htmlTemplate = html`
              >
                <gr-editable-content
                  id="commitMessageEditor"
-                  editing="[[_editingCommitMessage]]"
+                  editing="{{_editingCommitMessage}}"
                  content="{{_latestCommitMessage}}"
                  storage-key="[[_computeCommitMessageKey(_change._number, _change.current_revision)]]"
+                  hide-edit-commit-message="[[_hideEditCommitMessage]]"
+                  commit-collapsible="[[_commitCollapsible]]"
                  remove-zero-width-space=""
                  collapsed$="[[_computeCommitMessageCollapsed(_commitCollapsed, _commitCollapsible)]]"
                >
@@ -505,37 +485,6 @@ export const htmlTemplate = html`
                    remove-zero-width-space=""
                  ></gr-linked-text>
                </gr-editable-content>
-                <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
-                  <div
-                    class="show-all-container"
-                    hidden$="[[_hideShowAllContainer]]"
-                  >
-                    <gr-button
-                      link=""
-                      class="show-all-button"
-                      on-click="_toggleCommitCollapsed"
-                      hidden$="[[!_commitCollapsible]]"
-                      ><iron-icon
-                        icon="gr-icons:expand-more"
-                        hidden$="[[!_commitCollapsed]]"
-                      ></iron-icon
-                      ><iron-icon
-                        icon="gr-icons:expand-less"
-                        hidden$="[[_commitCollapsed]]"
-                      ></iron-icon>
-                      [[_computeCollapseText(_commitCollapsed)]]
-                    </gr-button>
-                    <gr-button
-                      link=""
-                      class="edit-commit-message"
-                      title="Edit commit message"
-                      on-click="_handleEditCommitMessage"
-                      hidden$="[[_hideEditCommitMessage]]"
-                      ><iron-icon icon="gr-icons:edit"></iron-icon>
-                      Edit</gr-button
-                    >
-                  </div>
-                </template>
                <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
                  <gr-button
                    link=""
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content.ts
@@ -25,6 +25,8 @@ import {PolymerElement} from '@polymer/polymer/polymer-element';
 import {customElement, property} from '@polymer/decorators';
 import {htmlTemplate} from './gr-editable-content_html';
 import {fireAlert, fireEvent} from '../../../utils/event-util';
+import {appContext} from '../../../services/app-context';
+import {KnownExperimentId} from '../../../services/flags/flags';

 const RESTORED_MESSAGE = 'Content restored from a previous edit.';
 const STORAGE_DEBOUNCE_INTERVAL_MS = 400;
@@ -67,7 +69,7 @@ export class GrEditableContent extends GestureEventListeners(
  @property({type: Boolean, reflectToAttribute: true})
  disabled = false;

-  @property({type: Boolean, observer: '_editingChanged'})
+  @property({type: Boolean, observer: '_editingChanged', notify: true})
  editing = false;

  @property({type: Boolean})
@@ -77,6 +79,29 @@ export class GrEditableContent extends GestureEventListeners(
  @property({type: String})
  storageKey?: string;

+  /** If false, then the "Show more" button was used to expand. */
+  @property({type: Boolean})
+  _commitCollapsed = true;
+
+  @property({type: Boolean})
+  commitCollapsible = true;
+
+  @property({
+    type: Boolean,
+    computed:
+      '_computeHideShowAllContainer(hideEditCommitMessage, _hideShowAllButton, editing)',
+  })
+  _hideShowAllContainer = false;
+
+  @property({
+    type: Boolean,
+    computed: '_computeHideShowAllButton(commitCollapsible, editing)',
+  })
+  _hideShowAllButton = false;
+
+  @property({type: Boolean})
+  hideEditCommitMessage?: boolean;
+
  @property({
    type: Boolean,
    computed: '_computeSaveDisabled(disabled, content, _newContent)',
@@ -86,8 +111,21 @@ export class GrEditableContent extends GestureEventListeners(
  @property({type: String, observer: '_newContentChanged'})
  _newContent?: string;

+  @property({type: Boolean})
+  _isNewChangeSummaryUiEnabled = false;
+
  private readonly storage = new GrStorage();

+  private readonly flagsService = appContext.flagsService;
+
+  /** @override */
+  ready() {
+    super.ready();
+    this._isNewChangeSummaryUiEnabled = this.flagsService.isEnabled(
+      KnownExperimentId.NEW_CHANGE_SUMMARY_UI
+    );
+  }
+
  _contentChanged() {
    /* A changed content means that either a different change has been loaded
     * or new content was saved. Either way, let's reset the component.
@@ -186,4 +224,37 @@ export class GrEditableContent extends GestureEventListeners(
    this.editing = false;
    fireEvent(this, 'editable-content-cancel');
  }
+
+  _computeCollapseText(collapsed: boolean) {
+    return collapsed ? 'Show all' : 'Show less';
+  }
+
+  _toggleCommitCollapsed() {
+    this._commitCollapsed = !this._commitCollapsed;
+    if (this._commitCollapsed) {
+      window.scrollTo(0, 0);
+    }
+  }
+
+  _computeHideShowAllContainer(
+    hideEditCommitMessage?: boolean,
+    _hideShowAllButton?: boolean,
+    editing?: boolean
+  ) {
+    if (editing) return false;
+    return _hideShowAllButton && hideEditCommitMessage;
+  }
+
+  _computeHideShowAllButton(commitCollapsible?: boolean, editing?: boolean) {
+    return !commitCollapsible || editing;
+  }
+
+  _computeCommitMessageCollapsed(collapsed?: boolean, collapsible?: boolean) {
+    return collapsible && collapsed;
+  }
+
+  _handleEditCommitMessage() {
+    this.editing = true;
+    this.focusTextarea();
+  }
 }
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
@@ -31,13 +31,19 @@ export const htmlTemplate = html`
      box-shadow: var(--elevation-level-1);
      padding: var(--spacing-m);
    }
-    :host([collapsed]) .viewer {
+    :host([collapsed]) .viewer,
+    .viewer[collapsed] {
      max-height: var(--collapsed-max-height, 300px);
      overflow: hidden;
    }
+    .editor.new-change-summary-true iron-autogrow-textarea,
+    .viewer.new-change-summary-true {
+      min-height: 160px;
+    }
    .editor iron-autogrow-textarea {
      background-color: var(--view-background-color);
      width: 100%;
+      display: block;

      /* You have to also repeat everything from shared-styles here, because
           you can only *replace* --iron-autogrow-textarea vars as a whole. */
@@ -52,23 +58,103 @@ export const htmlTemplate = html`
      display: flex;
      justify-content: space-between;
    }
+    .show-all-container {
+      background-color: var(--view-background-color);
+      display: flex;
+      justify-content: flex-end;
+      margin-bottom: 8px;
+      border-top-width: 1px;
+      border-top-style: solid;
+      border-radius: 0 0 4px 4px;
+      border-color: var(--border-color);
+      box-shadow: var(--elevation-level-1);
+    }
+    .show-all-container .show-all-button {
+      margin-right: auto;
+    }
+    .show-all-container iron-icon {
+      color: inherit;
+      --iron-icon-height: 18px;
+      --iron-icon-width: 18px;
+    }
+    .cancel-button {
+      margin-right: var(--spacing-l);
+    }
+    .save-button {
+      margin-right: var(--spacing-xs);
+    }
  </style>
-  <div class="viewer" hidden$="[[editing]]">
+  <div
+    class$="viewer new-change-summary-[[_isNewChangeSummaryUiEnabled]]"
+    hidden$="[[editing]]"
+    collapsed$="[[_computeCommitMessageCollapsed(_commitCollapsed, commitCollapsible)]]"
+  >
    <slot></slot>
  </div>
-  <div class="editor" hidden$="[[!editing]]">
-    <iron-autogrow-textarea
-      autocomplete="on"
-      bind-value="{{_newContent}}"
-      disabled="[[disabled]]"
-    ></iron-autogrow-textarea>
-    <div class="editButtons">
-      <gr-button primary="" on-click="_handleSave" disabled="[[_saveDisabled]]"
-        >Save</gr-button
-      >
-      <gr-button on-click="_handleCancel" disabled="[[disabled]]"
-        >Cancel</gr-button
-      >
+  <div
+    class$="editor new-change-summary-[[_isNewChangeSummaryUiEnabled]]"
+    hidden$="[[!editing]]"
+  >
+    <div>
+      <iron-autogrow-textarea
+        autocomplete="on"
+        bind-value="{{_newContent}}"
+        disabled="[[disabled]]"
+      ></iron-autogrow-textarea>
+      <div class="editButtons" hidden$="[[_isNewChangeSummaryUiEnabled]]">
+        <gr-button
+          primary=""
+          on-click="_handleSave"
+          disabled="[[_saveDisabled]]"
+          >Save</gr-button
+        >
+        <gr-button on-click="_handleCancel" disabled="[[disabled]]"
+          >Cancel</gr-button
+        >
+      </div>
    </div>
  </div>
+  <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
+    <div class="show-all-container" hidden$="[[_hideShowAllContainer]]">
+      <gr-button
+        link=""
+        class="show-all-button"
+        on-click="_toggleCommitCollapsed"
+        hidden$="[[_hideShowAllButton]]"
+        ><iron-icon
+          icon="gr-icons:expand-more"
+          hidden$="[[!_commitCollapsed]]"
+        ></iron-icon
+        ><iron-icon
+          icon="gr-icons:expand-less"
+          hidden$="[[_commitCollapsed]]"
+        ></iron-icon>
+        [[_computeCollapseText(_commitCollapsed)]]
+      </gr-button>
+      <gr-button
+        link=""
+        class="edit-commit-message"
+        title="Edit commit message"
+        on-click="_handleEditCommitMessage"
+        hidden$="[[hideEditCommitMessage]]"
+        ><iron-icon icon="gr-icons:edit"></iron-icon> Edit</gr-button
+      >
+      <div class="editButtons" hidden$="[[!editing]]">
+        <gr-button
+          link=""
+          class="cancel-button"
+          on-click="_handleCancel"
+          disabled="[[disabled]]"
+          >Cancel</gr-button
+        >
+        <gr-button
+          class="save-button"
+          primary=""
+          on-click="_handleSave"
+          disabled="[[_saveDisabled]]"
+          >Save</gr-button
+        >
+      </div>
+    </div>
+  </template>
 `;
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_test.js
@@ -105,7 +105,7 @@ suite('gr-editable-content tests', () => {

      assert.equal(element._newContent, 'stored content');
      assert.isTrue(dispatchSpy.called);
-      assert.equal(dispatchSpy.lastCall.args[0].type, 'show-alert');
+      assert.equal(dispatchSpy.firstCall.args[0].type, 'show-alert');
    });

    test('editing toggled to true, has no stored data', () => {
@@ -114,7 +114,7 @@ suite('gr-editable-content tests', () => {
      element.editing = true;

      assert.equal(element._newContent, 'current content');
-      assert.isFalse(dispatchSpy.called);
+      assert.equal(dispatchSpy.firstCall.args[0].type, 'editing-changed');
    });

    test('edits are cached', () => {