Merge branch 'leipert-ajax-spinner-icons' into 'master'

Simplify implementation of AjaxLoadingSpinner

See merge request gitlab-org/gitlab!39968

app/assets/javascripts/ajax_loading_spinner.js

-import $ from 'jquery';
-
-export default class AjaxLoadingSpinner {
-  static init() {
-    const $elements = $('.js-ajax-loading-spinner');
-
-    $elements.on('ajax:beforeSend', AjaxLoadingSpinner.ajaxBeforeSend);
-    $elements.on('ajax:complete', AjaxLoadingSpinner.ajaxComplete);
-  }
-
-  static ajaxBeforeSend(e) {
-    e.target.setAttribute('disabled', '');
-    const iconElement = e.target.querySelector('i');
-    // get first fa- icon
-    const originalIcon = iconElement.className.match(/(fa-)([^\s]+)/g)[0];
-    iconElement.dataset.icon = originalIcon;
-    AjaxLoadingSpinner.toggleLoadingIcon(iconElement);
-    $(e.target).off('ajax:beforeSend', AjaxLoadingSpinner.ajaxBeforeSend);
-  }
-
-  static ajaxComplete(e) {
-    e.target.removeAttribute('disabled');
-    const iconElement = e.target.querySelector('i');
-    AjaxLoadingSpinner.toggleLoadingIcon(iconElement);
-    $(e.target).off('ajax:complete', AjaxLoadingSpinner.ajaxComplete);
-  }
-
-  static toggleLoadingIcon(iconElement) {
-    const { classList } = iconElement;
-    classList.toggle(iconElement.dataset.icon);
-    classList.toggle('gl-spinner');
-    classList.toggle('gl-spinner-orange');
-    classList.toggle('gl-spinner-sm');
-  }
-}

app/assets/javascripts/branches/ajax_loading_spinner.js

+import $ from 'jquery';
+
+export default class AjaxLoadingSpinner {
+  static init() {
+    const $elements = $('.js-ajax-loading-spinner');
+    $elements.on('ajax:beforeSend', AjaxLoadingSpinner.ajaxBeforeSend);
+  }
+
+  static ajaxBeforeSend(e) {
+    const button = e.target;
+    const newButton = document.createElement('button');
+    newButton.classList.add('btn', 'btn-default', 'disabled', 'gl-button');
+    newButton.setAttribute('disabled', 'disabled');
+
+    const spinner = document.createElement('span');
+    spinner.classList.add('align-text-bottom', 'gl-spinner', 'gl-spinner-sm', 'gl-spinner-orange');
+    newButton.appendChild(spinner);
+
+    button.classList.add('hidden');
+    button.parentNode.insertBefore(newButton, button.nextSibling);
+
+    $(button).one('ajax:error', () => {
+      newButton.remove();
+      button.classList.remove('hidden');
+    });
+
+    $(button).one('ajax:success', () => {
+      $(button).off('ajax:beforeSend', AjaxLoadingSpinner.ajaxBeforeSend);
+    });
+  }
+}

app/assets/javascripts/pages/projects/branches/index/index.js

-import AjaxLoadingSpinner from '~/ajax_loading_spinner';
+import AjaxLoadingSpinner from '~/branches/ajax_loading_spinner';
 import DeleteModal from '~/branches/branches_delete_modal';
 import initDiverganceGraph from '~/branches/divergence_graph';
 

app/views/projects/branches/_branch.html.haml

@@ -50,25 +50,25 @@
 
     - if can?(current_user, :push_code, @project)
       - if branch.name == @project.repository.root_ref
-        %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled",
+        %button{ class: "btn btn-remove remove-row has-tooltip disabled",
           disabled: true,
           title: s_('Branches|The default branch cannot be deleted') }
-          = icon("trash-o")
+          = sprite_icon("remove")
       - elsif protected_branch?(@project, branch)
         - if can?(current_user, :push_to_delete_protected_branch, @project)
-          %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip",
+          %button{ class: "btn btn-remove remove-row has-tooltip",
             title: s_('Branches|Delete protected branch'),
             data: { toggle: "modal",
               target: "#modal-delete-branch",
               delete_path: project_branch_path(@project, branch.name),
               branch_name: branch.name,
               is_merged: ("true" if merged) } }
-            = icon("trash-o")
+            = sprite_icon("remove")
         - else
-          %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled",
+          %button{ class: "btn btn-remove remove-row has-tooltip disabled",
             disabled: true,
             title: s_('Branches|Only a project maintainer or owner can delete a protected branch') }
-            = icon("trash-o")
+            = sprite_icon("remove")
       - else
         = link_to project_branch_path(@project, branch.name),
           class: "btn btn-remove remove-row qa-remove-btn js-ajax-loading-spinner has-tooltip",
@@ -77,4 +77,4 @@
           data: { confirm: s_("Branches|Deleting the '%{branch_name}' branch cannot be undone. Are you sure?") % { branch_name: branch.name } },
           remote: true,
           'aria-label' => s_('Branches|Delete branch') do
-          = icon("trash-o")
+          = sprite_icon("remove")

changelogs/unreleased/leipert-ajax-spinner-icons.yml

+---
+title: Change icon for branch delete button
+merge_request: 39968
+author:
+type: changed

spec/frontend/ajax_loading_spinner_spec.js

-import $ from 'jquery';
-import AjaxLoadingSpinner from '~/ajax_loading_spinner';
-
-describe('Ajax Loading Spinner', () => {
-  const fixtureTemplate = 'static/ajax_loading_spinner.html';
-  preloadFixtures(fixtureTemplate);
-
-  beforeEach(() => {
-    loadFixtures(fixtureTemplate);
-    AjaxLoadingSpinner.init();
-  });
-
-  it('change current icon with spinner icon and disable link while waiting ajax response', done => {
-    jest.spyOn($, 'ajax').mockImplementation(req => {
-      const xhr = new XMLHttpRequest();
-      const ajaxLoadingSpinner = document.querySelector('.js-ajax-loading-spinner');
-      const icon = ajaxLoadingSpinner.querySelector('i');
-
-      req.beforeSend(xhr, { dataType: 'text/html' });
-
-      expect(icon).not.toHaveClass('fa-trash-o');
-      expect(icon).toHaveClass('gl-spinner');
-      expect(icon).toHaveClass('gl-spinner-orange');
-      expect(icon).toHaveClass('gl-spinner-sm');
-      expect(icon.dataset.icon).toEqual('fa-trash-o');
-      expect(ajaxLoadingSpinner.getAttribute('disabled')).toEqual('');
-
-      req.complete({});
-
-      done();
-      const deferred = $.Deferred();
-      return deferred.promise();
-    });
-    document.querySelector('.js-ajax-loading-spinner').click();
-  });
-
-  it('use original icon again and enabled the link after complete the ajax request', done => {
-    jest.spyOn($, 'ajax').mockImplementation(req => {
-      const xhr = new XMLHttpRequest();
-      const ajaxLoadingSpinner = document.querySelector('.js-ajax-loading-spinner');
-
-      req.beforeSend(xhr, { dataType: 'text/html' });
-      req.complete({});
-
-      const icon = ajaxLoadingSpinner.querySelector('i');
-
-      expect(icon).toHaveClass('fa-trash-o');
-      expect(icon).not.toHaveClass('gl-spinner');
-      expect(icon).not.toHaveClass('gl-spinner-orange');
-      expect(icon).not.toHaveClass('gl-spinner-sm');
-      expect(ajaxLoadingSpinner.getAttribute('disabled')).toEqual(null);
-
-      done();
-      const deferred = $.Deferred();
-      return deferred.promise();
-    });
-    document.querySelector('.js-ajax-loading-spinner').click();
-  });
-});

spec/frontend/branches/ajax_loading_spinner_spec.js

+import AjaxLoadingSpinner from '~/branches/ajax_loading_spinner';
+
+describe('Ajax Loading Spinner', () => {
+  let ajaxLoadingSpinnerElement;
+  let fauxEvent;
+  beforeEach(() => {
+    document.body.innerHTML = `
+    <div>
+    <a class="js-ajax-loading-spinner"
+       data-remote
+       href="http://goesnowhere.nothing/whereami">
+      <i class="fa fa-trash-o"></i>
+    </a></div>`;
+    AjaxLoadingSpinner.init();
+    ajaxLoadingSpinnerElement = document.querySelector('.js-ajax-loading-spinner');
+    fauxEvent = { target: ajaxLoadingSpinnerElement };
+  });
+
+  afterEach(() => {
+    document.body.innerHTML = '';
+  });
+
+  it('`ajaxBeforeSend` event handler sets current icon to spinner and disables link', () => {
+    expect(ajaxLoadingSpinnerElement.parentNode.querySelector('.gl-spinner')).toBeNull();
+    expect(ajaxLoadingSpinnerElement.classList.contains('hidden')).toBe(false);
+
+    AjaxLoadingSpinner.ajaxBeforeSend(fauxEvent);
+
+    expect(ajaxLoadingSpinnerElement.parentNode.querySelector('.gl-spinner')).not.toBeNull();
+    expect(ajaxLoadingSpinnerElement.classList.contains('hidden')).toBe(true);
+  });
+});

spec/frontend/fixtures/static/ajax_loading_spinner.html

-<a class="js-ajax-loading-spinner" data-remote href="http://goesnowhere.nothing/whereami">
-<i class="fa fa-trash-o"></i>
-</a>