Merge branch 'ide-activity-bar-review-mode' into '44846-improve-web-ide-left-panel-and-modes'

Added review mode to new IDE sidebar

See merge request gitlab-org/gitlab-ce!18501

app/assets/javascripts/ide/components/activity_bar.vue

@@ -51,6 +51,20 @@ export default {
           />
         </button>
       </li>
+      <li>
+        <button
+          type="button"
+          class="ide-sidebar-link js-ide-review-mode"
+          :class="{
+            active: currentActivityView === $options.activityBarViews.review
+          }"
+          @click.prevent="updateActivityBarView($options.activityBarViews.review)"
+        >
+          <icon
+            name="file-modified"
+          />
+        </button>
+      </li>
       <li>
         <button
           type="button"

app/assets/javascripts/ide/components/ide_review.vue

+<script>
+import IdeTreeList from './ide_tree_list.vue';
+
+export default {
+  components: {
+    IdeTreeList,
+  },
+};
+</script>
+
+<template>
+  <ide-tree-list
+    viewer-type="diff"
+    header-class="ide-review-header"
+    :disable-action-dropdown="true"
+  >
+    <template
+      slot="header"
+    >
+      {{ __('Review') }}
+      <div class="prepend-top-5 ide-review-sub-header">
+        {{ __('Lastest changes') }}
+      </div>
+    </template>
+  </ide-tree-list>
+</template>

app/assets/javascripts/ide/components/ide_side_bar.vue

@@ -9,6 +9,7 @@ import IdeTree from './ide_tree.vue';
 import ResizablePanel from './resizable_panel.vue';
 import ActivityBar from './activity_bar.vue';
 import CommitSection from './repo_commit_section.vue';
+import IdeReview from './ide_review.vue';
 
 export default {
   components: {
@@ -21,6 +22,7 @@ export default {
     Identicon,
     CommitSection,
     IdeTree,
+    IdeReview,
   },
   computed: {
     ...mapState(['loading', 'currentBranchId', 'currentActivityView']),

app/assets/javascripts/ide/components/ide_tree.vue

 <script>
-import { mapGetters, mapState } from 'vuex';
-import Icon from '~/vue_shared/components/icon.vue';
-import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue';
-import RepoFile from './repo_file.vue';
+import { mapState, mapGetters } from 'vuex';
 import NewDropdown from './new_dropdown/index.vue';
+import IdeTreeList from './ide_tree_list.vue';
 
 export default {
   components: {
-    Icon,
-    RepoFile,
-    SkeletonLoadingContainer,
     NewDropdown,
+    IdeTreeList,
   },
   computed: {
     ...mapState(['currentBranchId']),
-    ...mapGetters(['currentProject', 'currentTree']),
+    ...mapGetters(['currentProject']),
   },
 };
 </script>
 
 <template>
-  <div
-    class="ide-file-list"
+  <ide-tree-list
+    viewer-type="editor"
   >
-    <template v-if="!currentTree || currentTree.loading">
-      <div
-        class="multi-file-loading-container"
-        v-for="n in 3"
-        :key="n"
+    <template
+      slot="header"
     >
-        <skeleton-loading-container />
-      </div>
-    </template>
-    <template v-else>
-      <header class="ide-tree-header">
       {{ __('Edit') }}
       <new-dropdown
         :project-id="currentProject.name_with_namespace"
         :branch="currentBranchId"
-          path=""
-        />
-      </header>
-      <repo-file
-        v-for="file in currentTree.tree"
-        :key="file.key"
-        :file="file"
-        :level="0"
       />
     </template>
-  </div>
+  </ide-tree-list>
 </template>

app/assets/javascripts/ide/components/ide_tree_list.vue

+<script>
+import { mapActions, mapGetters, mapState } from 'vuex';
+import Icon from '~/vue_shared/components/icon.vue';
+import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue';
+import RepoFile from './repo_file.vue';
+import NewDropdown from './new_dropdown/index.vue';
+
+export default {
+  components: {
+    Icon,
+    RepoFile,
+    SkeletonLoadingContainer,
+    NewDropdown,
+  },
+  props: {
+    viewerType: {
+      type: String,
+      required: true,
+    },
+    headerClass: {
+      type: String,
+      required: false,
+      default: null,
+    },
+    disableActionDropdown: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    ...mapState(['currentBranchId']),
+    ...mapGetters(['currentProject', 'currentTree']),
+    showLoading() {
+      return !this.currentTree || this.currentTree.loading;
+    },
+  },
+  mounted() {
+    this.updateViewer(this.viewerType);
+  },
+  methods: {
+    ...mapActions(['updateViewer']),
+  },
+};
+</script>
+
+<template>
+  <div
+    class="ide-file-list"
+  >
+    <template v-if="showLoading">
+      <div
+        class="multi-file-loading-container"
+        v-for="n in 3"
+        :key="n"
+      >
+        <skeleton-loading-container />
+      </div>
+    </template>
+    <template v-else>
+      <header
+        class="ide-tree-header"
+        :class="headerClass"
+      >
+        <slot name="header"></slot>
+      </header>
+      <repo-file
+        v-for="file in currentTree.tree"
+        :key="file.key"
+        :file="file"
+        :level="0"
+        :disable-action-dropdown="disableActionDropdown"
+      />
+    </template>
+  </div>
+</template>

app/assets/javascripts/ide/components/new_dropdown/index.vue

@@ -17,7 +17,8 @@ export default {
     },
     path: {
       type: String,
-      required: true,
+      required: false,
+      default: '',
     },
   },
   data() {

app/assets/javascripts/ide/components/repo_editor.vue

@@ -3,6 +3,7 @@
 import { mapState, mapGetters, mapActions } from 'vuex';
 import flash from '~/flash';
 import ContentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue';
+import { activityBarViews } from '../constants';
 import monacoLoader from '../monaco_loader';
 import Editor from '../lib/editor';
 import IdeFileButtons from './ide_file_buttons.vue';
@@ -19,8 +20,13 @@ export default {
     },
   },
   computed: {
-    ...mapState(['rightPanelCollapsed', 'viewer', 'delayViewerUpdated', 'panelResizing']),
-    ...mapGetters(['currentMergeRequest', 'getStagedFile']),
+    ...mapState(['rightPanelCollapsed', 'viewer', 'panelResizing', 'currentActivityView']),
+    ...mapGetters([
+      'currentMergeRequest',
+      'getStagedFile',
+      'isEditModeActive',
+      'isCommitModeActive',
+    ]),
     shouldHideEditor() {
       return this.file && this.file.binary && !this.file.content;
     },
@@ -40,6 +46,21 @@ export default {
       // Compare key to allow for files opened in review mode to be cached differently
       if (newVal.key !== this.file.key) {
         this.initMonaco();
+
+        if (this.currentActivityView !== activityBarViews.edit) {
+          this.setFileViewMode({
+            file: this.file,
+            viewMode: 'edit',
+          });
+        }
+      }
+    },
+    currentActivityView() {
+      if (this.currentActivityView !== activityBarViews.edit) {
+        this.setFileViewMode({
+          file: this.file,
+          viewMode: 'edit',
+        });
       }
     },
     rightPanelCollapsed() {
@@ -77,7 +98,6 @@ export default {
       'setFileViewMode',
       'setFileEOL',
       'updateViewer',
-      'updateDelayViewerUpdated',
     ]),
     initMonaco() {
       if (this.shouldHideEditor) return;
@@ -89,14 +109,6 @@ export default {
         baseSha: this.currentMergeRequest ? this.currentMergeRequest.baseCommitSha : '',
       })
         .then(() => {
-          const viewerPromise = this.delayViewerUpdated
-            ? this.updateViewer(this.file.pending ? 'diff' : 'editor')
-            : Promise.resolve();
-
-          return viewerPromise;
-        })
-        .then(() => {
-          this.updateDelayViewerUpdated(false);
           this.createEditorInstance();
         })
         .catch(err => {
@@ -111,7 +123,7 @@ export default {
         if (this.viewer === 'editor') {
           this.editor.createInstance(this.$refs.editor);
         } else {
-          this.editor.createDiffInstance(this.$refs.editor);
+          this.editor.createDiffInstance(this.$refs.editor, !this.isReviewModeActive);
         }
 
         this.setupEditor();
@@ -176,10 +188,11 @@ export default {
     id="ide"
     class="blob-viewer-container blob-editor-container"
   >
-    <div class="ide-mode-tabs clearfix">
+    <div class="ide-mode-tabs clearfix" >
       <ul
         class="nav-links pull-left"
-        v-if="!shouldHideEditor">
+        v-if="!shouldHideEditor && isEditModeActive"
+      >
         <li :class="editTabCSS">
           <a
             href="javascript:void(0);"
@@ -212,6 +225,9 @@ export default {
       v-show="!shouldHideEditor && file.viewMode === 'edit'"
       ref="editor"
       class="multi-file-editor-holder"
+      :class="{
+        'is-readonly': isCommitModeActive,
+      }"
     >
     </div>
     <content-viewer

app/assets/javascripts/ide/components/repo_file.vue

@@ -27,6 +27,11 @@ export default {
       type: Number,
       required: true,
     },
+    disableActionDropdown: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   computed: {
     isTree() {
@@ -55,16 +60,14 @@ export default {
     }
   },
   methods: {
-    ...mapActions(['toggleTreeOpen', 'updateDelayViewerUpdated']),
+    ...mapActions(['toggleTreeOpen']),
     clickFile() {
       // Manual Action if a tree is selected/opened
       if (this.isTree && this.$router.currentRoute.path === `/project${this.file.url}`) {
         this.toggleTreeOpen(this.file.path);
       }
 
-      return this.updateDelayViewerUpdated(true).then(() => {
       router.push(`/project${this.file.url}`);
-      });
     },
   },
 };
@@ -111,7 +114,7 @@ export default {
           />
         </span>
         <new-dropdown
-          v-if="isTree"
+          v-if="isTree && !disableActionDropdown"
           :project-id="file.projectId"
           :branch="file.branchId"
           :path="file.path"

app/assets/javascripts/ide/components/repo_tabs.vue

@@ -32,16 +32,6 @@ export default {
       default: '',
     },
   },
-  data() {
-    return {
-      showShadow: false,
-    };
-  },
-  updated() {
-    if (!this.$refs.tabsScroller) return;
-
-    this.showShadow = this.$refs.tabsScroller.scrollWidth > this.$refs.tabsScroller.offsetWidth;
-  },
   methods: {
     ...mapActions(['updateViewer', 'removePendingTab']),
     openFileViewer(viewer) {
@@ -71,12 +61,5 @@ export default {
         :tab="tab"
       />
     </ul>
-    <editor-mode
-      :viewer="viewer"
-      :show-shadow="showShadow"
-      :has-changes="hasChanges"
-      :merge-request-id="mergeRequestId"
-      @click="openFileViewer"
-    />
   </div>
 </template>

app/assets/javascripts/ide/constants.js

@@ -10,4 +10,5 @@ export const MAX_BODY_LENGTH = 72;
 export const activityBarViews = {
   edit: 'ide-tree',
   commit: 'commit-section',
+  review: 'ide-review',
 };

app/assets/javascripts/ide/lib/editor.js

@@ -61,19 +61,19 @@ export default class Editor {
     }
   }
 
-  createDiffInstance(domElement) {
+  createDiffInstance(domElement, readOnly = true) {
     if (!this.instance) {
       clearDomElement(domElement);
 
       this.disposable.add(
         (this.instance = this.monaco.editor.createDiffEditor(domElement, {
           ...defaultEditorOptions,
-          readOnly: true,
           quickSuggestions: false,
           occurrencesHighlight: false,
-          renderLineHighlight: 'none',
-          hideCursorInOverviewRuler: true,
           renderSideBySide: Editor.renderSideBySide(domElement),
+          readOnly,
+          renderLineHighlight: readOnly ? 'all' : 'none',
+          hideCursorInOverviewRuler: !readOnly,
         })),
       );
 

app/assets/javascripts/ide/stores/actions/file.js

@@ -29,7 +29,6 @@ export const closeFile = ({ commit, state, dispatch }, file) => {
         keyPrefix: nextFileToOpen.staged ? 'staged' : 'unstaged',
       });
     } else {
-      dispatch('updateDelayViewerUpdated', true);
       router.push(`/project${nextFileToOpen.url}`);
     }
   } else if (!state.openFiles.length) {

app/assets/javascripts/ide/stores/getters.js

+import { activityBarViews } from '../constants';
+
 export const activeFile = state => state.openFiles.find(file => file.active) || null;
 
 export const addedFiles = state => state.changedFiles.filter(f => f.tempFile);
@@ -52,5 +54,8 @@ export const allBlobs = state =>
 
 export const getStagedFile = state => path => state.stagedFiles.find(f => f.path === path);
 
+export const isEditModeActive = state => state.currentActivityView === activityBarViews.edit;
+export const isCommitModeActive = state => state.currentActivityView === activityBarViews.commit;
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/stylesheets/framework/gitlab_theme.scss

@@ -186,13 +186,8 @@
   }
 
   .ide-sidebar-link {
-    &:hover,
-    &:focus,
     &.active {
       color: $color-700;
-    }
-
-    &.active {
       box-shadow: inset 3px 0 $color-700;
     }
   }

app/assets/stylesheets/pages/repo.scss

@@ -159,24 +159,6 @@
       border-bottom-color: $white-light;
     }
   }
-
-  .dropdown {
-    display: flex;
-    margin-left: auto;
-    margin-bottom: 1px;
-    padding: 0 $grid-size;
-    border-left: 1px solid $white-dark;
-    background-color: $white-light;
-
-    &.shadow {
-      box-shadow: 0 0 10px $dropdown-shadow-color;
-    }
-
-    .btn {
-      margin-top: auto;
-      margin-bottom: auto;
-    }
-  }
 }
 
 .multi-file-tab {
@@ -245,6 +227,17 @@
     display: none;
   }
 
+  .is-readonly,
+  .editor.original {
+    .view-lines {
+      cursor: default;
+    }
+
+    .cursors-layer {
+      display: none;
+    }
+  }
+
   .monaco-diff-editor.vs {
     .editor.modified {
       box-shadow: none;
@@ -303,10 +296,6 @@
     .margin-view-overlays .delete-sign {
       opacity: 0.4;
     }
-
-    .cursors-layer {
-      display: none;
-    }
   }
 }
 
@@ -378,6 +367,7 @@
 
 .ide-btn-group {
   padding: $gl-padding-4 $gl-vert-padding;
+  line-height: 24px;
 }
 
 .ide-status-bar {
@@ -812,10 +802,12 @@
   }
 
   &:hover {
+    color: $gl-text-color;
     background-color: $theme-gray-100;
   }
 
   &:focus {
+    color: $gl-text-color;
     background-color: $theme-gray-200;
   }
 
@@ -958,6 +950,15 @@
   }
 }
 
+.ide-review-header {
+  flex-direction: column;
+  align-items: flex-start;
+}
+
+.ide-review-sub-header {
+  color: $gl-text-color-secondary;
+}
+
 .ide-new-modal-label {
   line-height: 34px;
 }

spec/javascripts/ide/components/activity_bar_spec.js

@@ -62,6 +62,12 @@ describe('IDE activity bar', () => {
 
       expect(vm.updateActivityBarView).toHaveBeenCalledWith(activityBarViews.commit);
     });
+
+    it('calls updateActivityBarView with review value on click', () => {
+      vm.$el.querySelector('.js-ide-review-mode').click();
+
+      expect(vm.updateActivityBarView).toHaveBeenCalledWith(activityBarViews.review);
+    });
   });
 
   describe('active item', () => {

spec/javascripts/ide/components/ide_review_spec.js

+import Vue from 'vue';
+import IdeReview from '~/ide/components/ide_review.vue';
+import store from '~/ide/stores';
+import { createComponentWithStore } from '../../helpers/vue_mount_component_helper';
+import { resetStore, file } from '../helpers';
+import { projectData } from '../mock_data';
+
+describe('IDE review mode', () => {
+  const Component = Vue.extend(IdeReview);
+  let vm;
+
+  beforeEach(() => {
+    store.state.currentProjectId = 'abcproject';
+    store.state.currentBranchId = 'master';
+    store.state.projects.abcproject = Object.assign({}, projectData);
+    Vue.set(store.state.trees, 'abcproject/master', {
+      tree: [file('fileName')],
+      loading: false,
+    });
+
+    vm = createComponentWithStore(Component, store).$mount();
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+
+    resetStore(vm.$store);
+  });
+
+  it('renders list of files', () => {
+    expect(vm.$el.textContent).toContain('fileName');
+  });
+});

spec/javascripts/ide/components/ide_tree_list_spec.js

+import Vue from 'vue';
+import IdeTreeList from '~/ide/components/ide_tree_list.vue';
+import store from '~/ide/stores';
+import { createComponentWithStore } from '../../helpers/vue_mount_component_helper';
+import { resetStore, file } from '../helpers';
+import { projectData } from '../mock_data';
+
+describe('IDE tree list', () => {
+  const Component = Vue.extend(IdeTreeList);
+  let vm;
+
+  beforeEach(() => {
+    store.state.currentProjectId = 'abcproject';
+    store.state.currentBranchId = 'master';
+    store.state.projects.abcproject = Object.assign({}, projectData);
+    Vue.set(store.state.trees, 'abcproject/master', {
+      tree: [file('fileName')],
+      loading: false,
+    });
+
+    vm = createComponentWithStore(Component, store, {
+      viewerType: 'edit',
+    });
+
+    spyOn(vm, 'updateViewer').and.callThrough();
+
+    vm.$mount();
+  });
+
+  afterEach(() => {
+    vm.$destroy();
+
+    resetStore(vm.$store);
+  });
+
+  it('updates viewer on mount', () => {
+    expect(vm.updateViewer).toHaveBeenCalledWith('edit');
+  });
+
+  it('renders loading indicator', done => {
+    store.state.trees['abcproject/master'].loading = true;
+
+    vm.$nextTick(() => {
+      expect(vm.$el.querySelector('.multi-file-loading-container')).not.toBeNull();
+      expect(vm.$el.querySelectorAll('.multi-file-loading-container').length).toBe(3);
+
+      done();
+    });
+  });
+
+  it('renders list of files', () => {
+    expect(vm.$el.textContent).toContain('fileName');
+  });
+});

spec/javascripts/ide/components/ide_tree_spec.js

@@ -28,15 +28,6 @@ describe('IdeRepoTree', () => {
     resetStore(vm.$store);
   });
 
-  it('renders loading', done => {
-    vm.currentTree.loading = true;
-
-    vm.$nextTick(() => {
-      expect(vm.$el.querySelectorAll('.multi-file-loading-container').length).toBe(3);
-      done();
-    });
-  });
-
   it('renders list of files', () => {
     expect(vm.$el.textContent).toContain('fileName');
   });

spec/javascripts/ide/components/repo_editor_spec.js

@@ -5,6 +5,7 @@ import store from '~/ide/stores';
 import repoEditor from '~/ide/components/repo_editor.vue';
 import monacoLoader from '~/ide/monaco_loader';
 import Editor from '~/ide/lib/editor';
+import { activityBarViews } from '~/ide/constants';
 import { createComponentWithStore } from '../../helpers/vue_mount_component_helper';
 import setTimeoutPromise from '../../helpers/set_timeout_promise_helper';
 import { file, resetStore } from '../helpers';
@@ -295,4 +296,30 @@ describe('RepoEditor', () => {
       });
     });
   });
+
+  describe('show tabs', () => {
+    it('shows tabs in edit mode', () => {
+      expect(vm.$el.querySelector('.nav-links')).not.toBe(null);
+    });
+
+    it('hides tabs in review mode', done => {
+      vm.$store.state.currentActivityView = activityBarViews.review;
+
+      vm.$nextTick(() => {
+        expect(vm.$el.querySelector('.nav-links')).toBe(null);
+
+        done();
+      });
+    });
+
+    it('hides tabs in commit mode', done => {
+      vm.$store.state.currentActivityView = activityBarViews.commit;
+
+      vm.$nextTick(() => {
+        expect(vm.$el.querySelector('.nav-links')).toBe(null);
+
+        done();
+      });
+    });
+  });
 });

spec/javascripts/ide/components/repo_file_spec.js

@@ -72,9 +72,47 @@ describe('RepoFile', () => {
 
     it('renders a tooltip', () => {
       expect(
-        vm.$el.querySelector('.ide-file-name span:nth-child(2)').dataset
-          .originalTitle,
+        vm.$el.querySelector('.ide-file-name span:nth-child(2)').dataset.originalTitle,
       ).toContain('Locked by testuser');
     });
   });
+
+  describe('folder', () => {
+    it('renders action dropdown', done => {
+      createComponent({
+        file: {
+          ...file('t4'),
+          type: 'tree',
+          branchId: 'master',
+          projectId: 'project',
+        },
+        level: 0,
+      });
+
+      setTimeout(() => {
+        expect(vm.$el.querySelector('.ide-new-btn')).not.toBeNull();
+
+        done();
+      });
+    });
+
+    it('disables action dropdown', done => {
+      createComponent({
+        file: {
+          ...file('t4'),
+          type: 'tree',
+          branchId: 'master',
+          projectId: 'project',
+        },
+        level: 0,
+        disableActionDropdown: true,
+      });
+
+      setTimeout(() => {
+        expect(vm.$el.querySelector('.ide-new-btn')).toBeNull();
+
+        done();
+      });
+    });
+  });
 });

spec/javascripts/ide/components/repo_tabs_spec.js

@@ -32,54 +32,4 @@ describe('RepoTabs', () => {
       done();
     });
   });
-
-  describe('updated', () => {
-    it('sets showShadow as true when scroll width is larger than width', done => {
-      const el = document.createElement('div');
-      el.innerHTML = '<div id="test-app"></div>';
-      document.body.appendChild(el);
-
-      const style = document.createElement('style');
-      style.innerText = `
-        .multi-file-tabs {
-          width: 100px;
-        }
-
-        .multi-file-tabs .list-unstyled {
-          display: flex;
-          overflow-x: auto;
-        }
-      `;
-      document.head.appendChild(style);
-
-      vm = createComponent(
-        RepoTabs,
-        {
-          files: [],
-          viewer: 'editor',
-          hasChanges: false,
-          activeFile: file('activeFile'),
-          hasMergeRequest: false,
-        },
-        '#test-app',
-      );
-
-      vm
-        .$nextTick()
-        .then(() => {
-          expect(vm.showShadow).toEqual(false);
-
-          vm.files = openedFiles;
-        })
-        .then(vm.$nextTick)
-        .then(() => {
-          expect(vm.showShadow).toEqual(true);
-
-          style.remove();
-          el.remove();
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-  });
 });

spec/javascripts/ide/lib/editor_spec.js

@@ -74,10 +74,10 @@ describe('Multi-file editor library', () => {
         scrollBeyondLastLine: false,
         quickSuggestions: false,
         occurrencesHighlight: false,
-        renderLineHighlight: 'none',
-        hideCursorInOverviewRuler: true,
         wordWrap: 'on',
         renderSideBySide: true,
+        renderLineHighlight: 'all',
+        hideCursorInOverviewRuler: false,
       });
     });
   });