Merge branch 'tz-fix-ide-bugs' into 'master'

WebIDE: Fix Commit bugs

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

app/assets/javascripts/api.js

@@ -218,6 +218,7 @@ const Api = {
         (jqXHR, textStatus, errorThrown) => {
           const error = new Error(`${options.url}: ${errorThrown}`);
           error.textStatus = textStatus;
+          if (jqXHR && jqXHR.responseJSON) error.responseJSON = jqXHR.responseJSON;
           reject(error);
         },
       );

app/assets/javascripts/flash.js

@@ -10,6 +10,7 @@ const hideFlash = (flashEl, fadeTransition = true) => {
 
   flashEl.addEventListener('transitionend', () => {
     flashEl.remove();
+    if (document.body.classList.contains('flash-shown')) document.body.classList.remove('flash-shown');
   }, {
     once: true,
     passive: true,
@@ -64,6 +65,7 @@ const createFlash = function createFlash(
   parent = document,
   actionConfig = null,
   fadeTransition = true,
+  addBodyClass = false,
 ) {
   const flashContainer = parent.querySelector('.flash-container');
 
@@ -86,6 +88,8 @@ const createFlash = function createFlash(
 
   flashContainer.style.display = 'block';
 
+  if (addBodyClass) document.body.classList.add('flash-shown');
+
   return flashContainer;
 };
 

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

@@ -68,12 +68,8 @@ export default {
       this.commitChanges({ payload, newMr: this.startNewMR })
         .then(() => {
           this.submitCommitsLoading = false;
-          this.$store.dispatch('getTreeData', {
-            projectId: this.currentProjectId,
-            branch: this.currentBranchId,
-            endpoint: `/tree/${this.currentBranchId}`,
-            force: true,
-          });
+          this.commitMessage = '';
+          this.startNewMR = false;
         })
         .catch(() => {
           this.submitCommitsLoading = false;
@@ -153,6 +149,7 @@ you started editing. Would you like to create a new branch?`)"
           type="submit"
           :disabled="commitButtonDisabled"
           class="btn btn-default btn-sm append-right-10 prepend-left-10"
+          :class="{ disabled: submitCommitsLoading }"
         >
           <i
             v-if="submitCommitsLoading"

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

@@ -70,7 +70,10 @@ export default {
           this.editor.createInstance(this.$refs.editor);
         })
         .then(() => this.setupEditor())
-        .catch(() => flash('Error setting up monaco. Please try again.'));
+        .catch((err) => {
+          flash('Error setting up monaco. Please try again.', 'alert', document, null, false, true);
+          throw err;
+        });
     },
     setupEditor() {
       if (!this.activeFile) return;

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

@@ -35,9 +35,12 @@
         return this.file.type === 'tree';
       },
       levelIndentation() {
+        if (this.file.level > 0) {
           return {
             marginLeft: `${this.file.level * 16}px`,
           };
+        }
+        return {};
       },
       shortId() {
         return this.file.id.substr(0, 8);
@@ -111,7 +114,7 @@
       />
       <i
         class="fa"
-        v-if="changedClass"
+        v-if="file.changed || file.tempFile"
         :class="changedClass"
         aria-hidden="true"
       >

app/assets/javascripts/ide/ide_router.js

@@ -84,13 +84,13 @@ router.beforeEach((to, from, next) => {
           }
         })
         .catch((e) => {
-          flash('Error while loading the branch files. Please try again.');
+          flash('Error while loading the branch files. Please try again.', 'alert', document, null, false, true);
           throw e;
         });
       }
     })
     .catch((e) => {
-      flash('Error while loading the project data. Please try again.');
+      flash('Error while loading the project data. Please try again.', 'alert', document, null, false, true);
       throw e;
     });
   }

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

@@ -55,7 +55,7 @@ export default class Editor {
 
   attachModel(model) {
     this.instance.setModel(model.getModel());
-    this.dirtyDiffController.attachModel(model);
+    if (this.dirtyDiffController) this.dirtyDiffController.attachModel(model);
 
     this.currentModel = model;
 
@@ -68,7 +68,7 @@ export default class Editor {
       return acc;
     }, {}));
 
-    this.dirtyDiffController.reDecorate(model);
+    if (this.dirtyDiffController) this.dirtyDiffController.reDecorate(model);
   }
 
   clearEditor() {

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

@@ -3,6 +3,7 @@ import { visitUrl } from '../../lib/utils/url_utility';
 import flash from '../../flash';
 import service from '../services';
 import * as types from './mutation_types';
+import { stripHtml } from '../../lib/utils/text_utility';
 
 export const redirectToUrl = (_, url) => visitUrl(url);
 
@@ -81,7 +82,7 @@ export const checkCommitStatus = ({ state }) =>
 
       return false;
     })
-    .catch(() => flash('Error checking branch data. Please try again.'));
+    .catch(() => flash('Error checking branch data. Please try again.', 'alert', document, null, false, true));
 
 export const commitChanges = (
   { commit, state, dispatch, getters },
@@ -92,7 +93,7 @@ export const commitChanges = (
     .then((data) => {
       const { branch } = payload;
       if (!data.short_id) {
-        flash(data.message);
+        flash(data.message, 'alert', document, null, false, true);
         return;
       }
 
@@ -105,19 +106,25 @@ export const commitChanges = (
         },
       };
 
+      let commitMsg = `Your changes have been committed. Commit ${data.short_id}`;
+      if (data.stats) {
+        commitMsg += ` with ${data.stats.additions} additions, ${data.stats.deletions} deletions.`;
+      }
+
       flash(
-        `Your changes have been committed. Commit ${data.short_id} with ${
-          data.stats.additions
-        } additions, ${data.stats.deletions} deletions.`,
+        commitMsg,
         'notice',
-      );
+        document,
+        null,
+        false,
+        true);
+      window.dispatchEvent(new Event('resize'));
 
       if (newMr) {
+        dispatch('discardAllChanges');
         dispatch(
           'redirectToUrl',
-          `${
-            selectedProject.web_url
-          }/merge_requests/new?merge_request%5Bsource_branch%5D=${branch}`,
+          `${selectedProject.web_url}/merge_requests/new?merge_request%5Bsource_branch%5D=${branch}`,
         );
       } else {
         commit(types.SET_BRANCH_WORKING_REFERENCE, {
@@ -134,12 +141,18 @@ export const commitChanges = (
         });
 
         dispatch('discardAllChanges');
-        dispatch('closeAllFiles');
 
         window.scrollTo(0, 0);
       }
     })
-    .catch(() => flash('Error committing changes. Please try again.'));
+    .catch((err) => {
+      let errMsg = 'Error committing changes. Please try again.';
+      if (err.responseJSON && err.responseJSON.message) {
+        errMsg += ` (${stripHtml(err.responseJSON.message)})`;
+      }
+      flash(errMsg, 'alert', document, null, false, true);
+      window.dispatchEvent(new Event('resize'));
+    });
 
 export const createTempEntry = (
   { state, dispatch },

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

@@ -17,7 +17,7 @@ export const getBranchData = (
         resolve(data);
       })
       .catch(() => {
-        flash('Error loading branch data. Please try again.');
+        flash('Error loading branch data. Please try again.', 'alert', document, null, false, true);
         reject(new Error(`Branch not loaded - ${projectId}/${branchId}`));
       });
   } else {

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

@@ -69,7 +69,7 @@ export const getFileData = ({ state, commit, dispatch }, file) => {
     })
     .catch(() => {
       commit(types.TOGGLE_LOADING, file);
-      flash('Error loading file data. Please try again.');
+      flash('Error loading file data. Please try again.', 'alert', document, null, false, true);
     });
 };
 
@@ -77,22 +77,28 @@ export const getRawFileData = ({ commit, dispatch }, file) => service.getRawFile
   .then((raw) => {
     commit(types.SET_FILE_RAW_DATA, { file, raw });
   })
-  .catch(() => flash('Error loading file content. Please try again.'));
+  .catch(() => flash('Error loading file content. Please try again.', 'alert', document, null, false, true));
 
 export const changeFileContent = ({ commit }, { file, content }) => {
   commit(types.UPDATE_FILE_CONTENT, { file, content });
 };
 
 export const setFileLanguage = ({ state, commit }, { fileLanguage }) => {
+  if (state.selectedFile) {
     commit(types.SET_FILE_LANGUAGE, { file: state.selectedFile, fileLanguage });
+  }
 };
 
 export const setFileEOL = ({ state, commit }, { eol }) => {
+  if (state.selectedFile) {
     commit(types.SET_FILE_EOL, { file: state.selectedFile, eol });
+  }
 };
 
 export const setEditorPosition = ({ state, commit }, { editorRow, editorColumn }) => {
+  if (state.selectedFile) {
     commit(types.SET_FILE_POSITION, { file: state.selectedFile, editorRow, editorColumn });
+  }
 };
 
 export const createTempFile = ({ state, commit, dispatch }, { projectId, branchId, parent, name, content = '', base64 = '' }) => {
@@ -112,7 +118,7 @@ export const createTempFile = ({ state, commit, dispatch }, { projectId, branchI
     url: newUrl,
   });
 
-  if (findEntry(parent.tree, 'blob', file.name)) return flash(`The name "${file.name}" is already taken in this directory.`);
+  if (findEntry(parent.tree, 'blob', file.name)) return flash(`The name "${file.name}" is already taken in this directory.`, 'alert', document, null, false, true);
 
   commit(types.CREATE_TMP_FILE, {
     parent,

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

@@ -18,7 +18,7 @@ export const getProjectData = (
       resolve(data);
     })
     .catch(() => {
-      flash('Error loading project data. Please try again.');
+      flash('Error loading project data. Please try again.', 'alert', document, null, false, true);
       reject(new Error(`Project not loaded ${namespace}/${projectId}`));
     });
   } else {

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

@@ -52,7 +52,7 @@ export const getTreeData = (
         resolve(data);
       })
       .catch((e) => {
-        flash('Error loading tree data. Please try again.');
+        flash('Error loading tree data. Please try again.', 'alert', document, null, false, true);
         if (tree) commit(types.TOGGLE_LOADING, tree);
         reject(e);
       });
@@ -151,7 +151,7 @@ export const getLastCommitData = ({ state, commit, dispatch, getters }, tree = s
 
       dispatch('getLastCommitData', tree);
     })
-    .catch(() => flash('Error fetching log data.'));
+    .catch(() => flash('Error fetching log data.', 'alert', document, null, false, true));
 };
 
 export const updateDirectoryData = (

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

@@ -64,7 +64,7 @@ export default {
   },
   [types.DISCARD_FILE_CHANGES](state, file) {
     Object.assign(file, {
-      content: '',
+      content: file.raw,
       changed: false,
     });
   },

app/assets/javascripts/lib/utils/text_utility.js

@@ -72,4 +72,4 @@ export function capitalizeFirstCharacter(text) {
  * @param {*} replace
  * @returns {String}
  */
-export const stripeHtml = (string, replace = '') => string.replace(/<[^>]*>/g, replace);
+export const stripHtml = (string, replace = '') => string.replace(/<[^>]*>/g, replace);

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

@@ -33,7 +33,7 @@
 <template>
   <component
     :is="rootElementType"
-    class="text-center">
+    class="loading-container text-center">
     <i
       class="fa fa-spin fa-spinner"
       :class="cssClass"

app/assets/stylesheets/framework/variables.scss

@@ -258,6 +258,8 @@ $general-hover-transition-duration: 100ms;
 $general-hover-transition-curve: linear;
 $highlight-changes-color: rgb(235, 255, 232);
 $performance-bar-height: 35px;
+$flash-height: 52px;
+$context-header-height: 60px;
 
 /*
 * Common component specific colors

app/assets/stylesheets/pages/repo.scss

@@ -107,6 +107,11 @@ table.table tr td.multi-file-table-name {
     vertical-align: middle;
     margin-right: 2px;
   }
+
+  .loading-container {
+    margin-right: 4px;
+    display: inline-block;
+  }
 }
 
 .multi-file-table-col-commit-message {
@@ -247,7 +252,6 @@ table.table tr td.multi-file-table-name {
   display: flex;
   position: relative;
   flex-direction: column;
-  height: 100%;
   width: 290px;
   padding: 0;
   background-color: $gray-light;
@@ -256,6 +260,11 @@ table.table tr td.multi-file-table-name {
   .projects-sidebar {
     display: flex;
     flex-direction: column;
+
+    .context-header {
+      width: auto;
+      margin-right: 0;
+    }
   }
 
   .multi-file-commit-panel-inner {
@@ -496,19 +505,70 @@ table.table tr td.multi-file-table-name {
   }
 }
 
-.ide-flash-container.flash-container {
+.ide.nav-only {
+  .flash-container {
     margin-top: $header-height;
     margin-bottom: 0;
+  }
+
+  .alert-wrapper .flash-container .flash-alert:last-child,
+  .alert-wrapper .flash-container .flash-notice:last-child {
+    margin-bottom: 0;
+  }
+
+  .content {
+    margin-top: $header-height;
+  }
+
+  .multi-file-commit-panel .multi-file-commit-panel-inner-scroll {
+    max-height: calc(100vh - #{$header-height + $context-header-height});
+  }
+
+  &.flash-shown {
+    .content {
+      margin-top: 0;
+    }
+
+    .ide-view {
+      height: calc(100vh - #{$header-height + $flash-height});
+    }
+
+    .multi-file-commit-panel .multi-file-commit-panel-inner-scroll {
+      max-height: calc(100vh - #{$header-height + $flash-height + $context-header-height});
+    }
+  }
 }
 
-.with-performance-bar {
-  .ide-flash-container.flash-container {
-    margin-top: $header-height + $performance-bar-height;
+.with-performance-bar .ide.nav-only {
+  .flash-container {
+    margin-top: #{$header-height + $performance-bar-height};
+  }
+
+  .content {
+    margin-top: #{$header-height + $performance-bar-height};
   }
 
   .ide-view {
     height: calc(100vh - #{$header-height + $performance-bar-height});
   }
+
+  .multi-file-commit-panel .multi-file-commit-panel-inner-scroll {
+    max-height: calc(100vh - #{$header-height + $performance-bar-height + 60});
+  }
+
+  &.flash-shown {
+    .content {
+      margin-top: 0;
+    }
+
+    .ide-view {
+      height: calc(100vh - #{$header-height + $performance-bar-height + $flash-height});
+    }
+
+    .multi-file-commit-panel .multi-file-commit-panel-inner-scroll {
+      max-height: calc(100vh - #{$header-height + $performance-bar-height + $flash-height + $context-header-height});
+    }
+  }
 }
 
 

app/helpers/webpack_helper.rb

 require 'webpack/rails/manifest'
 
 module WebpackHelper
-  def webpack_bundle_tag(bundle)
-    javascript_include_tag(*gitlab_webpack_asset_paths(bundle))
+  def webpack_bundle_tag(bundle, force_same_domain: false)
+    javascript_include_tag(*gitlab_webpack_asset_paths(bundle, force_same_domain: true))
   end
 
   # override webpack-rails gem helper until changes can make it upstream
-  def gitlab_webpack_asset_paths(source, extension: nil)
+  def gitlab_webpack_asset_paths(source, extension: nil, force_same_domain: false)
     return "" unless source.present?
 
     paths = Webpack::Rails::Manifest.asset_paths(source)
@@ -14,10 +14,12 @@ module WebpackHelper
       paths.select! { |p| p.ends_with? ".#{extension}" }
     end
 
+    unless force_same_domain
       force_host = webpack_public_host
       if force_host
         paths.map! { |p| "#{force_host}#{p}" }
       end
+    end
 
     paths
   end

app/views/ide/index.html.haml

+- @body_class = 'ide'
 - page_title 'IDE'
 
 - content_for :page_specific_javascripts do
   = webpack_bundle_tag 'common_vue'
-  = webpack_bundle_tag 'ide'
-
-.ide-flash-container.flash-container
+  = webpack_bundle_tag 'ide', force_same_domain: true
 
 #ide.ide-loading{ data: {"empty-state-svg-path" => image_path('illustrations/multi_file_editor_empty.svg')} }
   .text-center

app/views/layouts/nav_only.html.haml

 !!! 5
 %html{ lang: I18n.locale, class: page_class }
   = render "layouts/head"
-  %body{ class: "#{user_application_theme} #{@body_class}", data: { page: body_data_page } }
+  %body{ class: "#{user_application_theme} #{@body_class} nav-only", data: { page: body_data_page } }
     = render 'peek/bar'
     = render "layouts/header/default"
     = render 'shared/outdated_browser'
@@ -10,4 +10,5 @@
       = render "layouts/broadcast"
       = yield :flash_message
       = render "layouts/flash"
+    .content{ id: "content-body" }
       = yield

config/webpack.config.js

@@ -119,7 +119,12 @@ var config = {
       {
         test: /\_worker\.js$/,
         use: [
-          { loader: 'worker-loader' },
+          { 
+            loader: 'worker-loader',
+            options: { 
+              inline: true
+            }
+          },
           { loader: 'babel-loader' },
         ],
       },

spec/javascripts/flash_spec.js

@@ -183,11 +183,15 @@ describe('Flash', () => {
       });
 
       it('adds flash element into container', () => {
-        flash('test');
+        flash('test', 'alert', document, null, false, true);
 
         expect(
           document.querySelector('.flash-alert'),
         ).not.toBeNull();
+
+        expect(
+          document.body.className,
+        ).toContain('flash-shown');
       });
 
       it('adds flash into specified parent', () => {
@@ -220,13 +224,17 @@ describe('Flash', () => {
       });
 
       it('removes element after clicking', () => {
-        flash('test', 'alert', document, null, false);
+        flash('test', 'alert', document, null, false, true);
 
         document.querySelector('.flash-alert').click();
 
         expect(
           document.querySelector('.flash-alert'),
         ).toBeNull();
+
+        expect(
+          document.body.className,
+        ).not.toContain('flash-shown');
       });
 
       describe('with actionConfig', () => {

spec/javascripts/lib/utils/text_utility_spec.js

@@ -63,13 +63,13 @@ describe('text_utility', () => {
     });
   });
 
-  describe('stripeHtml', () => {
+  describe('stripHtml', () => {
     it('replaces html tag with the default replacement', () => {
-      expect(textUtils.stripeHtml('This is a text with <p>html</p>.')).toEqual('This is a text with html.');
+      expect(textUtils.stripHtml('This is a text with <p>html</p>.')).toEqual('This is a text with html.');
     });
 
     it('replaces html tags with the provided replacement', () => {
-      expect(textUtils.stripeHtml('This is a text with <p>html</p>.', ' ')).toEqual('This is a text with  html .');
+      expect(textUtils.stripHtml('This is a text with <p>html</p>.', ' ')).toEqual('This is a text with  html .');
     });
   });
 });

spec/javascripts/repo/stores/actions_spec.js

@@ -300,19 +300,6 @@ describe('Multi-file store actions', () => {
           }).catch(done.fail);
       });
 
-      it('closes all files', (done) => {
-        store.state.openFiles.push(file());
-        store.state.openFiles[0].opened = true;
-
-        store.dispatch('commitChanges', { payload, newMr: false })
-          .then(Vue.nextTick)
-          .then(() => {
-            expect(store.state.openFiles.length).toBe(0);
-
-            done();
-          }).catch(done.fail);
-      });
-
       it('scrolls to top of page', (done) => {
         store.dispatch('commitChanges', { payload, newMr: false })
           .then(() => {

spec/javascripts/vue_shared/components/loading_icon_spec.js

@@ -16,7 +16,8 @@ describe('Loading Icon Component', () => {
     ).toEqual('fa fa-spin fa-spinner fa-1x');
 
     expect(component.$el.tagName).toEqual('DIV');
-    expect(component.$el.classList.contains('text-center')).toEqual(true);
+    expect(component.$el.classList).toContain('text-center');
+    expect(component.$el.classList).toContain('loading-container');
   });
 
   it('should render accessibility attributes', () => {