Merge branch '32714-snippets-ux-improvement' into 'master'

Part 1 - Improve Snippets form UX with description field

See merge request gitlab-org/gitlab!24762

app/assets/javascripts/snippet/collapsible_input.js

+const hide = el => el.classList.add('d-none');
+const show = el => el.classList.remove('d-none');
+
+const setupCollapsibleInput = el => {
+  const collapsedEl = el.querySelector('.js-collapsed');
+  const expandedEl = el.querySelector('.js-expanded');
+  const collapsedInputEl = collapsedEl.querySelector('textarea,input,select');
+  const expandedInputEl = expandedEl.querySelector('textarea,input,select');
+  const formEl = el.closest('form');
+
+  const collapse = () => {
+    hide(expandedEl);
+    show(collapsedEl);
+  };
+
+  const expand = () => {
+    hide(collapsedEl);
+    show(expandedEl);
+  };
+
+  // NOTE:
+  // We add focus listener to all form inputs so that we can collapse
+  // when something is focused that's not the expanded input.
+  formEl.addEventListener('focusin', e => {
+    if (e.target === collapsedInputEl) {
+      expand();
+      expandedInputEl.focus();
+    } else if (!el.contains(e.target) && !expandedInputEl.value) {
+      collapse();
+    }
+  });
+};
+
+/**
+ * Usage in HAML
+ *
+ * .js-collapsible-input
+ *   .js-collapsed{ class: ('d-none' if is_expanded) }
+ *     = input
+ *   .js-expanded{ class: ('d-none' if !is_expanded) }
+ *     = big_input
+ */
+export default () => {
+  Array.from(document.querySelectorAll('.js-collapsible-input')).forEach(setupCollapsibleInput);
+};

app/assets/javascripts/snippet/snippet_bundle.js

 /* global ace */
 
 import $ from 'jquery';
+import setupCollapsibleInputs from './collapsible_input';
 
 export default () => {
   const editor = ace.edit('editor');
@@ -8,4 +9,6 @@ export default () => {
   $('.snippet-form-holder form').on('submit', () => {
     $('.snippet-file-content').val(editor.getValue());
   });
+
+  setupCollapsibleInputs();
 };

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

 - add_to_breadcrumbs _("Snippets"), project_snippets_path(@project)
 - breadcrumb_title @snippet.to_reference
 - page_title _("Edit"), "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets")
+- @content_class = "limit-container-width" unless fluid_layout
 
 %h3.page-title
   = _("Edit Snippet")

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

 - add_to_breadcrumbs _("Snippets"), project_snippets_path(@project)
 - breadcrumb_title _("New")
 - page_title _("New Snippet")
+- @content_class = "limit-container-width" unless fluid_layout
 
 %h3.page-title
   = _("New Snippet")

app/views/shared/snippets/_form.html.haml

@@ -6,27 +6,37 @@
              html: { class: "snippet-form js-requires-input js-quick-submit common-note-form" } do |f|
     = form_errors(@snippet)
 
-    .form-group.row
-      .col-sm-2.col-form-label
-        = f.label :title
-      .col-sm-10
-        = f.text_field :title, class: 'form-control qa-snippet-title', required: true, autofocus: true
-
-    = render 'shared/form_elements/description', model: @snippet, project: @project, form: f
-
-    = render 'shared/old_visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet, with_label: false
-
-    .file-editor
-      .form-group.row
-        .col-sm-2.col-form-label
-          = f.label :file_name, "File"
-        .col-sm-10
-          .file-holder.snippet
-            .js-file-title.file-title-flex-parent
-              = f.text_field :file_name, placeholder: "Optionally name this file to add code highlighting, e.g. example.rb for Ruby.", class: 'form-control snippet-file-name qa-snippet-file-name'
-            .file-content.code
-              %pre#editor= @snippet.content
-              = f.hidden_field :content, class: 'snippet-file-content'
+    .form-group
+      = f.label :title, class: 'label-bold'
+      = f.text_field :title, class: 'form-control qa-snippet-title', required: true, autofocus: true
+
+    .form-group.js-description-input
+      - description_placeholder = s_('Snippets|Optionally add a description about what your snippet does or how to use it...')
+      - is_expanded = @snippet.description && !@snippet.description.empty?
+      = f.label :description, s_("Snippets|Description (optional)"), class: 'label-bold'
+      .js-collapsible-input
+        .js-collapsed{ class: ('d-none' if is_expanded) }
+          = text_field_tag nil, nil, class: 'form-control', placeholder: description_placeholder
+        .js-expanded{ class: ('d-none' if !is_expanded) }
+          = render layout: 'projects/md_preview', locals: { url: preview_markdown_path(@project), referenced_users: true } do
+            = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', placeholder: description_placeholder
+            = render 'shared/notes/hints'
+
+    .form-group.file-editor
+      = f.label :file_name, s_('Snippets|File')
+      .file-holder.snippet
+        .js-file-title.file-title-flex-parent
+          = f.text_field :file_name, placeholder: s_("Snippets|Give your file a name to add code highlighting, e.g. example.rb for Ruby"), class: 'form-control snippet-file-name qa-snippet-file-name'
+        .file-content.code
+          %pre#editor= @snippet.content
+          = f.hidden_field :content, class: 'snippet-file-content'
+
+    .form-group
+      .font-weight-bold
+        = _('Visibility level')
+        = link_to icon('question-circle'), help_page_path("public_access/public_access"), target: '_blank'
+      = render 'shared/visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet, with_label: false
+
     - if params[:files]
       - params[:files].each_with_index do |file, index|
         = hidden_field_tag "files[]", file, id: "files_#{index}"

app/views/snippets/edit.html.haml

 - page_title _("Edit"), "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets")
+- @content_class = "limit-container-width" unless fluid_layout
 
 %h3.page-title
   = _("Edit Snippet")

app/views/snippets/new.html.haml

 - @hide_top_links = true
 - @hide_breadcrumbs = true
 - page_title _("New Snippet")
+- @content_class = "limit-container-width" unless fluid_layout
 
 .page-title-holder.d-flex.align-items-center
   %h1.page-title= _('New Snippet')

changelogs/unreleased/32714-snippets-ux-improvement.yml

+---
+title: Improve UX of optional fields in Snippets form
+merge_request: 24762
+author:
+type: other

locale/gitlab.pot

@@ -17681,6 +17681,18 @@ msgstr ""
 msgid "SnippetsEmptyState|There are no snippets to show."
 msgstr ""
 
+msgid "Snippets|Description (optional)"
+msgstr ""
+
+msgid "Snippets|File"
+msgstr ""
+
+msgid "Snippets|Give your file a name to add code highlighting, e.g. example.rb for Ruby"
+msgstr ""
+
+msgid "Snippets|Optionally add a description about what your snippet does or how to use it..."
+msgstr ""
+
 msgid "Snowplow"
 msgstr ""
 

spec/features/projects/snippets/create_snippet_spec.rb

@@ -8,9 +8,17 @@ describe 'Projects > Snippets > Create Snippet', :js do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
 
+  def description_field
+    find('.js-description-input input,textarea')
+  end
+
   def fill_form
     fill_in 'project_snippet_title', with: 'My Snippet Title'
+
+    # Click placeholder first to expand full description field
+    description_field.click
     fill_in 'project_snippet_description', with: 'My Snippet **Description**'
+
     page.within('.file-editor') do
       find('.ace_text-input', visible: false).send_keys('Hello World!')
     end
@@ -27,6 +35,18 @@ describe 'Projects > Snippets > Create Snippet', :js do
       click_on('New snippet')
     end
 
+    it 'shows collapsible description input' do
+      collapsed = description_field
+
+      expect(page).not_to have_field('project_snippet_description')
+      expect(collapsed).to be_visible
+
+      collapsed.click
+
+      expect(page).to have_field('project_snippet_description')
+      expect(collapsed).not_to be_visible
+    end
+
     it 'creates a new snippet' do
       fill_form
       click_button('Create snippet')

spec/features/snippets/spam_snippets_spec.rb

@@ -5,6 +5,10 @@ require 'spec_helper'
 describe 'User creates snippet', :js do
   let(:user) { create(:user) }
 
+  def description_field
+    find('.js-description-input input,textarea')
+  end
+
   before do
     stub_feature_flags(allow_possible_spam: false)
     stub_feature_flags(snippets_vue: false)
@@ -22,7 +26,11 @@ describe 'User creates snippet', :js do
     visit new_snippet_path
 
     fill_in 'personal_snippet_title', with: 'My Snippet Title'
+
+    # Click placeholder first to expand full description field
+    description_field.click
     fill_in 'personal_snippet_description', with: 'My Snippet **Description**'
+
     find('#personal_snippet_visibility_level_20').set(true)
     page.within('.file-editor') do
       find('.ace_text-input', visible: false).send_keys 'Hello World!'

spec/features/snippets/user_creates_snippet_spec.rb

@@ -13,9 +13,17 @@ describe 'User creates snippet', :js do
     visit new_snippet_path
   end
 
+  def description_field
+    find('.js-description-input input,textarea')
+  end
+
   def fill_form
     fill_in 'personal_snippet_title', with: 'My Snippet Title'
+
+    # Click placeholder first to expand full description field
+    description_field.click
     fill_in 'personal_snippet_description', with: 'My Snippet **Description**'
+
     page.within('.file-editor') do
       find('.ace_text-input', visible: false).send_keys 'Hello World!'
     end
@@ -36,6 +44,8 @@ describe 'User creates snippet', :js do
   end
 
   it 'previews a snippet with file' do
+    # Click placeholder first to expand full description field
+    description_field.click
     fill_in 'personal_snippet_description', with: 'My Snippet'
     dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif')
     find('.js-md-preview-button').click

spec/frontend/snippet/collapsible_input_spec.js

+import setupCollapsibleInputs from '~/snippet/collapsible_input';
+import { setHTMLFixture } from 'helpers/fixtures';
+
+describe('~/snippet/collapsible_input', () => {
+  let formEl;
+  let descriptionEl;
+  let titleEl;
+  let fooEl;
+
+  beforeEach(() => {
+    setHTMLFixture(`
+      <form>    
+        <div class="js-collapsible-input js-title">
+          <div class="js-collapsed d-none">
+            <input type="text" />
+          </div>
+          <div class="js-expanded">
+            <textarea>Hello World!</textarea>
+          </div>
+        </div>
+        <div class="js-collapsible-input js-description">
+          <div class="js-collapsed">
+            <input type="text" />
+          </div>
+          <div class="js-expanded d-none">
+            <textarea></textarea>
+          </div>
+        </div>
+        <input type="text" class="js-foo" />
+      </form>
+    `);
+
+    formEl = document.querySelector('form');
+    titleEl = formEl.querySelector('.js-title');
+    descriptionEl = formEl.querySelector('.js-description');
+    fooEl = formEl.querySelector('.js-foo');
+
+    setupCollapsibleInputs();
+  });
+
+  const findInput = el => el.querySelector('textarea,input');
+  const findCollapsed = el => el.querySelector('.js-collapsed');
+  const findExpanded = el => el.querySelector('.js-expanded');
+  const findCollapsedInput = el => findInput(findCollapsed(el));
+  const findExpandedInput = el => findInput(findExpanded(el));
+  const focusIn = target => target.dispatchEvent(new Event('focusin', { bubbles: true }));
+  const expectIsCollapsed = (el, isCollapsed) => {
+    expect(findCollapsed(el).classList.contains('d-none')).toEqual(!isCollapsed);
+    expect(findExpanded(el).classList.contains('d-none')).toEqual(isCollapsed);
+  };
+
+  describe('when collapsed', () => {
+    it('is collapsed', () => {
+      expectIsCollapsed(descriptionEl, true);
+    });
+
+    describe('when focused', () => {
+      beforeEach(() => {
+        focusIn(findCollapsedInput(descriptionEl));
+      });
+
+      it('is expanded', () => {
+        expectIsCollapsed(descriptionEl, false);
+      });
+
+      describe.each`
+        desc                           | value             | isCollapsed
+        ${'is collapsed'}              | ${''}             | ${true}
+        ${'stays open if given value'} | ${'Hello world!'} | ${false}
+      `('when loses focus', ({ desc, value, isCollapsed }) => {
+        it(desc, () => {
+          findExpandedInput(descriptionEl).value = value;
+          focusIn(fooEl);
+
+          expectIsCollapsed(descriptionEl, isCollapsed);
+        });
+      });
+    });
+  });
+
+  describe('when expanded and has value', () => {
+    it('does not collapse, when focusing out', () => {
+      expectIsCollapsed(titleEl, false);
+
+      focusIn(fooEl);
+
+      expectIsCollapsed(titleEl, false);
+    });
+
+    describe('and loses value', () => {
+      beforeEach(() => {
+        findExpandedInput(titleEl).value = '';
+      });
+
+      it('collapses, when focusing out', () => {
+        expectIsCollapsed(titleEl, false);
+
+        focusIn(fooEl);
+
+        expectIsCollapsed(titleEl, true);
+      });
+    });
+  });
+});