Fix usability problems for file template

This commmit is the start for issue 30807 and
resolving usability problems with file templates.
I've moved the exisiting UI and changed some functionality.

Use toast component to undo template actions

Set active class to template type

Fix failing rspec tests

Fixed failing rspec tests due to refactor
of the UI. New functionality was introduced
so the tests needed to be reworked.

Fix failing test and add mobile styles

Generate changelog for merge request

Fix eslint errors for i18n

Fix failed job due to locale/gitlab.pot

Fix linting error in haml file

Change dropdown labels and fix dropdown headers

Adjust rspec for template selectors

Change toggle text after undo template action

Fix broken toast due to udpated master

Remove toggle label on template selector

Fix failing rspec tests due to toggle text change

Remove un-need conditional check

Add template selection to toast

Adjust rspec to pass for undo action

app/assets/javascripts/blob/file_template_mediator.js

@@ -7,6 +7,8 @@ import BlobCiYamlSelector from './template_selectors/ci_yaml_selector';
 import DockerfileSelector from './template_selectors/dockerfile_selector';
 import GitignoreSelector from './template_selectors/gitignore_selector';
 import LicenseSelector from './template_selectors/license_selector';
+import toast from '~/vue_shared/plugins/global_toast';
+import { __ } from '~/locale';
 
 export default class FileTemplateMediator {
   constructor({ editor, currentAction, projectId }) {
@@ -19,6 +21,7 @@ export default class FileTemplateMediator {
     this.initDomElements();
     this.initDropdowns();
     this.initPageEvents();
+    this.cacheFileContents();
   }
 
   initTemplateSelectors() {
@@ -40,6 +43,7 @@ export default class FileTemplateMediator {
         return {
           name: cfg.name,
           key: cfg.key,
+          id: cfg.key,
         };
       }),
     });
@@ -58,6 +62,7 @@ export default class FileTemplateMediator {
     this.$fileContent = $fileEditor.find('#file-content');
     this.$commitForm = $fileEditor.find('form');
     this.$navLinks = $fileEditor.find('.nav-links');
+    this.$templateTypes = this.$templateSelectors.find('.template-type-selector');
   }
 
   initDropdowns() {
@@ -113,7 +118,11 @@ export default class FileTemplateMediator {
       }
     });
 
-    this.typeSelector.setToggleText(item.name);
+    this.setFilename(item.name);
+
+    if (this.editor.getValue() !== '') {
+      this.setTypeSelectorToggleText(item.name);
+    }
 
     this.cacheToggleText();
   }
@@ -123,15 +132,24 @@ export default class FileTemplateMediator {
   }
 
   selectTemplateFile(selector, query, data) {
+    const self = this;
+
     selector.renderLoading();
-    // in case undo menu is already there
-    this.destroyUndoMenu();
+
     this.fetchFileTemplate(selector.config.type, query, data)
       .then(file => {
-        this.showUndoMenu();
         this.setEditorContent(file);
-        this.setFilename(selector.config.name);
         selector.renderLoaded();
+        this.typeSelector.setToggleText(selector.config.name);
+        toast(__(`${query} template applied`), {
+          action: {
+            text: __('Undo'),
+            onClick: (e, toastObj) => {
+              self.restoreFromCache();
+              toastObj.goAway(0);
+            },
+          },
+        });
       })
       .catch(err => new Flash(`An error occurred while fetching the template: ${err}`));
   }
@@ -173,22 +191,6 @@ export default class FileTemplateMediator {
     return this.templateSelectors.find(selector => selector.config.key === key);
   }
 
-  showUndoMenu() {
-    this.$undoMenu.removeClass('hidden');
-
-    this.$undoBtn.on('click', () => {
-      this.restoreFromCache();
-      this.destroyUndoMenu();
-    });
-  }
-
-  destroyUndoMenu() {
-    this.cacheFileContents();
-    this.cacheToggleText();
-    this.$undoMenu.addClass('hidden');
-    this.$undoBtn.off('click');
-  }
-
   hideTemplateSelectorMenu() {
     this.$templatesMenu.hide();
   }
@@ -210,6 +212,7 @@ export default class FileTemplateMediator {
     this.setEditorContent(this.cachedContent);
     this.setFilename(this.cachedFilename);
     this.setTemplateSelectorToggleText();
+    this.setTypeSelectorToggleText(__('Select a template type'));
   }
 
   getTemplateSelectorToggleText() {
@@ -228,6 +231,10 @@ export default class FileTemplateMediator {
     return this.typeSelector.getToggleText();
   }
 
+  setTypeSelectorToggleText(text) {
+    this.typeSelector.setToggleText(text);
+  }
+
   getFilename() {
     return this.$filenameInput.val();
   }

app/assets/javascripts/blob/template_selectors/ci_yaml_selector.js

@@ -19,7 +19,6 @@ export default class BlobCiYamlSelector extends FileTemplateSelector {
       data: this.$dropdown.data('data'),
       filterable: true,
       selectable: true,
-      toggleLabel: item => item.name,
       search: {
         fields: ['name'],
       },

app/assets/javascripts/blob/template_selectors/dockerfile_selector.js

@@ -20,7 +20,6 @@ export default class DockerfileSelector extends FileTemplateSelector {
       data: this.$dropdown.data('data'),
       filterable: true,
       selectable: true,
-      toggleLabel: item => item.name,
       search: {
         fields: ['name'],
       },

app/assets/javascripts/blob/template_selectors/gitignore_selector.js

@@ -18,7 +18,6 @@ export default class BlobGitignoreSelector extends FileTemplateSelector {
       data: this.$dropdown.data('data'),
       filterable: true,
       selectable: true,
-      toggleLabel: item => item.name,
       search: {
         fields: ['name'],
       },

app/assets/javascripts/blob/template_selectors/license_selector.js

@@ -18,7 +18,6 @@ export default class BlobLicenseSelector extends FileTemplateSelector {
       data: this.$dropdown.data('data'),
       filterable: true,
       selectable: true,
-      toggleLabel: item => item.name,
       search: {
         fields: ['name'],
       },

app/assets/javascripts/blob/template_selectors/type_selector.js

@@ -16,7 +16,6 @@ export default class FileTemplateTypeSelector extends FileTemplateSelector {
       data: this.config.dropdownData,
       filterable: false,
       selectable: true,
-      toggleLabel: item => item.name,
       clicked: options => this.mediator.selectTemplateTypeOptions(options),
       text: item => item.name,
     });

app/assets/stylesheets/framework/dropdowns.scss

@@ -326,8 +326,9 @@
   }
 
   .dropdown-header {
-    color: $gl-text-color-secondary;
+    color: $black;
     font-size: 13px;
+    font-weight: $gl-font-weight-bold;
     line-height: $gl-line-height;
     padding: $dropdown-item-padding-y $dropdown-item-padding-x;
   }

app/assets/stylesheets/pages/editor.scss

@@ -47,14 +47,19 @@
     margin-right: 10px;
   }
 
-  .new-file-name {
+  .new-file-name,
+  .new-file-path {
     display: inline-block;
-    max-width: 420px;
+    max-width: 250px;
     float: left;
 
     @media(max-width: map-get($grid-breakpoints, lg)-1) {
       width: 180px;
     }
+
+    @media (max-width: 1360px) {
+      width: auto;
+    }
   }
 
   .file-buttons {
@@ -98,13 +103,14 @@
 }
 
 
-@include media-breakpoint-down(sm) {
+@include media-breakpoint-down(md) {
   .file-editor {
     .file-title {
       display: block;
     }
 
-    .new-file-name {
+    .new-file-name,
+    .new-file-path {
       max-width: none;
       width: 100%;
       margin-bottom: 3px;
@@ -146,20 +152,17 @@
   vertical-align: top;
   display: inline-block;
 
-  @media(max-width: map-get($grid-breakpoints, md)-1) {
+  @media(max-width: map-get($grid-breakpoints, lg)-1) {
     display: block;
     margin: 19px 0 12px;
   }
 }
 
 .template-selectors-menu {
-  display: inline-block;
+  display: flex;
   vertical-align: top;
-  margin: 14px 0 0 16px;
-  padding: 0 0 0 14px;
-  border-left: 1px solid $border-color;
 
-  @media(max-width: map-get($grid-breakpoints, md)-1) {
+  @media(max-width: map-get($grid-breakpoints, lg)-1) {
     display: block;
     width: 100%;
     margin: 5px 0;
@@ -168,24 +171,11 @@
   }
 }
 
-.templates-selectors-label {
-  display: inline-block;
-  vertical-align: top;
-  margin-top: 6px;
-  line-height: 21px;
-
-  @media(max-width: map-get($grid-breakpoints, md)-1) {
-    display: block;
-    margin: 5px 0;
-  }
-}
-
 .template-selector-dropdowns-wrap {
   display: inline-block;
-  margin: 5px 0 0 8px;
   vertical-align: top;
 
-  @media(max-width: map-get($grid-breakpoints, md)-1) {
+  @media(max-width: map-get($grid-breakpoints, lg)-1) {
     display: block;
     width: 100%;
     margin: 0 0 16px;
@@ -199,9 +189,8 @@
     display: inline-block;
     vertical-align: top;
     font-family: $regular_font;
-    margin-top: -5px;
 
-    @media(max-width: map-get($grid-breakpoints, md)-1) {
+    @media(max-width: map-get($grid-breakpoints, lg)-1) {
       display: block;
       width: 100%;
       margin: 5px 0;
@@ -212,30 +201,22 @@
     }
 
     .dropdown-menu-toggle {
-      width: 250px;
+      width: 200px;
       vertical-align: top;
 
-      @media(max-width: map-get($grid-breakpoints, md)-1) {
+      @media (max-width: map-get($grid-breakpoints, xl)-1) {
+        width: auto;
+      }
+
+      @media(max-width: map-get($grid-breakpoints, lg)-1) {
         display: block;
         width: 100%;
         margin: 5px 0;
       }
     }
-
   }
 }
 
-.template-selectors-undo-menu {
-  display: inline-block;
-  margin: 7px 0 0 10px;
-
-  @media(max-width: map-get($grid-breakpoints, md)-1) {
-    display: block;
-    width: 100%;
-    margin: 20px 0;
-  }
-
-  button {
-    margin: -4px 0 0 15px;
-  }
+.editor-title-row {
+  margin-bottom: 20px;
 }

app/views/projects/blob/_editor.html.haml

@@ -3,20 +3,22 @@
 - is_markdown = Gitlab::MarkupHelper.gitlab_markdown?(file_name)
 
 .file-holder-bottom-radius.file-holder.file.append-bottom-default
-  .js-file-title.file-title.clearfix{ data: { current_action: action } }
+  .js-file-title.file-title.align-items-center.clearfix{ data: { current_action: action } }
     .editor-ref.block-truncated
       = sprite_icon('fork', size: 12)
       = ref
     - if current_action?(:edit) || current_action?(:update)
       %span.pull-left.append-right-10
-        = text_field_tag 'file_path', (params[:file_path] || @path),
-                                      class: 'form-control new-file-path js-file-path-name-input'
+      = text_field_tag 'file_path', (params[:file_path] || @path),
+        class: 'form-control new-file-path js-file-path-name-input'
+      = render 'template_selectors'
 
     - if current_action?(:new) || current_action?(:create)
       %span.pull-left.append-right-10
         \/
       = text_field_tag 'file_name', params[:file_name], placeholder: "File name",
         required: true, class: 'form-control new-file-name js-file-path-name-input'
+      = render 'template_selectors'
 
     .file-buttons
       - if is_markdown

app/views/projects/blob/_template_selectors.html.haml

-.template-selectors-menu
-  .templates-selectors-label
-    Template
+.template-selectors-menu.gl-pl-2
   .template-selector-dropdowns-wrap
     .template-type-selector.js-template-type-selector-wrap.hidden
-      = dropdown_tag("Choose type", options: { toggle_class: 'js-template-type-selector qa-template-type-dropdown', title: "Choose a template type" } )
+      = dropdown_tag(_("Select a template type"), options: { toggle_class: 'js-template-type-selector qa-template-type-dropdown', dropdown_class: 'dropdown-menu-selectable'} )
     .license-selector.js-license-selector-wrap.js-template-selector-wrap.hidden
-      = dropdown_tag("Apply a license template", options: { toggle_class: 'js-license-selector qa-license-dropdown', title: "Apply a license", filter: true, placeholder: "Filter", data: { data: licenses_for_select(@project), project: @project.name, fullname: @project.namespace.human_name } } )
+      = dropdown_tag(_("Apply a template"), options: { toggle_class: 'js-license-selector qa-license-dropdown', dropdown_class: 'dropdown-menu-selectable', filter: true, placeholder: "Filter", data: { data: licenses_for_select(@project), project: @project.name, fullname: @project.namespace.human_name } } )
     .gitignore-selector.js-gitignore-selector-wrap.js-template-selector-wrap.hidden
-      = dropdown_tag("Apply a .gitignore template", options: { toggle_class: 'js-gitignore-selector qa-gitignore-dropdown', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: gitignore_names(@project) } } )
+      = dropdown_tag(_("Apply a template"), options: { toggle_class: 'js-gitignore-selector qa-gitignore-dropdown', dropdown_class: 'dropdown-menu-selectable', filter: true, placeholder: "Filter", data: { data: gitignore_names(@project) } } )
     .gitlab-ci-yml-selector.js-gitlab-ci-yml-selector-wrap.js-template-selector-wrap.hidden
-      = dropdown_tag("Apply a GitLab CI Yaml template", options: { toggle_class: 'js-gitlab-ci-yml-selector qa-gitlab-ci-yml-dropdown', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: gitlab_ci_ymls(@project) } } )
+      = dropdown_tag(_("Apply a template"), options: { toggle_class: 'js-gitlab-ci-yml-selector qa-gitlab-ci-yml-dropdown', dropdown_class: 'dropdown-menu-selectable', filter: true, placeholder: "Filter", data: { data: gitlab_ci_ymls(@project) } } )
     .dockerfile-selector.js-dockerfile-selector-wrap.js-template-selector-wrap.hidden
-      = dropdown_tag("Apply a Dockerfile template", options: { toggle_class: 'js-dockerfile-selector qa-dockerfile-dropdown', title: "Apply a template", filter: true, placeholder: "Filter", data: { data: dockerfile_names(@project) } } )
-  .template-selectors-undo-menu.hidden
-    %span.text-info Template applied
-    %button.btn.btn-sm.btn-info Undo
+      = dropdown_tag(_("Apply a template"), options: { toggle_class: 'js-dockerfile-selector qa-dockerfile-dropdown', dropdown_class: 'dropdown-menu-selectable', filter: true, placeholder: "Filter", data: { data: dockerfile_names(@project) } } )

app/views/projects/blob/edit.html.haml

@@ -11,7 +11,6 @@
 .editor-title-row
   %h3.page-title.blob-edit-page-title
     Edit file
-  = render 'template_selectors'
 .file-editor
   %ul.nav-links.no-bottom.js-edit-mode.nav.nav-tabs
     %li.active

app/views/projects/blob/new.html.haml

@@ -5,7 +5,6 @@
 .editor-title-row
   %h3.page-title.blob-new-page-title
     New file
-  = render 'template_selectors'
 .file-editor
   = form_tag(project_create_blob_path(@project, @id), method: :post, class: 'js-edit-blob-form js-new-blob-form js-quick-submit js-requires-input', data: blob_editor_paths(@project)) do
     = render 'projects/blob/editor', ref: @ref

changelogs/unreleased/30807-fix-usability-problem.yml

+---
+title: Fix usability problems with the file template picker
+merge_request: 17522
+author:
+type: changed

locale/gitlab.pot

@@ -1758,6 +1758,9 @@ msgstr ""
 msgid "Apply a label"
 msgstr ""
 
+msgid "Apply a template"
+msgstr ""
+
 msgid "Apply suggestion"
 msgstr ""
 
@@ -14372,6 +14375,9 @@ msgstr ""
 msgid "Select a template repository"
 msgstr ""
 
+msgid "Select a template type"
+msgstr ""
+
 msgid "Select a timezone"
 msgstr ""
 

spec/features/projects/files/dockerfile_dropdown_spec.rb

@@ -23,7 +23,7 @@ describe 'Projects > Files > User wants to add a Dockerfile file' do
 
     wait_for_requests
 
-    expect(page).to have_css('.dockerfile-selector .dropdown-toggle-text', text: 'HTTPd')
+    expect(page).to have_css('.dockerfile-selector .dropdown-toggle-text', text: 'Apply a template')
     expect(page).to have_content('COPY ./ /usr/local/apache2/htdocs/')
   end
 end

spec/features/projects/files/gitignore_dropdown_spec.rb

@@ -23,7 +23,7 @@ describe 'Projects > Files > User wants to add a .gitignore file' do
 
     wait_for_requests
 
-    expect(page).to have_css('.gitignore-selector .dropdown-toggle-text', text: 'Rails')
+    expect(page).to have_css('.gitignore-selector .dropdown-toggle-text', text: 'Apply a template')
     expect(page).to have_content('/.bundle')
     expect(page).to have_content('# Gemfile.lock, .ruby-version, .ruby-gemset')
   end

spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb

@@ -23,7 +23,7 @@ describe 'Projects > Files > User wants to add a .gitlab-ci.yml file' do
 
     wait_for_requests
 
-    expect(page).to have_css('.gitlab-ci-yml-selector .dropdown-toggle-text', text: 'Jekyll')
+    expect(page).to have_css('.gitlab-ci-yml-selector .dropdown-toggle-text', text: 'Apply a template')
     expect(page).to have_content('This file is a template, and might need editing before it works on your project')
     expect(page).to have_content('jekyll build -d test')
   end

spec/features/projects/files/project_owner_creates_license_file_spec.rb

@@ -64,7 +64,7 @@ describe 'Projects > Files > Project owner creates a license file', :js do
 
   def select_template(template)
     page.within('.js-license-selector-wrap') do
-      click_button 'Apply a license template'
+      click_button 'Apply a template'
       click_link template
       wait_for_requests
     end

spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb

@@ -37,7 +37,7 @@ describe 'Projects > Files > Project owner sees a link to create a license file
 
   def select_template(template)
     page.within('.js-license-selector-wrap') do
-      click_button 'Apply a license template'
+      click_button 'Apply a template'
       click_link template
       wait_for_requests
     end

spec/features/projects/files/template_type_dropdown_spec.rb

@@ -24,8 +24,9 @@ describe 'Projects > Files > Template type dropdown selector', :js do
       try_selecting_all_types
     end
 
-    it 'updates toggle value when input matches' do
+    it 'updates template type toggle value when template is chosen' do
       fill_in 'file_path', with: '.gitignore'
+      select_template('gitignore', 'Actionscript')
       check_type_selector_toggle_text('.gitignore')
     end
   end
@@ -70,6 +71,7 @@ describe 'Projects > Files > Template type dropdown selector', :js do
     end
 
     it 'toggle is set to the correct value' do
+      select_template('gitignore', 'Actionscript')
       check_type_selector_toggle_text('.gitignore')
     end
 
@@ -88,7 +90,7 @@ describe 'Projects > Files > Template type dropdown selector', :js do
     end
 
     it 'toggle is set to the proper value' do
-      check_type_selector_toggle_text('Choose type')
+      check_type_selector_toggle_text('Select a template type')
     end
 
     it 'selects every template type correctly' do
@@ -103,16 +105,15 @@ def check_type_selector_display(is_visible)
 end
 
 def try_selecting_all_types
-  try_selecting_template_type('LICENSE', 'Apply a license template')
-  try_selecting_template_type('Dockerfile', 'Apply a Dockerfile template')
-  try_selecting_template_type('.gitlab-ci.yml', 'Apply a GitLab CI Yaml template')
-  try_selecting_template_type('.gitignore', 'Apply a .gitignore template')
+  try_selecting_template_type('LICENSE', 'Apply a template')
+  try_selecting_template_type('Dockerfile', 'Apply a template')
+  try_selecting_template_type('.gitlab-ci.yml', 'Apply a template')
+  try_selecting_template_type('.gitignore', 'Apply a template')
 end
 
 def try_selecting_template_type(template_type, selector_label)
   select_template_type(template_type)
   check_template_selector_display(selector_label)
-  check_type_selector_toggle_text(template_type)
 end
 
 def select_template_type(template_type)
@@ -120,6 +121,11 @@ def select_template_type(template_type)
   find('.dropdown-content li', text: template_type).click
 end
 
+def select_template(type, template)
+  find(".js-#{type}-selector-wrap").click
+  find('.dropdown-content li', text: template).click
+end
+
 def check_template_selector_display(content)
   expect(page).to have_content(content)
 end

spec/features/projects/files/undo_template_spec.rb

@@ -13,11 +13,12 @@ describe 'Projects > Files > Template Undo Button', :js do
   context 'editing a matching file and applying a template' do
     before do
       visit project_edit_blob_path(project, File.join(project.default_branch, "LICENSE"))
+      select_file_template_type('LICENSE')
       select_file_template('.js-license-selector', 'Apache License 2.0')
     end
 
     it 'reverts template application' do
-      try_template_undo('http://www.apache.org/licenses/', 'Apply a license template')
+      try_template_undo('http://www.apache.org/licenses/', 'Apply a template')
     end
   end
 
@@ -29,7 +30,7 @@ describe 'Projects > Files > Template Undo Button', :js do
     end
 
     it 'reverts template application' do
-      try_template_undo('http://www.apache.org/licenses/', 'Apply a license template')
+      try_template_undo('http://www.apache.org/licenses/', 'Apply a template')
     end
   end
 end
@@ -45,12 +46,12 @@ def check_toggle_text_set(neutral_toggle_text)
 end
 
 def check_undo_button_display
-  expect(page).to have_content('Template applied')
-  expect(page).to have_css('.template-selectors-undo-menu .btn-info')
+  expect(page).to have_content('template applied')
+  expect(page).to have_css('.toasted-container')
 end
 
 def check_content_reverted(template_content)
-  find('.template-selectors-undo-menu .btn-info').click
+  find('.toasted-container a', text: 'Undo').click
   expect(page).not_to have_content(template_content)
   expect(page).to have_css('.template-type-selector .dropdown-toggle-text')
 end