Merge branch 'ide-commit-box-highlight' into 'master'

Improve web IDE commit input Closes #44832 See merge request gitlab-org/gitlab-ce!18389
2018-04-18 11:43:49 +00:00 · 2018-04-18 11:43:49 +00:00 · d411fb8299
commit d411fb8299
parent 019c0d5761 e90261588a
9 changed files with 503 additions and 169 deletions
--- a/app/assets/javascripts/ide/components/commit_sidebar/actions.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/actions.vue
@ -1,41 +1,27 @@
 <script>
-  import { mapState } from 'vuex';
-  import { sprintf, __ } from '~/locale';
-  import * as consts from '../../stores/modules/commit/constants';
-  import RadioGroup from './radio_group.vue';
+import { mapState } from 'vuex';
+import { sprintf, __ } from '~/locale';
+import * as consts from '../../stores/modules/commit/constants';
+import RadioGroup from './radio_group.vue';

-  export default {
-    components: {
-      RadioGroup,
+export default {
+  components: {
+    RadioGroup,
+  },
+  computed: {
+    ...mapState(['currentBranchId']),
+    commitToCurrentBranchText() {
+      return sprintf(
+        __('Commit to %{branchName} branch'),
+        { branchName: `<strong class="monospace">${this.currentBranchId}</strong>` },
+        false,
+      );
    },
-    computed: {
-      ...mapState([
-        'currentBranchId',
-      ]),
-      newMergeRequestHelpText() {
-        return sprintf(
-          __('Creates a new branch from %{branchName} and re-directs to create a new merge request'),
-          { branchName: this.currentBranchId },
-        );
-      },
-      commitToCurrentBranchText() {
-        return sprintf(
-          __('Commit to %{branchName} branch'),
-          { branchName: `<strong>${this.currentBranchId}</strong>` },
-          false,
-        );
-      },
-      commitToNewBranchText() {
-        return sprintf(
-          __('Creates a new branch from %{branchName}'),
-          { branchName: this.currentBranchId },
-        );
-      },
-    },
-    commitToCurrentBranch: consts.COMMIT_TO_CURRENT_BRANCH,
-    commitToNewBranch: consts.COMMIT_TO_NEW_BRANCH,
-    commitToNewBranchMR: consts.COMMIT_TO_NEW_BRANCH_MR,
-  };
+  },
+  commitToCurrentBranch: consts.COMMIT_TO_CURRENT_BRANCH,
+  commitToNewBranch: consts.COMMIT_TO_NEW_BRANCH,
+  commitToNewBranchMR: consts.COMMIT_TO_NEW_BRANCH_MR,
+};
 </script>

 <template>
@ -53,13 +39,11 @@
      :value="$options.commitToNewBranch"
      :label="__('Create a new branch')"
      :show-input="true"
-      :help-text="commitToNewBranchText"
    />
    <radio-group
      :value="$options.commitToNewBranchMR"
      :label="__('Create a new branch and merge request')"
      :show-input="true"
-      :help-text="newMergeRequestHelpText"
    />
  </div>
 </template>
--- a/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue
@ -0,0 +1,130 @@
+<script>
+import { __, sprintf } from '../../../locale';
+import Icon from '../../../vue_shared/components/icon.vue';
+import popover from '../../../vue_shared/directives/popover';
+import { MAX_TITLE_LENGTH, MAX_BODY_LENGTH } from '../../constants';
+
+export default {
+  directives: {
+    popover,
+  },
+  components: {
+    Icon,
+  },
+  props: {
+    text: {
+      type: String,
+      required: true,
+    },
+  },
+  data() {
+    return {
+      scrollTop: 0,
+      isFocused: false,
+    };
+  },
+  computed: {
+    allLines() {
+      return this.text.split('\n').map((line, i) => ({
+        text: line.substr(0, this.getLineLength(i)) || ' ',
+        highlightedText: line.substr(this.getLineLength(i)),
+      }));
+    },
+  },
+  methods: {
+    handleScroll() {
+      if (this.$refs.textarea) {
+        this.$nextTick(() => {
+          this.scrollTop = this.$refs.textarea.scrollTop;
+        });
+      }
+    },
+    getLineLength(i) {
+      return i === 0 ? MAX_TITLE_LENGTH : MAX_BODY_LENGTH;
+    },
+    onInput(e) {
+      this.$emit('input', e.target.value);
+    },
+    updateIsFocused(isFocused) {
+      this.isFocused = isFocused;
+    },
+  },
+  popoverOptions: {
+    trigger: 'hover',
+    placement: 'top',
+    content: sprintf(
+      __(`
+        The character highligher helps you keep the subject line to %{titleLength} characters
+        and wrap the body at %{bodyLength} so they are readable in git.
+      `),
+      { titleLength: MAX_TITLE_LENGTH, bodyLength: MAX_BODY_LENGTH },
+    ),
+  },
+};
+</script>
+
+<template>
+  <fieldset class="common-note-form ide-commit-message-field">
+    <div
+      class="md-area"
+      :class="{
+        'is-focused': isFocused
+      }"
+    >
+      <div
+        v-once
+        class="md-header"
+      >
+        <ul class="nav-links">
+          <li>
+            {{ __('Commit Message') }}
+            <span
+              v-popover="$options.popoverOptions"
+              class="help-block prepend-left-10"
+            >
+              <icon
+                name="question"
+              />
+            </span>
+          </li>
+        </ul>
+      </div>
+      <div class="ide-commit-message-textarea-container">
+        <div class="ide-commit-message-highlights-container">
+          <div
+            class="note-textarea highlights monospace"
+            :style="{
+              transform: `translate3d(0, ${-scrollTop}px, 0)`
+            }"
+          >
+            <div
+              v-for="(line, index) in allLines"
+              :key="index"
+            >
+              <span
+                v-text="line.text"
+              >
+              </span><mark
+                v-show="line.highlightedText"
+                v-text="line.highlightedText"
+              >
+              </mark>
+            </div>
+          </div>
+        </div>
+        <textarea
+          class="note-textarea ide-commit-message-textarea"
+          name="commit-message"
+          :placeholder="__('Write a commit message...')"
+          :value="text"
+          @scroll="handleScroll"
+          @input="onInput"
+          @focus="updateIsFocused(true)"
+          @blur="updateIsFocused(false)"
+          ref="textarea"
+        >
+        </textarea>
+      </div>
+    </div>
+  </fieldset>
+</template>
--- a/app/assets/javascripts/ide/components/commit_sidebar/radio_group.vue
+++ b/app/assets/javascripts/ide/components/commit_sidebar/radio_group.vue
@ -1,52 +1,40 @@
 <script>
-  import { mapActions, mapState, mapGetters } from 'vuex';
-  import tooltip from '~/vue_shared/directives/tooltip';
+import { mapActions, mapState, mapGetters } from 'vuex';
+import tooltip from '~/vue_shared/directives/tooltip';

-  export default {
-    directives: {
-      tooltip,
+export default {
+  directives: {
+    tooltip,
+  },
+  props: {
+    value: {
+      type: String,
+      required: true,
    },
-    props: {
-      value: {
-        type: String,
-        required: true,
-      },
-      label: {
-        type: String,
-        required: false,
-        default: null,
-      },
-      checked: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
-      showInput: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
-      helpText: {
-        type: String,
-        required: false,
-        default: null,
-      },
+    label: {
+      type: String,
+      required: false,
+      default: null,
    },
-    computed: {
-      ...mapState('commit', [
-        'commitAction',
-      ]),
-      ...mapGetters('commit', [
-        'newBranchName',
-      ]),
+    checked: {
+      type: Boolean,
+      required: false,
+      default: false,
    },
-    methods: {
-      ...mapActions('commit', [
-        'updateCommitAction',
-        'updateBranchName',
-      ]),
+    showInput: {
+      type: Boolean,
+      required: false,
+      default: false,
    },
-  };
+  },
+  computed: {
+    ...mapState('commit', ['commitAction']),
+    ...mapGetters('commit', ['newBranchName']),
+  },
+  methods: {
+    ...mapActions('commit', ['updateCommitAction', 'updateBranchName']),
+  },
+};
 </script>

 <template>
@ -65,18 +53,6 @@
          {{ label }}
        </template>
        <slot v-else></slot>
-        <span
-          v-if="helpText"
-          v-tooltip
-          class="help-block inline"
-          :title="helpText"
-        >
-          <i
-            class="fa fa-question-circle"
-            aria-hidden="true"
-          >
-          </i>
-        </span>
      </span>
    </label>
    <div
@ -85,7 +61,7 @@
    >
      <input
        type="text"
-        class="form-control"
+        class="form-control monospace"
        :placeholder="newBranchName"
        @input="updateBranchName($event.target.value)"
      />
--- a/app/assets/javascripts/ide/components/repo_commit_section.vue
+++ b/app/assets/javascripts/ide/components/repo_commit_section.vue
@ -5,6 +5,7 @@ import icon from '~/vue_shared/components/icon.vue';
 import DeprecatedModal from '~/vue_shared/components/deprecated_modal.vue';
 import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import commitFilesList from './commit_sidebar/list.vue';
+import CommitMessageField from './commit_sidebar/message_field.vue';
 import * as consts from '../stores/modules/commit/constants';
 import Actions from './commit_sidebar/actions.vue';

@ -15,6 +16,7 @@ export default {
    commitFilesList,
    Actions,
    LoadingButton,
+    CommitMessageField,
  },
  directives: {
    tooltip,
@ -38,15 +40,9 @@ export default {
      'changedFiles',
    ]),
    ...mapState('commit', ['commitMessage', 'submitCommitLoading']),
-    ...mapGetters('commit', [
-      'commitButtonDisabled',
-      'discardDraftButtonDisabled',
-      'branchName',
-    ]),
+    ...mapGetters('commit', ['commitButtonDisabled', 'discardDraftButtonDisabled', 'branchName']),
    statusSvg() {
-      return this.lastCommitMsg
-        ? this.committedStateSvgPath
-        : this.noChangesStateSvgPath;
+      return this.lastCommitMsg ? this.committedStateSvgPath : this.noChangesStateSvgPath;
    },
  },
  methods: {
@ -64,9 +60,7 @@ export default {
      });
    },
    forceCreateNewBranch() {
-      return this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH).then(() =>
-        this.commitChanges(),
-      );
+      return this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH).then(() => this.commitChanges());
    },
  },
 };
@ -105,16 +99,10 @@ export default {
        @submit.prevent.stop="commitChanges"
        v-if="!rightPanelCollapsed"
      >
-        <div class="multi-file-commit-fieldset">
-          <textarea
-            class="form-control multi-file-commit-message"
-            name="commit-message"
-            :value="commitMessage"
-            :placeholder="__('Write a commit message...')"
-            @input="updateCommitMessage($event.target.value)"
-          >
-          </textarea>
-        </div>
+        <commit-message-field
+          :text="commitMessage"
+          @input="updateCommitMessage"
+        />
        <div class="clearfix prepend-top-15">
          <actions />
          <loading-button
--- a/app/assets/javascripts/ide/constants.js
+++ b/app/assets/javascripts/ide/constants.js
@ -0,0 +1,3 @@
+// Fuzzy file finder
+export const MAX_TITLE_LENGTH = 50;
+export const MAX_BODY_LENGTH = 72;
--- a/app/assets/javascripts/ide/stores/actions/project.js
+++ b/app/assets/javascripts/ide/stores/actions/project.js
@ -5,45 +5,71 @@ import * as types from '../mutation_types';
 export const getProjectData = (
  { commit, state, dispatch },
  { namespace, projectId, force = false } = {},
-) => new Promise((resolve, reject) => {
-  if (!state.projects[`${namespace}/${projectId}`] || force) {
-    commit(types.TOGGLE_LOADING, { entry: state });
-    service.getProjectData(namespace, projectId)
-    .then(res => res.data)
-    .then((data) => {
+) =>
+  new Promise((resolve, reject) => {
+    if (!state.projects[`${namespace}/${projectId}`] || force) {
      commit(types.TOGGLE_LOADING, { entry: state });
-      commit(types.SET_PROJECT, { projectPath: `${namespace}/${projectId}`, project: data });
-      if (!state.currentProjectId) commit(types.SET_CURRENT_PROJECT, `${namespace}/${projectId}`);
-      resolve(data);
-    })
-    .catch(() => {
-      flash('Error loading project data. Please try again.', 'alert', document, null, false, true);
-      reject(new Error(`Project not loaded ${namespace}/${projectId}`));
-    });
-  } else {
-    resolve(state.projects[`${namespace}/${projectId}`]);
-  }
-});
+      service
+        .getProjectData(namespace, projectId)
+        .then(res => res.data)
+        .then(data => {
+          commit(types.TOGGLE_LOADING, { entry: state });
+          commit(types.SET_PROJECT, { projectPath: `${namespace}/${projectId}`, project: data });
+          if (!state.currentProjectId)
+            commit(types.SET_CURRENT_PROJECT, `${namespace}/${projectId}`);
+          resolve(data);
+        })
+        .catch(() => {
+          flash(
+            'Error loading project data. Please try again.',
+            'alert',
+            document,
+            null,
+            false,
+            true,
+          );
+          reject(new Error(`Project not loaded ${namespace}/${projectId}`));
+        });
+    } else {
+      resolve(state.projects[`${namespace}/${projectId}`]);
+    }
+  });

 export const getBranchData = (
  { commit, state, dispatch },
  { projectId, branchId, force = false } = {},
-) => new Promise((resolve, reject) => {
-  if ((typeof state.projects[`${projectId}`] === 'undefined' ||
-        !state.projects[`${projectId}`].branches[branchId])
-        || force) {
-    service.getBranchData(`${projectId}`, branchId)
-      .then(({ data }) => {
-        const { id } = data.commit;
-        commit(types.SET_BRANCH, { projectPath: `${projectId}`, branchName: branchId, branch: data });
-        commit(types.SET_BRANCH_WORKING_REFERENCE, { projectId, branchId, reference: id });
-        resolve(data);
-      })
-      .catch(() => {
-        flash('Error loading branch data. Please try again.', 'alert', document, null, false, true);
-        reject(new Error(`Branch not loaded - ${projectId}/${branchId}`));
-      });
-  } else {
-    resolve(state.projects[`${projectId}`].branches[branchId]);
-  }
-});
+) =>
+  new Promise((resolve, reject) => {
+    if (
+      typeof state.projects[`${projectId}`] === 'undefined' ||
+      !state.projects[`${projectId}`].branches[branchId] ||
+      force
+    ) {
+      service
+        .getBranchData(`${projectId}`, branchId)
+        .then(({ data }) => {
+          const { id } = data.commit;
+          commit(types.SET_BRANCH, {
+            projectPath: `${projectId}`,
+            branchName: branchId,
+            branch: data,
+          });
+          commit(types.SET_BRANCH_WORKING_REFERENCE, { projectId, branchId, reference: id });
+          commit(types.SET_CURRENT_BRANCH, branchId);
+          resolve(data);
+        })
+        .catch(() => {
+          flash(
+            'Error loading branch data. Please try again.',
+            'alert',
+            document,
+            null,
+            false,
+            true,
+          );
+          reject(new Error(`Branch not loaded - ${projectId}/${branchId}`));
+        });
+    } else {
+      resolve(state.projects[`${projectId}`].branches[branchId]);
+    }
+  });
--- a/app/assets/stylesheets/pages/repo.scss
+++ b/app/assets/stylesheets/pages/repo.scss
@ -662,11 +662,6 @@
  }
 }

-.multi-file-commit-message.form-control {
-  height: 160px;
-  resize: none;
-}
-
 .dirty-diff {
  // !important need to override monaco inline style
  width: 4px !important;
@ -839,3 +834,74 @@
  align-items: center;
  font-weight: $gl-font-weight-bold;
 }
+
+.ide-commit-message-field {
+  height: 200px;
+  background-color: $white-light;
+
+  .md-area {
+    display: flex;
+    flex-direction: column;
+    height: 100%;
+  }
+
+  .nav-links {
+    height: 30px;
+  }
+
+  .help-block {
+    margin-top: 2px;
+    color: $blue-500;
+    cursor: pointer;
+  }
+}
+
+.ide-commit-message-textarea-container {
+  position: relative;
+  width: 100%;
+  height: 100%;
+  overflow: hidden;
+
+  .note-textarea {
+    font-family: $monospace_font;
+  }
+}
+
+.ide-commit-message-highlights-container {
+  position: absolute;
+  left: 0;
+  top: 0;
+  right: -100px;
+  bottom: 0;
+  padding-right: 100px;
+  pointer-events: none;
+  z-index: 1;
+
+  .highlights {
+    white-space: pre-wrap;
+    word-wrap: break-word;
+    color: transparent;
+  }
+
+  mark {
+    margin-left: -1px;
+    padding: 0 2px;
+    border-radius: $border-radius-small;
+    background-color: $orange-200;
+    color: transparent;
+    opacity: 0.6;
+  }
+}
+
+.ide-commit-message-textarea {
+  position: absolute;
+  left: 0;
+  top: 0;
+  right: 0;
+  bottom: 0;
+  width: 100%;
+  height: 100%;
+  z-index: 2;
+  background: transparent;
+  resize: none;
+}
--- a/spec/javascripts/ide/components/commit_sidebar/message_field_spec.js
+++ b/spec/javascripts/ide/components/commit_sidebar/message_field_spec.js
@ -0,0 +1,174 @@
+import Vue from 'vue';
+import CommitMessageField from '~/ide/components/commit_sidebar/message_field.vue';
+import createComponent from 'spec/helpers/vue_mount_component_helper';
+
+describe('IDE commit message field', () => {
+  const Component = Vue.extend(CommitMessageField);
+  let vm;
+
+  beforeEach(() => {
+    setFixtures('<div id="app"></div>');
+
+    vm = createComponent(
+      Component,
+      {
+        text: '',
+      },
+      '#app',
+    );
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+  });
+
+  it('adds is-focused class on focus', done => {
+    vm.$el.querySelector('textarea').focus();
+
+    vm.$nextTick(() => {
+      expect(vm.$el.querySelector('.is-focused')).not.toBeNull();
+
+      done();
+    });
+  });
+
+  it('removed is-focused class on blur', done => {
+    vm.$el.querySelector('textarea').focus();
+
+    vm
+      .$nextTick()
+      .then(() => {
+        expect(vm.$el.querySelector('.is-focused')).not.toBeNull();
+
+        vm.$el.querySelector('textarea').blur();
+
+        return vm.$nextTick();
+      })
+      .then(() => {
+        expect(vm.$el.querySelector('.is-focused')).toBeNull();
+
+        done();
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+
+  it('emits input event on input', () => {
+    spyOn(vm, '$emit');
+
+    const textarea = vm.$el.querySelector('textarea');
+    textarea.value = 'testing';
+
+    textarea.dispatchEvent(new Event('input'));
+
+    expect(vm.$emit).toHaveBeenCalledWith('input', 'testing');
+  });
+
+  describe('highlights', () => {
+    describe('subject line', () => {
+      it('does not highlight less than 50 characters', done => {
+        vm.text = 'text less than 50 chars';
+
+        vm
+          .$nextTick()
+          .then(() => {
+            expect(vm.$el.querySelector('.highlights span').textContent).toContain(
+              'text less than 50 chars',
+            );
+            expect(vm.$el.querySelector('mark').style.display).toBe('none');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+
+      it('highlights characters over 50 length', done => {
+        vm.text =
+          'text less than 50 chars that should not highlighted. text more than 50 should be highlighted';
+
+        vm
+          .$nextTick()
+          .then(() => {
+            expect(vm.$el.querySelector('.highlights span').textContent).toContain(
+              'text less than 50 chars that should not highlighte',
+            );
+            expect(vm.$el.querySelector('mark').style.display).not.toBe('none');
+            expect(vm.$el.querySelector('mark').textContent).toBe(
+              'd. text more than 50 should be highlighted',
+            );
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+
+    describe('body text', () => {
+      it('does not highlight body text less tan 72 characters', done => {
+        vm.text = 'subject line\nbody content';
+
+        vm
+          .$nextTick()
+          .then(() => {
+            expect(vm.$el.querySelectorAll('.highlights span').length).toBe(2);
+            expect(vm.$el.querySelectorAll('mark')[1].style.display).toBe('none');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+
+      it('highlights body text more than 72 characters', done => {
+        vm.text =
+          'subject line\nbody content that will be highlighted when it is more than 72 characters in length';
+
+        vm
+          .$nextTick()
+          .then(() => {
+            expect(vm.$el.querySelectorAll('.highlights span').length).toBe(2);
+            expect(vm.$el.querySelectorAll('mark')[1].style.display).not.toBe('none');
+            expect(vm.$el.querySelectorAll('mark')[1].textContent).toBe(' in length');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+
+      it('highlights body text & subject line', done => {
+        vm.text =
+          'text less than 50 chars that should not highlighted\nbody content that will be highlighted when it is more than 72 characters in length';
+
+        vm
+          .$nextTick()
+          .then(() => {
+            expect(vm.$el.querySelectorAll('.highlights span').length).toBe(2);
+            expect(vm.$el.querySelectorAll('mark').length).toBe(2);
+
+            expect(vm.$el.querySelectorAll('mark')[0].textContent).toContain('d');
+            expect(vm.$el.querySelectorAll('mark')[1].textContent).toBe(' in length');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+  });
+
+  describe('scrolling textarea', () => {
+    it('updates transform of highlights', done => {
+      vm.text = 'subject line\n\n\n\n\n\n\n\n\n\n\nbody content';
+
+      vm
+        .$nextTick()
+        .then(() => {
+          vm.$el.querySelector('textarea').scrollTo(0, 50);
+
+          vm.handleScroll();
+        })
+        .then(vm.$nextTick)
+        .then(() => {
+          expect(vm.scrollTop).toBe(50);
+          expect(vm.$el.querySelector('.highlights').style.transform).toBe(
+            'translate3d(0px, -50px, 0px)',
+          );
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+});
--- a/spec/javascripts/ide/components/commit_sidebar/radio_group_spec.js
+++ b/spec/javascripts/ide/components/commit_sidebar/radio_group_spec.js
@ -69,19 +69,6 @@ describe('IDE commit sidebar radio group', () => {
    });
  });

-  it('renders helpText tooltip', done => {
-    vm.helpText = 'help text';
-
-    Vue.nextTick(() => {
-      const help = vm.$el.querySelector('.help-block');
-
-      expect(help).not.toBeNull();
-      expect(help.getAttribute('data-original-title')).toBe('help text');
-
-      done();
-    });
-  });
-
  describe('with input', () => {
    beforeEach(done => {
      vm.$destroy();