Merge branch 'ide-temp-file-folder-fixes' into 'master'

Fixed bugs with IDE new directory

Closes #44838

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

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

@@ -26,11 +26,18 @@ export default {
       required: false,
       default: false,
     },
+    forceModifiedIcon: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   computed: {
     changedIcon() {
       const suffix = this.file.staged && !this.showStagedIcon ? '-solid' : '';
-      return this.file.tempFile ? `file-addition${suffix}` : `file-modified${suffix}`;
+      return this.file.tempFile && !this.forceModifiedIcon
+        ? `file-addition${suffix}`
+        : `file-modified${suffix}`;
     },
     stagedIcon() {
       return `${this.changedIcon}-solid`;

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

 <script>
-  import { mapActions } from 'vuex';
-  import icon from '~/vue_shared/components/icon.vue';
-  import newModal from './modal.vue';
-  import upload from './upload.vue';
+import { mapActions } from 'vuex';
+import icon from '~/vue_shared/components/icon.vue';
+import newModal from './modal.vue';
+import upload from './upload.vue';
 
-  export default {
-    components: {
-      icon,
-      newModal,
-      upload,
+export default {
+  components: {
+    icon,
+    newModal,
+    upload,
+  },
+  props: {
+    branch: {
+      type: String,
+      required: true,
     },
-    props: {
-      branch: {
-        type: String,
-        required: true,
-      },
-      path: {
-        type: String,
-        required: true,
-      },
+    path: {
+      type: String,
+      required: true,
     },
-    data() {
-      return {
-        openModal: false,
-        modalType: '',
-        dropdownOpen: false,
-      };
+  },
+  data() {
+    return {
+      openModal: false,
+      modalType: '',
+      dropdownOpen: false,
+    };
+  },
+  watch: {
+    dropdownOpen() {
+      this.$nextTick(() => {
+        this.$refs.dropdownMenu.scrollIntoView();
+      });
     },
-    methods: {
-      ...mapActions([
-        'createTempEntry',
-      ]),
-      createNewItem(type) {
-        this.modalType = type;
-        this.openModal = true;
-        this.dropdownOpen = false;
-      },
-      hideModal() {
-        this.openModal = false;
-      },
-      openDropdown() {
-        this.dropdownOpen = !this.dropdownOpen;
-      },
+  },
+  methods: {
+    ...mapActions(['createTempEntry']),
+    createNewItem(type) {
+      this.modalType = type;
+      this.openModal = true;
+      this.dropdownOpen = false;
     },
-  };
+    hideModal() {
+      this.openModal = false;
+    },
+    openDropdown() {
+      this.dropdownOpen = !this.dropdownOpen;
+    },
+  },
+};
 </script>
 
 <template>
@@ -71,7 +76,10 @@
           css-classes="pull-left"
         />
       </button>
-      <ul class="dropdown-menu dropdown-menu-right">
+      <ul
+        class="dropdown-menu dropdown-menu-right"
+        ref="dropdownMenu"
+      >
         <li>
           <a
             href="#"

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

@@ -40,13 +40,6 @@ export default {
 
       return __('Create file');
     },
-    formLabelName() {
-      if (this.type === 'tree') {
-        return __('Directory name');
-      }
-
-      return __('File name');
-    },
   },
   mounted() {
     this.$refs.fieldName.focus();
@@ -82,8 +75,8 @@ export default {
       @submit.prevent="createEntryInStore"
     >
       <fieldset class="form-group append-bottom-0">
-        <label class="label-light col-sm-3">
-          {{ formLabelName }}
+        <label class="label-light col-sm-3 ide-new-modal-label">
+          {{ __('Name') }}
         </label>
         <div class="col-sm-9">
           <input

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

@@ -97,7 +97,7 @@ export default {
             :file="file"
           />
         </span>
-        <span class="pull-right">
+        <span class="pull-right ide-file-icon-holder">
           <mr-file-icon
             v-if="file.mrChange"
           />
@@ -106,7 +106,8 @@ export default {
             :file="file"
             :show-tooltip="true"
             :show-staged-icon="true"
-            class="prepend-top-5 pull-right"
+            :force-modified-icon="true"
+            class="pull-right"
           />
         </span>
         <new-dropdown

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

@@ -84,6 +84,7 @@ export default {
       <changed-file-icon
         v-else
         :file="tab"
+        :force-modified-icon="true"
       />
     </button>
 

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

@@ -33,10 +33,7 @@ export const setPanelCollapsedStatus = ({ commit }, { side, collapsed }) => {
   }
 };
 
-export const toggleRightPanelCollapsed = (
-  { dispatch, state },
-  e = undefined,
-) => {
+export const toggleRightPanelCollapsed = ({ dispatch, state }, e = undefined) => {
   if (e) {
     $(e.currentTarget)
       .tooltip('hide')
@@ -77,7 +74,7 @@ export const createTempEntry = (
     }
 
     worker.addEventListener('message', ({ data }) => {
-      const { file } = data;
+      const { file, parentPath } = data;
 
       worker.terminate();
 
@@ -93,6 +90,10 @@ export const createTempEntry = (
         dispatch('setFileActive', file.path);
       }
 
+      if (parentPath && !state.entries[parentPath].opened) {
+        commit(types.TOGGLE_TREE_OPEN, parentPath);
+      }
+
       resolve(file);
     });
 
@@ -137,6 +138,14 @@ export const updateDelayViewerUpdated = ({ commit }, delay) => {
   commit(types.UPDATE_DELAY_VIEWER_CHANGE, delay);
 };
 
+export const updateTempFlagForEntry = ({ commit, dispatch, state }, { file, tempFile }) => {
+  commit(types.UPDATE_TEMP_FLAG, { path: file.path, tempFile });
+
+  if (file.parentPath) {
+    dispatch('updateTempFlagForEntry', { file: state.entries[file.parentPath], tempFile });
+  }
+};
+
 export const toggleFileFinder = ({ commit }, fileFindVisible) =>
   commit(types.TOGGLE_FILE_FINDER, fileFindVisible);
 

app/assets/javascripts/ide/stores/modules/commit/actions.js

@@ -110,6 +110,17 @@ export const updateFilesAfterCommit = (
       { root: true },
     );
 
+    commit(
+      rootTypes.TOGGLE_FILE_CHANGED,
+      {
+        file,
+        changed: false,
+      },
+      { root: true },
+    );
+
+    dispatch('updateTempFlagForEntry', { file, tempFile: false }, { root: true });
+
     eventHub.$emit(`editor.update.model.content.${file.key}`, {
       content: file.content,
       changed: !!changedFile,

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

@@ -59,4 +59,5 @@ export const UPDATE_FILE_AFTER_COMMIT = 'UPDATE_FILE_AFTER_COMMIT';
 export const ADD_PENDING_TAB = 'ADD_PENDING_TAB';
 export const REMOVE_PENDING_TAB = 'REMOVE_PENDING_TAB';
 
+export const UPDATE_TEMP_FLAG = 'UPDATE_TEMP_FLAG';
 export const TOGGLE_FILE_FINDER = 'TOGGLE_FILE_FINDER';

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

@@ -4,6 +4,7 @@ import mergeRequestMutation from './mutations/merge_request';
 import fileMutations from './mutations/file';
 import treeMutations from './mutations/tree';
 import branchMutations from './mutations/branch';
+import { sortTree } from './utils';
 
 export default {
   [types.SET_INITIAL_DATA](state, data) {
@@ -73,7 +74,7 @@ export default {
           f => foundEntry.tree.find(e => e.path === f.path) === undefined,
         );
         Object.assign(foundEntry, {
-          tree: foundEntry.tree.concat(tree),
+          tree: sortTree(foundEntry.tree.concat(tree)),
         });
       }
 
@@ -86,10 +87,16 @@ export default {
 
     if (!foundEntry) {
       Object.assign(state.trees[`${projectId}/${branchId}`], {
-        tree: state.trees[`${projectId}/${branchId}`].tree.concat(data.treeList),
+        tree: sortTree(state.trees[`${projectId}/${branchId}`].tree.concat(data.treeList)),
       });
     }
   },
+  [types.UPDATE_TEMP_FLAG](state, { path, tempFile }) {
+    Object.assign(state.entries[path], {
+      tempFile,
+      changed: tempFile,
+    });
+  },
   [types.UPDATE_VIEWER](state, viewer) {
     Object.assign(state, {
       viewer,

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

@@ -33,6 +33,7 @@ export const dataStructure = () => ({
   raw: '',
   content: '',
   parentTreeUrl: '',
+  parentPath: '',
   renderError: false,
   base64: false,
   editorRow: 1,
@@ -65,6 +66,7 @@ export const decorateData = entity => {
     previewMode,
     file_lock,
     html,
+    parentPath = '',
   } = entity;
 
   return {
@@ -81,6 +83,7 @@ export const decorateData = entity => {
     opened,
     active,
     parentTreeUrl,
+    parentPath,
     changed,
     renderError,
     content,
@@ -121,8 +124,8 @@ const sortTreesByTypeAndName = (a, b) => {
   } else if (a.type === 'blob' && b.type === 'tree') {
     return 1;
   }
-  if (a.name.toLowerCase() < b.name.toLowerCase()) return -1;
-  if (a.name.toLowerCase() > b.name.toLowerCase()) return 1;
+  if (a.name < b.name) return -1;
+  if (a.name > b.name) return 1;
   return 0;
 };
 

app/assets/javascripts/ide/stores/workers/files_decorator_worker.js

@@ -6,6 +6,7 @@ self.addEventListener('message', e => {
 
   const treeList = [];
   let file;
+  let parentPath;
   const entries = data.reduce((acc, path) => {
     const pathSplit = path.split('/');
     const blobName = pathSplit.pop().trim();
@@ -17,6 +18,8 @@ self.addEventListener('message', e => {
         const foundEntry = acc[folderPath];
 
         if (!foundEntry) {
+          parentPath = parentFolder ? parentFolder.path : null;
+
           const tree = decorateData({
             projectId,
             branchId,
@@ -29,6 +32,7 @@ self.addEventListener('message', e => {
             tempFile,
             changed: tempFile,
             opened: tempFile,
+            parentPath,
           });
 
           Object.assign(acc, {
@@ -52,6 +56,8 @@ self.addEventListener('message', e => {
 
     if (blobName !== '') {
       const fileFolder = acc[pathSplit.join('/')];
+      parentPath = fileFolder ? fileFolder.path : null;
+
       file = decorateData({
         projectId,
         branchId,
@@ -66,6 +72,7 @@ self.addEventListener('message', e => {
         content,
         base64,
         previewMode: viewerInformationForPath(blobName),
+        parentPath,
       });
 
       Object.assign(acc, {
@@ -86,5 +93,6 @@ self.addEventListener('message', e => {
     entries,
     treeList: sortTree(treeList),
     file,
+    parentPath,
   });
 });

app/assets/javascripts/vue_shared/components/file_icon.vue

 <script>
-  import getIconForFile from './file_icon/file_icon_map';
-  import loadingIcon from '../../vue_shared/components/loading_icon.vue';
-  import icon from '../../vue_shared/components/icon.vue';
+import getIconForFile from './file_icon/file_icon_map';
+import loadingIcon from '../../vue_shared/components/loading_icon.vue';
+import icon from '../../vue_shared/components/icon.vue';
 
-  /* This is a re-usable vue component for rendering a svg sprite
+/* This is a re-usable vue component for rendering a svg sprite
     icon
 
     Sample configuration:
@@ -15,60 +15,60 @@
     />
 
   */
-  export default {
-    components: {
-      loadingIcon,
-      icon,
+export default {
+  components: {
+    loadingIcon,
+    icon,
+  },
+  props: {
+    fileName: {
+      type: String,
+      required: true,
     },
-    props: {
-      fileName: {
-        type: String,
-        required: true,
-      },
 
-      folder: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
+    folder: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
 
-      opened: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
+    opened: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
 
-      loading: {
-        type: Boolean,
-        required: false,
-        default: false,
-      },
+    loading: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
 
-      size: {
-        type: Number,
-        required: false,
-        default: 16,
-      },
+    size: {
+      type: Number,
+      required: false,
+      default: 16,
+    },
 
-      cssClasses: {
-        type: String,
-        required: false,
-        default: '',
-      },
+    cssClasses: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  computed: {
+    spriteHref() {
+      const iconName = getIconForFile(this.fileName) || 'file';
+      return `${gon.sprite_file_icons}#${iconName}`;
+    },
+    folderIconName() {
+      return this.opened ? 'folder-open' : 'folder';
     },
-    computed: {
-      spriteHref() {
-        const iconName = getIconForFile(this.fileName) || 'file';
-        return `${gon.sprite_file_icons}#${iconName}`;
-      },
-      folderIconName() {
-        return this.opened ? 'folder-open' : 'folder';
-      },
-      iconSizeClass() {
-        return this.size ? `s${this.size}` : '';
-      },
+    iconSizeClass() {
+      return this.size ? `s${this.size}` : '';
     },
-  };
+  },
+};
 </script>
 <template>
   <span>
@@ -82,6 +82,7 @@
       v-if="!loading && folder"
       :name="folderIconName"
       :size="size"
+      css-classes="folder-icon"
     />
     <loading-icon
       v-if="loading"

app/assets/stylesheets/pages/repo.scss

@@ -55,6 +55,7 @@
       white-space: nowrap;
       text-overflow: ellipsis;
       max-width: inherit;
+      line-height: 22px;
 
       svg {
         vertical-align: middle;
@@ -67,6 +68,11 @@
       }
     }
 
+    .ide-file-icon-holder {
+      display: flex;
+      align-items: center;
+    }
+
     .ide-file-changed-icon {
       margin-left: auto;
 
@@ -77,7 +83,6 @@
 
     .ide-new-btn {
       display: none;
-      margin-bottom: -4px;
       margin-right: -8px;
     }
 
@@ -90,10 +95,8 @@
       }
     }
 
-    &.folder {
-      svg {
-        fill: $gl-text-color-secondary;
-      }
+    .folder-icon {
+      fill: $gl-text-color-secondary;
     }
   }
 
@@ -111,6 +114,7 @@
 .file-col-commit-message {
   display: flex;
   overflow: visible;
+  align-items: center;
   padding: 6px 12px;
 }
 
@@ -438,7 +442,7 @@
   .projects-sidebar {
     display: flex;
     flex-direction: column;
-    height: 100%;
+    flex: 1;
 
     .context-header {
       width: auto;
@@ -967,3 +971,7 @@
   background: transparent;
   resize: none;
 }
+
+.ide-new-modal-label {
+  line-height: 34px;
+}

spec/javascripts/ide/components/new_dropdown/index_spec.js

@@ -32,12 +32,8 @@ describe('new dropdown component', () => {
 
   it('renders new file, upload and new directory links', () => {
     expect(vm.$el.querySelectorAll('a')[0].textContent.trim()).toBe('New file');
-    expect(vm.$el.querySelectorAll('a')[1].textContent.trim()).toBe(
-      'Upload file',
-    );
-    expect(vm.$el.querySelectorAll('a')[2].textContent.trim()).toBe(
-      'New directory',
-    );
+    expect(vm.$el.querySelectorAll('a')[1].textContent.trim()).toBe('Upload file');
+    expect(vm.$el.querySelectorAll('a')[2].textContent.trim()).toBe('New directory');
   });
 
   describe('createNewItem', () => {
@@ -81,4 +77,18 @@ describe('new dropdown component', () => {
         .catch(done.fail);
     });
   });
+
+  describe('dropdownOpen', () => {
+    it('scrolls dropdown into view', done => {
+      spyOn(vm.$refs.dropdownMenu, 'scrollIntoView');
+
+      vm.dropdownOpen = true;
+
+      setTimeout(() => {
+        expect(vm.$refs.dropdownMenu.scrollIntoView).toHaveBeenCalled();
+
+        done();
+      });
+    });
+  });
 });

spec/javascripts/ide/components/new_dropdown/modal_spec.js

@@ -25,25 +25,17 @@ describe('new file modal component', () => {
       it(`sets modal title as ${type}`, () => {
         const title = type === 'tree' ? 'directory' : 'file';
 
-        expect(vm.$el.querySelector('.modal-title').textContent.trim()).toBe(
-          `Create new ${title}`,
-        );
+        expect(vm.$el.querySelector('.modal-title').textContent.trim()).toBe(`Create new ${title}`);
       });
 
       it(`sets button label as ${type}`, () => {
         const title = type === 'tree' ? 'directory' : 'file';
 
-        expect(vm.$el.querySelector('.btn-success').textContent.trim()).toBe(
-          `Create ${title}`,
-        );
+        expect(vm.$el.querySelector('.btn-success').textContent.trim()).toBe(`Create ${title}`);
       });
 
       it(`sets form label as ${type}`, () => {
-        const title = type === 'tree' ? 'Directory' : 'File';
-
-        expect(vm.$el.querySelector('.label-light').textContent.trim()).toBe(
-          `${title} name`,
-        );
+        expect(vm.$el.querySelector('.label-light').textContent.trim()).toBe('Name');
       });
 
       describe('createEntryInStore', () => {

spec/javascripts/ide/stores/actions_spec.js

-import actions, { stageAllChanges, unstageAllChanges, toggleFileFinder } from '~/ide/stores/actions';
+import actions, {
+  stageAllChanges,
+  unstageAllChanges,
+  toggleFileFinder,
+  updateTempFlagForEntry,
+} from '~/ide/stores/actions';
 import store from '~/ide/stores';
 import * as types from '~/ide/stores/mutation_types';
 import router from '~/ide/ide_router';
@@ -340,6 +345,49 @@ describe('Multi-file store actions', () => {
     });
   });
 
+  describe('updateTempFlagForEntry', () => {
+    it('commits UPDATE_TEMP_FLAG', done => {
+      const f = {
+        ...file(),
+        path: 'test',
+        tempFile: true,
+      };
+      store.state.entries[f.path] = f;
+
+      testAction(
+        updateTempFlagForEntry,
+        { file: f, tempFile: false },
+        store.state,
+        [{ type: 'UPDATE_TEMP_FLAG', payload: { path: f.path, tempFile: false } }],
+        [],
+        done,
+      );
+    });
+
+    it('commits UPDATE_TEMP_FLAG and dispatches for parent', done => {
+      const parent = {
+        ...file(),
+        path: 'testing',
+      };
+      const f = {
+        ...file(),
+        path: 'test',
+        parentPath: 'testing',
+      };
+      store.state.entries[parent.path] = parent;
+      store.state.entries[f.path] = f;
+
+      testAction(
+        updateTempFlagForEntry,
+        { file: f, tempFile: false },
+        store.state,
+        [{ type: 'UPDATE_TEMP_FLAG', payload: { path: f.path, tempFile: false } }],
+        [{ type: 'updateTempFlagForEntry', payload: { file: parent, tempFile: false } }],
+        done,
+      );
+    });
+  });
+
   describe('toggleFileFinder', () => {
     it('commits TOGGLE_FILE_FINDER', done => {
       testAction(

spec/javascripts/ide/stores/mutations_spec.js

@@ -87,6 +87,28 @@ describe('Multi-file store mutations', () => {
     });
   });
 
+  describe('UPDATE_TEMP_FLAG', () => {
+    beforeEach(() => {
+      localState.entries.test = {
+        ...file(),
+        tempFile: true,
+        changed: true,
+      };
+    });
+
+    it('updates tempFile flag', () => {
+      mutations.UPDATE_TEMP_FLAG(localState, { path: 'test', tempFile: false });
+
+      expect(localState.entries.test.tempFile).toBe(false);
+    });
+
+    it('updates changed flag', () => {
+      mutations.UPDATE_TEMP_FLAG(localState, { path: 'test', tempFile: false });
+
+      expect(localState.entries.test.changed).toBe(false);
+    });
+  });
+
   describe('TOGGLE_FILE_FINDER', () => {
     it('updates fileFindVisible', () => {
       mutations.TOGGLE_FILE_FINDER(localState, true);