Merge branch 'ability_to_cancel_attaching_file' into 'master'

Add an ability to cancel attaching file and redesign attaching files UI

Closes #15611, #24270, and #28905

See merge request !9431

app/assets/stylesheets/pages/note_form.scss

@@ -277,6 +277,7 @@
 .toolbar-text {
   font-size: 14px;
   line-height: 16px;
+  margin-top: 2px;
 
   @media (min-width: $screen-md-min) {
     float: left;
@@ -402,3 +403,45 @@
     }
   }
 }
+
+.uploading-container {
+  float: right;
+
+  @media (max-width: $screen-xs-max) {
+    float: left;
+    margin-top: 5px;
+  }
+}
+
+.uploading-error-icon,
+.uploading-error-message {
+  color: $gl-text-red;
+}
+
+.uploading-error-message {
+  @media (max-width: $screen-xs-max) {
+    &::after {
+      content: "\a";
+      white-space: pre;
+    }
+  }
+}
+
+.uploading-progress {
+  margin-right: 5px;
+}
+
+.attach-new-file,
+.button-attach-file,
+.retry-uploading-link {
+  color: $gl-link-color;
+  padding: 0;
+  background: none;
+  border: 0;
+  font-size: 14px;
+  line-height: 16px;
+}
+
+.markdown-selector {
+  color: $gl-link-color;
+}

app/helpers/icons_helper.rb

@@ -7,9 +7,10 @@ module IconsHelper
   # font-awesome-rails gem, but should we ever use a different icon pack in the
   # future we won't have to change hundreds of method calls.
   def icon(names, options = {})
-    if (options.keys & %w[aria-hidden aria-label]).empty?
-      # Add `aria-hidden` if there are no aria's set
+    if (options.keys & %w[aria-hidden aria-label data-hidden]).empty?
+      # Add 'aria-hidden' and 'data-hidden' if they are not set in options.
       options['aria-hidden'] = true
+      options['data-hidden'] = true
     end
 
     options.include?(:base) ? fa_stacked_icon(names, options) : fa_icon(names, options)

app/views/shared/notes/_hints.html.haml

@@ -9,6 +9,27 @@
     - else
       is
     supported
-  %button.toolbar-button.markdown-selector{ type: 'button', tabindex: '-1' }
-    = icon('file-image-o', class: 'toolbar-button-icon')
-    Attach a file
+
+  %span.uploading-container
+    %span.uploading-progress-container.hide
+      = icon('file-image-o', class: 'toolbar-button-icon')
+      %span.attaching-file-message
+        -# Populated by app/assets/javascripts/dropzone_input.js
+      %span.uploading-progress 0%
+      %span.uploading-spinner
+        = icon('spinner spin', class: 'toolbar-button-icon')
+
+    %span.uploading-error-container.hide
+      %span.uploading-error-icon
+        = icon('file-image-o', class: 'toolbar-button-icon')
+      %span.uploading-error-message
+        -# Populated by app/assets/javascripts/dropzone_input.js
+      %button.retry-uploading-link{ type: 'button' } Try again
+      or
+      %button.attach-new-file.markdown-selector{ type: 'button' } attach a new file
+
+    %button.markdown-selector.button-attach-file{ type: 'button', tabindex: '-1' }
+      = icon('file-image-o', class: 'toolbar-button-icon')
+      Attach a file
+
+    %button.btn.btn-default.btn-xs.hide.button-cancel-uploading-files{ type: 'button' } Cancel

changelogs/unreleased/add_ability_to_cancel_attaching_file_and_redesign_attaching_files_ui.yml

+---
+title: Add an ability to cancel attaching file and redesign attaching files UI
+merge_request: 9431
+author: blackst0ne

spec/features/uploads/user_uploads_file_to_note_spec.rb

@@ -5,18 +5,78 @@ feature 'User uploads file to note', feature: true do
 
   let(:user) { create(:user) }
   let(:project) { create(:empty_project, creator: user, namespace: user.namespace) }
+  let(:issue) { create(:issue, project: project, author: user) }
 
-  scenario 'they see the attached file', js: true do
-    issue = create(:issue, project: project, author: user)
-
+  before do
     login_as(user)
     visit namespace_project_issue_path(project.namespace, project, issue)
+  end
+
+  context 'before uploading' do
+    it 'shows "Attach a file" button', js: true do
+      expect(page).to have_button('Attach a file')
+      expect(page).not_to have_selector('.uploading-progress-container', visible: true)
+    end
+  end
+
+  context 'uploading is in progress' do
+    it 'shows "Cancel" button on uploading', js: true do
+      dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')], 0, false)
+
+      expect(page).to have_button('Cancel')
+    end
+
+    it 'cancels uploading on clicking to "Cancel" button', js: true do
+      dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')], 0, false)
+
+      click_button 'Cancel'
+
+      expect(page).to have_button('Attach a file')
+      expect(page).not_to have_button('Cancel')
+      expect(page).not_to have_selector('.uploading-progress-container', visible: true)
+    end
+
+    it 'shows "Attaching a file" message on uploading 1 file', js: true do
+      dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')], 0, false)
+
+      expect(page).to have_selector('.attaching-file-message', visible: true, text: 'Attaching a file -')
+    end
+
+    it 'shows "Attaching 2 files" message on uploading 2 file', js: true do
+      dropzone_file([Rails.root.join('spec', 'fixtures', 'video_sample.mp4'),
+                     Rails.root.join('spec', 'fixtures', 'dk.png')], 0, false)
+
+      expect(page).to have_selector('.attaching-file-message', visible: true, text: 'Attaching 2 files -')
+    end
+
+    it 'shows error message, "retry" and "attach a new file" link a if file is too big', js: true do
+      dropzone_file([Rails.root.join('spec', 'fixtures', 'video_sample.mp4')], 0.01)
+
+      error_text = 'File is too big (0.06MiB). Max filesize: 0.01MiB.'
+
+      expect(page).to have_selector('.uploading-error-message', visible: true, text: error_text)
+      expect(page).to have_selector('.retry-uploading-link', visible: true, text: 'Try again')
+      expect(page).to have_selector('.attach-new-file', visible: true, text: 'attach a new file')
+      expect(page).not_to have_button('Attach a file')
+    end
+  end
+
+  context 'uploading is complete' do
+    it 'shows "Attach a file" button on uploading complete', js: true do
+      dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')])
+      wait_for_ajax
+
+      expect(page).to have_button('Attach a file')
+      expect(page).not_to have_selector('.uploading-progress-container', visible: true)
+    end
 
-    dropzone_file(Rails.root.join('spec', 'fixtures', 'dk.png'))
-    click_button 'Comment'
-    wait_for_ajax
+    scenario 'they see the attached file', js: true do
+      dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')])
+      click_button 'Comment'
+      wait_for_ajax
 
-    expect(find('a.no-attachment-icon img[alt="dk"]')['src'])
-      .to match(%r{/#{project.full_path}/uploads/\h{32}/dk\.png$})
+      expect(find('a.no-attachment-icon img[alt="dk"]')['src'])
+        .to match(%r{/#{project.full_path}/uploads/\h{32}/dk\.png$})
+    end
   end
 end

spec/support/dropzone_helper.rb

@@ -6,32 +6,52 @@ module DropzoneHelper
   # Dropzone events to perform the actual upload.
   #
   # This method waits for the upload to complete before returning.
-  def dropzone_file(file_path)
+  # max_file_size is an optional parameter.
+  # If it's not 0, then it used in dropzone.maxFilesize parameter.
+  # wait_for_queuecomplete is an optional parameter.
+  # If it's 'false', then the helper will NOT wait for backend response
+  # It lets to test behaviors while AJAX is processing.
+  def dropzone_file(files, max_file_size = 0, wait_for_queuecomplete = true)
     # Generate a fake file input that Capybara can attach to
     page.execute_script <<-JS.strip_heredoc
+      $('#fakeFileInput').remove();
       var fakeFileInput = window.$('<input/>').attr(
-        {id: 'fakeFileInput', type: 'file'}
+        {id: 'fakeFileInput', type: 'file', multiple: true}
       ).appendTo('body');
 
       window._dropzoneComplete = false;
     JS
 
-    # Attach the file to the fake input selector with Capybara
-    attach_file('fakeFileInput', file_path)
+    # Attach files to the fake input selector with Capybara
+    attach_file('fakeFileInput', files)
 
     # Manually trigger a Dropzone "drop" event with the fake input's file list
     page.execute_script <<-JS.strip_heredoc
-      var fileList = [$('#fakeFileInput')[0].files[0]];
-      var e = jQuery.Event('drop', { dataTransfer : { files : fileList } });
-
       var dropzone = $('.div-dropzone')[0].dropzone;
+      dropzone.options.autoProcessQueue = false;
+
+      if (#{max_file_size} > 0) {
+        dropzone.options.maxFilesize = #{max_file_size};
+      }
+
       dropzone.on('queuecomplete', function() {
         window._dropzoneComplete = true;
       });
-      dropzone.listeners[0].events.drop(e);
+
+      var fileList = [$('#fakeFileInput')[0].files];
+
+      $.map(fileList, function(file){
+        var e = jQuery.Event('drop', { dataTransfer : { files : file } });
+
+        dropzone.listeners[0].events.drop(e);
+      });
+
+      dropzone.processQueue();
     JS
 
-    # Wait until Dropzone's fired `queuecomplete`
-    loop until page.evaluate_script('window._dropzoneComplete === true')
+    if wait_for_queuecomplete
+      # Wait until Dropzone's fired `queuecomplete`
+      loop until page.evaluate_script('window._dropzoneComplete === true')
+    end
   end
 end