Merge branch '196630-commit-tab' into 'master'

Don't hide Commit tab in Web IDE when there are no changes yet

See merge request gitlab-org/gitlab!32979

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

 <script>
 import $ from 'jquery';
-import { mapActions, mapGetters, mapState } from 'vuex';
+import { mapActions, mapState } from 'vuex';
 import Icon from '~/vue_shared/components/icon.vue';
 import tooltip from '~/vue_shared/directives/tooltip';
 import { leftSidebarViews } from '../constants';
@@ -13,7 +13,6 @@ export default {
     tooltip,
   },
   computed: {
-    ...mapGetters(['someUncommittedChanges']),
     ...mapState(['currentActivityView']),
   },
   methods: {
@@ -69,7 +68,7 @@ export default {
           <icon name="file-modified" />
         </button>
       </li>
-      <li v-show="someUncommittedChanges">
+      <li>
         <button
           v-tooltip
           :class="{

app/assets/javascripts/ide/components/commit_sidebar/form.vue

@@ -40,20 +40,9 @@ export default {
     },
   },
   watch: {
-    currentActivityView() {
-      if (this.lastCommitMsg) {
-        this.isCompact = false;
-      } else {
-        this.isCompact = !(
-          this.currentViewIsCommitView && window.innerHeight >= MAX_WINDOW_HEIGHT_COMPACT
-        );
-      }
-    },
-
-    lastCommitMsg() {
-      this.isCompact =
-        this.currentActivityView !== leftSidebarViews.commit.name && this.lastCommitMsg === '';
-    },
+    currentActivityView: 'handleCompactState',
+    someUncommittedChanges: 'handleCompactState',
+    lastCommitMsg: 'handleCompactState',
   },
   methods: {
     ...mapActions(['updateActivityBarView']),
@@ -71,18 +60,23 @@ export default {
     forceCreateNewBranch() {
       return this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH).then(() => this.commit());
     },
+    handleCompactState() {
+      if (this.lastCommitMsg) {
+        this.isCompact = false;
+      } else {
+        this.isCompact =
+          !this.someUncommittedChanges ||
+          !this.currentViewIsCommitView ||
+          window.innerHeight < MAX_WINDOW_HEIGHT_COMPACT;
+      }
+    },
     toggleIsCompact() {
-      if (this.currentViewIsCommitView) {
       this.isCompact = !this.isCompact;
-      } else {
-        this.updateActivityBarView(leftSidebarViews.commit.name)
-          .then(() => {
+    },
+    beginCommit() {
+      return this.updateActivityBarView(leftSidebarViews.commit.name).then(() => {
         this.isCompact = false;
-          })
-          .catch(e => {
-            throw e;
       });
-      }
     },
     beforeEnterTransition() {
       const elHeight = this.isCompact
@@ -129,13 +123,14 @@ export default {
           :disabled="!someUncommittedChanges"
           type="button"
           class="btn btn-primary btn-sm btn-block qa-begin-commit-button"
-          @click="toggleIsCompact"
+          data-testid="begin-commit-button"
+          @click="beginCommit"
         >
           {{ __('Commit…') }}
         </button>
         <p class="text-center bold">{{ overviewText }}</p>
       </div>
-      <form v-if="!isCompact" ref="formEl" @submit.prevent.stop="commit">
+      <form v-else ref="formEl" @submit.prevent.stop="commit">
         <transition name="fade"> <success-message v-show="lastCommitMsg" /> </transition>
         <commit-message-field
           :text="commitMessage"

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

@@ -3,7 +3,7 @@ import { mapState, mapActions, mapGetters } from 'vuex';
 import tooltip from '~/vue_shared/directives/tooltip';
 import CommitFilesList from './commit_sidebar/list.vue';
 import EmptyState from './commit_sidebar/empty_state.vue';
-import { leftSidebarViews, stageKeys } from '../constants';
+import { stageKeys } from '../constants';
 
 export default {
   components: {
@@ -25,18 +25,17 @@ export default {
       return this.activeFile ? this.activeFile.key : null;
     },
   },
-  watch: {
-    someUncommittedChanges() {
-      if (!this.someUncommittedChanges) {
-        this.updateActivityBarView(leftSidebarViews.edit.name);
-      }
-    },
-  },
   mounted() {
-    if (this.lastOpenedFile && this.lastOpenedFile.type !== 'tree') {
+    const file =
+      this.lastOpenedFile && this.lastOpenedFile.type !== 'tree'
+        ? this.lastOpenedFile
+        : this.activeFile;
+
+    if (!file) return;
+
     this.openPendingTab({
-        file: this.lastOpenedFile,
-        keyPrefix: this.lastOpenedFile.staged ? stageKeys.staged : stageKeys.unstaged,
+      file,
+      keyPrefix: file.staged ? stageKeys.staged : stageKeys.unstaged,
     })
       .then(changeViewer => {
         if (changeViewer) {
@@ -46,7 +45,6 @@ export default {
       .catch(e => {
         throw e;
       });
-    }
   },
   methods: {
     ...mapActions(['openPendingTab', 'updateViewer', 'updateActivityBarView']),

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

@@ -54,7 +54,6 @@ export default {
       'getStagedFile',
       'isEditModeActive',
       'isCommitModeActive',
-      'isReviewModeActive',
       'currentBranch',
     ]),
     ...mapGetters('fileTemplates', ['showFileTemplatesBar']),
@@ -235,7 +234,7 @@ export default {
         if (this.viewer === viewerTypes.edit) {
           this.editor.createInstance(this.$refs.editor);
         } else {
-          this.editor.createDiffInstance(this.$refs.editor, !this.isReviewModeActive);
+          this.editor.createDiffInstance(this.$refs.editor);
         }
 
         this.setupEditor();

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

@@ -37,6 +37,10 @@ export default class Editor {
       ...defaultEditorOptions,
       ...options,
     };
+    this.diffOptions = {
+      ...defaultDiffEditorOptions,
+      ...options,
+    };
 
     setupThemes();
     registerLanguages(...languages);
@@ -66,18 +70,14 @@ export default class Editor {
     }
   }
 
-  createDiffInstance(domElement, readOnly = true) {
+  createDiffInstance(domElement) {
     if (!this.instance) {
       clearDomElement(domElement);
 
       this.disposable.add(
         (this.instance = monacoEditor.createDiffEditor(domElement, {
-          ...this.options,
-          ...defaultDiffEditorOptions,
+          ...this.diffOptions,
           renderSideBySide: Editor.renderSideBySide(domElement),
-          readOnly,
-          renderLineHighlight: readOnly ? 'all' : 'none',
-          hideCursorInOverviewRuler: !readOnly,
         })),
       );
 

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

@@ -14,9 +14,13 @@ export const defaultDiffOptions = {
 };
 
 export const defaultDiffEditorOptions = {
+  ...defaultEditorOptions,
   quickSuggestions: false,
   occurrencesHighlight: false,
   ignoreTrimWhitespace: false,
+  readOnly: false,
+  renderLineHighlight: 'none',
+  hideCursorInOverviewRuler: true,
 };
 
 export const defaultModelOptions = {

app/assets/stylesheets/page_bundles/_ide_monaco_overrides.scss

@@ -27,8 +27,7 @@
     z-index: 2;
   }
 
-  .is-readonly,
-  .editor.original {
+  .is-readonly .editor.original {
     .view-lines {
       cursor: default;
     }

changelogs/unreleased/196630-commit-tab.yml

+---
+title: Don't hide Commit tab in Web IDE when there are no changes yet
+merge_request: 32979
+author:
+type: added

spec/features/ide/user_commits_changes_spec.rb

@@ -30,14 +30,4 @@ describe 'IDE user commits changes', :js do
     expect(project.repository.blob_at('master', 'foo/bar/.gitkeep')).to be_nil
     expect(project.repository.blob_at('master', 'foo/bar/lorem_ipsum.md').data).to eql(content)
   end
-
-  it 'user adds then deletes new file' do
-    ide_create_new_file('foo/bar/lorem_ipsum.md')
-
-    expect(page).to have_selector(ide_commit_tab_selector)
-
-    ide_delete_file('foo/bar/lorem_ipsum.md')
-
-    expect(page).not_to have_selector(ide_commit_tab_selector)
-  end
 end

spec/frontend/ide/components/commit_sidebar/form_spec.js

@@ -5,11 +5,14 @@ import store from '~/ide/stores';
 import CommitForm from '~/ide/components/commit_sidebar/form.vue';
 import { leftSidebarViews } from '~/ide/constants';
 import { resetStore } from '../../helpers';
+import waitForPromises from 'helpers/wait_for_promises';
 
 describe('IDE commit form', () => {
   const Component = Vue.extend(CommitForm);
   let vm;
 
+  const beginCommitButton = () => vm.$el.querySelector('[data-testid="begin-commit-button"]');
+
   beforeEach(() => {
     store.state.changedFiles.push('test');
     store.state.currentProjectId = 'abcproject';
@@ -25,8 +28,15 @@ describe('IDE commit form', () => {
     resetStore(vm.$store);
   });
 
-  it('enables button when has changes', () => {
-    expect(vm.$el.querySelector('[disabled]')).toBe(null);
+  it('enables begin commit button when there are changes', () => {
+    expect(beginCommitButton()).not.toHaveAttr('disabled');
+  });
+
+  it('disables begin commit button when there are no changes', async () => {
+    store.state.changedFiles = [];
+    await vm.$nextTick();
+
+    expect(beginCommitButton()).toHaveAttr('disabled');
   });
 
   describe('compact', () => {
@@ -37,8 +47,8 @@ describe('IDE commit form', () => {
     });
 
     it('renders commit button in compact mode', () => {
-      expect(vm.$el.querySelector('.btn-primary')).not.toBeNull();
-      expect(vm.$el.querySelector('.btn-primary').textContent).toContain('Commit');
+      expect(beginCommitButton()).not.toBeNull();
+      expect(beginCommitButton().textContent).toContain('Commit');
     });
 
     it('does not render form', () => {
@@ -54,7 +64,7 @@ describe('IDE commit form', () => {
     });
 
     it('shows form when clicking commit button', () => {
-      vm.$el.querySelector('.btn-primary').click();
+      beginCommitButton().click();
 
       return vm.$nextTick(() => {
         expect(vm.$el.querySelector('form')).not.toBeNull();
@@ -62,7 +72,7 @@ describe('IDE commit form', () => {
     });
 
     it('toggles activity bar view when clicking commit button', () => {
-      vm.$el.querySelector('.btn-primary').click();
+      beginCommitButton().click();
 
       return vm.$nextTick(() => {
         expect(store.state.currentActivityView).toBe(leftSidebarViews.commit.name);
@@ -83,6 +93,97 @@ describe('IDE commit form', () => {
       // collapsed when set to empty
       expect(vm.isCompact).toBe(true);
     });
+
+    it('collapses if in commit view but there are no changes and vice versa', async () => {
+      store.state.currentActivityView = leftSidebarViews.commit.name;
+      await vm.$nextTick();
+
+      // expanded by default if there are changes
+      expect(vm.isCompact).toBe(false);
+
+      store.state.changedFiles = [];
+      await vm.$nextTick();
+
+      expect(vm.isCompact).toBe(true);
+
+      store.state.changedFiles.push('test');
+      await vm.$nextTick();
+
+      // uncollapsed once again
+      expect(vm.isCompact).toBe(false);
+    });
+
+    it('collapses if switched from commit view to edit view and vice versa', async () => {
+      store.state.currentActivityView = leftSidebarViews.edit.name;
+      await vm.$nextTick();
+
+      expect(vm.isCompact).toBe(true);
+
+      store.state.currentActivityView = leftSidebarViews.commit.name;
+      await vm.$nextTick();
+
+      expect(vm.isCompact).toBe(false);
+
+      store.state.currentActivityView = leftSidebarViews.edit.name;
+      await vm.$nextTick();
+
+      expect(vm.isCompact).toBe(true);
+    });
+
+    describe('when window height is less than MAX_WINDOW_HEIGHT', () => {
+      let oldHeight;
+
+      beforeEach(() => {
+        oldHeight = window.innerHeight;
+        window.innerHeight = 700;
+      });
+
+      afterEach(() => {
+        window.innerHeight = oldHeight;
+      });
+
+      it('stays collapsed when switching from edit view to commit view and back', async () => {
+        store.state.currentActivityView = leftSidebarViews.edit.name;
+        await vm.$nextTick();
+
+        expect(vm.isCompact).toBe(true);
+
+        store.state.currentActivityView = leftSidebarViews.commit.name;
+        await vm.$nextTick();
+
+        expect(vm.isCompact).toBe(true);
+
+        store.state.currentActivityView = leftSidebarViews.edit.name;
+        await vm.$nextTick();
+
+        expect(vm.isCompact).toBe(true);
+      });
+
+      it('stays uncollapsed if changes are added or removed', async () => {
+        store.state.currentActivityView = leftSidebarViews.commit.name;
+        await vm.$nextTick();
+
+        expect(vm.isCompact).toBe(true);
+
+        store.state.changedFiles = [];
+        await vm.$nextTick();
+
+        expect(vm.isCompact).toBe(true);
+
+        store.state.changedFiles.push('test');
+        await vm.$nextTick();
+
+        expect(vm.isCompact).toBe(true);
+      });
+
+      it('uncollapses when clicked on Commit button in the edit view', async () => {
+        store.state.currentActivityView = leftSidebarViews.edit.name;
+        beginCommitButton().click();
+        await waitForPromises();
+
+        expect(vm.isCompact).toBe(false);
+      });
+    });
   });
 
   describe('full', () => {
@@ -113,7 +214,7 @@ describe('IDE commit form', () => {
     });
 
     it('always opens itself in full view current activity view is not commit view when clicking commit button', () => {
-      vm.$el.querySelector('.btn-primary').click();
+      beginCommitButton().click();
 
       return vm.$nextTick(() => {
         expect(store.state.currentActivityView).toBe(leftSidebarViews.commit.name);

spec/frontend/ide/components/repo_commit_section_spec.js

@@ -123,6 +123,28 @@ describe('RepoCommitSection', () => {
     });
   });
 
+  describe('if nothing is changed or staged', () => {
+    beforeEach(() => {
+      setupDefaultState();
+
+      store.state.openFiles = [...Object.values(store.state.entries)];
+      store.state.openFiles[0].active = true;
+      store.state.stagedFiles = [];
+
+      createComponent();
+    });
+
+    it('opens currently active file', () => {
+      expect(store.state.openFiles.length).toBe(1);
+      expect(store.state.openFiles[0].pending).toBe(true);
+
+      expect(store.dispatch).toHaveBeenCalledWith('openPendingTab', {
+        file: store.state.entries[store.getters.activeFile.path],
+        keyPrefix: stageKeys.unstaged,
+      });
+    });
+  });
+
   describe('with unstaged file', () => {
     beforeEach(() => {
       setupDefaultState();

spec/frontend/ide/lib/editor_spec.js

@@ -81,9 +81,9 @@ describe('Multi-file editor library', () => {
         quickSuggestions: false,
         occurrencesHighlight: false,
         renderSideBySide: false,
-        readOnly: true,
-        renderLineHighlight: 'all',
-        hideCursorInOverviewRuler: false,
+        readOnly: false,
+        renderLineHighlight: 'none',
+        hideCursorInOverviewRuler: true,
       });
     });
   });