Merge branch '30807-fix-usability-problem' into 'master'

Fix usability problems for file template

Closes #30807

See merge request gitlab-org/gitlab!17522

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