Remove negative-margin-top class

Users can click on "Run Pipeline" to start their pipelines on
merge requests.

The layout was achieved by repurposing the "Run pipeline" button
so it shows in the right place in mobile and desktop by adding
some negative margins.

Now this is done by adding a slot to place the button inside the table.

app/assets/javascripts/commit/pipelines/pipelines_table.vue

 <script>
 import { GlButton, GlLoadingIcon, GlModal, GlLink } from '@gitlab/ui';
-import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
 import PipelinesService from '~/pipelines/services/pipelines_service';
 import PipelineStore from '~/pipelines/stores/pipelines_store';
 import pipelinesMixin from '~/pipelines/mixins/pipelines';
@@ -126,16 +125,6 @@ export default {
         (latest.flags.detached_merge_request_pipeline || latest.flags.merge_request_pipeline)
       );
     },
-    /**
-     * When we are on Desktop and the button is visible
-     * we need to add a negative margin to the table
-     * to make it inline with the button
-     *
-     * @returns {Boolean}
-     */
-    shouldAddNegativeMargin() {
-      return this.canRenderPipelineButton && bp.isDesktop();
-    },
   },
   created() {
     this.service = new PipelinesService(this.endpoint);
@@ -205,65 +194,76 @@ export default {
     />
 
     <div v-else-if="shouldRenderTable" class="table-holder">
-      <div v-if="canRenderPipelineButton" class="nav justify-content-end">
-        <gl-button
-          variant="success"
-          class="js-run-mr-pipeline gl-mt-3 btn-wide-on-xs"
-          :disabled="state.isRunningMergeRequestPipeline"
-          @click="tryRunPipeline"
-        >
-          <gl-loading-icon v-if="state.isRunningMergeRequestPipeline" inline />
-          {{ s__('Pipelines|Run Pipeline') }}
-        </gl-button>
-
-        <gl-modal
-          :id="modalId"
-          ref="modal"
-          :modal-id="modalId"
-          :title="s__('Pipelines|Are you sure you want to run this pipeline?')"
-          :ok-title="s__('Pipelines|Run Pipeline')"
-          ok-variant="danger"
-          @ok="onClickRunPipeline"
-        >
-          <p>
-            {{
-              s__(
-                'Pipelines|This pipeline will run code originating from a forked project merge request. This means that the code can potentially have security considerations like exposing CI variables.',
-              )
-            }}
-          </p>
-          <p>
-            {{
-              s__(
-                "Pipelines|It is recommended the code is reviewed thoroughly before running this pipeline with the parent project's CI resource.",
-              )
-            }}
-          </p>
-          <p>
-            {{
-              s__(
-                'Pipelines|If you are unsure, please ask a project maintainer to review it for you.',
-              )
-            }}
-          </p>
-          <gl-link
-            href="/help/ci/merge_request_pipelines/index.html#run-pipelines-in-the-parent-project-for-merge-requests-from-a-forked-project"
-            target="_blank"
-          >
-            {{ s__('Pipelines|More Information') }}
-          </gl-link>
-        </gl-modal>
-      </div>
+      <gl-button
+        v-if="canRenderPipelineButton"
+        block
+        class="gl-mt-3 gl-mb-0 gl-display-md-none"
+        variant="success"
+        data-testid="run_pipeline_button_mobile"
+        :loading="state.isRunningMergeRequestPipeline"
+        @click="tryRunPipeline"
+      >
+        {{ s__('Pipelines|Run Pipeline') }}
+      </gl-button>
 
       <pipelines-table-component
         :pipelines="state.pipelines"
         :update-graph-dropdown="updateGraphDropdown"
         :auto-devops-help-path="autoDevopsHelpPath"
         :view-type="viewType"
-        :class="{ 'negative-margin-top': shouldAddNegativeMargin }"
-      />
+      >
+        <template #table-header-actions>
+          <div v-if="canRenderPipelineButton" class="gl-text-right">
+            <gl-button
+              variant="success"
+              data-testid="run_pipeline_button"
+              :loading="state.isRunningMergeRequestPipeline"
+              @click="tryRunPipeline"
+            >
+              {{ s__('Pipelines|Run Pipeline') }}
+            </gl-button>
+          </div>
+        </template>
+      </pipelines-table-component>
     </div>
 
+    <gl-modal
+      v-if="canRenderPipelineButton"
+      :id="modalId"
+      ref="modal"
+      :modal-id="modalId"
+      :title="s__('Pipelines|Are you sure you want to run this pipeline?')"
+      :ok-title="s__('Pipelines|Run Pipeline')"
+      ok-variant="danger"
+      @ok="onClickRunPipeline"
+    >
+      <p>
+        {{
+          s__(
+            'Pipelines|This pipeline will run code originating from a forked project merge request. This means that the code can potentially have security considerations like exposing CI variables.',
+          )
+        }}
+      </p>
+      <p>
+        {{
+          s__(
+            "Pipelines|It is recommended the code is reviewed thoroughly before running this pipeline with the parent project's CI resource.",
+          )
+        }}
+      </p>
+      <p>
+        {{
+          s__('Pipelines|If you are unsure, please ask a project maintainer to review it for you.')
+        }}
+      </p>
+      <gl-link
+        href="/help/ci/merge_request_pipelines/index.html#run-pipelines-in-the-parent-project-for-merge-requests-from-a-forked-project"
+        target="_blank"
+      >
+        {{ s__('Pipelines|More Information') }}
+      </gl-link>
+    </gl-modal>
+
     <table-pagination
       v-if="shouldRenderPagination"
       :change="onChangePage"

app/assets/javascripts/pipelines/components/pipelines_list/pipelines_table.vue

@@ -91,6 +91,10 @@ export default {
       <div class="table-section section-15 js-pipeline-stages pipeline-stages" role="rowheader">
         {{ s__('Pipeline|Stages') }}
       </div>
+      <div class="table-section section-15" role="rowheader"></div>
+      <div class="table-section section-20" role="rowheader">
+        <slot name="table-header-actions"></slot>
+      </div>
     </div>
     <pipelines-table-row-component
       v-for="model in pipelines"

app/assets/stylesheets/framework/buttons.scss

@@ -417,12 +417,6 @@
   }
 }
 
-@include media-breakpoint-down(xs) {
-  .btn-wide-on-xs {
-    width: 100%;
-  }
-}
-
 .btn-blank {
   padding: 0;
   background: transparent;

app/assets/stylesheets/framework/variables.scss

@@ -819,7 +819,6 @@ $pipeline-dropdown-line-height: 20px;
 $pipeline-dropdown-status-icon-size: 18px;
 $ci-action-dropdown-button-size: 24px;
 $ci-action-dropdown-svg-size: 12px;
-$pipelines-table-header-height: 40px;
 
 /*
 CI variable lists

app/assets/stylesheets/pages/pipelines.scss

@@ -26,10 +26,6 @@
 }
 
 .pipelines {
-  .negative-margin-top {
-    margin-top: -$pipelines-table-header-height;
-  }
-
   .stage {
     max-width: 90px;
     width: 90px;

spec/features/merge_request/user_sees_pipelines_spec.rb

@@ -50,7 +50,7 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
 
           wait_for_requests
 
-          expect(page.find('.js-run-mr-pipeline')).to have_text('Run Pipeline')
+          expect(page.find('[data-testid="run_pipeline_button"]')).to have_text('Run Pipeline')
         end
       end
 
@@ -66,7 +66,7 @@ RSpec.describe 'Merge request > User sees pipelines', :js do
 
           wait_for_requests
 
-          expect(page.find('.js-run-mr-pipeline')).to have_text('Run Pipeline')
+          expect(page.find('[data-testid="run_pipeline_button"]')).to have_text('Run Pipeline')
         end
       end
     end

spec/frontend/commit/pipelines/pipelines_spec.js

@@ -21,6 +21,10 @@ describe('Pipelines table in Commits and Merge requests', () => {
 
   preloadFixtures(jsonFixtureName);
 
+  const findRunPipelineBtn = () => vm.$el.querySelector('[data-testid="run_pipeline_button"]');
+  const findRunPipelineBtnMobile = () =>
+    vm.$el.querySelector('[data-testid="run_pipeline_button_mobile"]');
+
   beforeEach(() => {
     mock = new MockAdapter(axios);
 
@@ -131,7 +135,8 @@ describe('Pipelines table in Commits and Merge requests', () => {
         vm = mountComponent(PipelinesTable, { ...props });
 
         setImmediate(() => {
-          expect(vm.$el.querySelector('.js-run-mr-pipeline')).not.toBeNull();
+          expect(findRunPipelineBtn()).not.toBeNull();
+          expect(findRunPipelineBtnMobile()).not.toBeNull();
           done();
         });
       });
@@ -147,7 +152,8 @@ describe('Pipelines table in Commits and Merge requests', () => {
         vm = mountComponent(PipelinesTable, { ...props });
 
         setImmediate(() => {
-          expect(vm.$el.querySelector('.js-run-mr-pipeline')).toBeNull();
+          expect(findRunPipelineBtn()).toBeNull();
+          expect(findRunPipelineBtnMobile()).toBeNull();
           done();
         });
       });
@@ -157,7 +163,7 @@ describe('Pipelines table in Commits and Merge requests', () => {
       const findModal = () =>
         document.querySelector('#create-pipeline-for-fork-merge-request-modal');
 
-      beforeEach(() => {
+      beforeEach(done => {
         pipelineCopy.flags.detached_merge_request_pipeline = true;
 
         mock.onGet('endpoint.json').reply(200, [pipelineCopy]);
@@ -168,23 +174,46 @@ describe('Pipelines table in Commits and Merge requests', () => {
           projectId: '5',
           mergeRequestId: 3,
         });
-      });
 
-      it('updates the loading state', done => {
         jest.spyOn(Api, 'postMergeRequestPipeline').mockReturnValue(Promise.resolve());
 
         setImmediate(() => {
-          vm.$el.querySelector('.js-run-mr-pipeline').click();
+          done();
+        });
+      });
 
-          vm.$nextTick(() => {
-            expect(findModal()).toBeNull();
-            expect(vm.state.isRunningMergeRequestPipeline).toBe(true);
+      it('on desktop, shows a loading button', done => {
+        findRunPipelineBtn().click();
 
-            setImmediate(() => {
-              expect(vm.state.isRunningMergeRequestPipeline).toBe(false);
+        vm.$nextTick(() => {
+          expect(findModal()).toBeNull();
 
-              done();
-            });
+          expect(findRunPipelineBtn().disabled).toBe(true);
+          expect(findRunPipelineBtn().querySelector('.gl-spinner')).not.toBeNull();
+
+          setImmediate(() => {
+            expect(findRunPipelineBtn().disabled).toBe(false);
+            expect(findRunPipelineBtn().querySelector('.gl-spinner')).toBeNull();
+
+            done();
+          });
+        });
+      });
+
+      it('on mobile, shows a loading button', done => {
+        findRunPipelineBtnMobile().click();
+
+        vm.$nextTick(() => {
+          expect(findModal()).toBeNull();
+
+          expect(findModal()).toBeNull();
+          expect(findRunPipelineBtn().querySelector('.gl-spinner')).not.toBeNull();
+
+          setImmediate(() => {
+            expect(findRunPipelineBtn().disabled).toBe(false);
+            expect(findRunPipelineBtn().querySelector('.gl-spinner')).toBeNull();
+
+            done();
           });
         });
       });
@@ -194,7 +223,7 @@ describe('Pipelines table in Commits and Merge requests', () => {
       const findModal = () =>
         document.querySelector('#create-pipeline-for-fork-merge-request-modal');
 
-      beforeEach(() => {
+      beforeEach(done => {
         pipelineCopy.flags.detached_merge_request_pipeline = true;
 
         mock.onGet('endpoint.json').reply(200, [pipelineCopy]);
@@ -207,18 +236,29 @@ describe('Pipelines table in Commits and Merge requests', () => {
           sourceProjectFullPath: 'test/parent-project',
           targetProjectFullPath: 'test/fork-project',
         });
-      });
 
-      it('shows a security warning modal', done => {
         jest.spyOn(Api, 'postMergeRequestPipeline').mockReturnValue(Promise.resolve());
 
         setImmediate(() => {
-          vm.$el.querySelector('.js-run-mr-pipeline').click();
+          done();
+        });
+      });
 
-          vm.$nextTick(() => {
-            expect(findModal()).not.toBeNull();
-            done();
-          });
+      it('on desktop, shows a security warning modal', done => {
+        findRunPipelineBtn().click();
+
+        vm.$nextTick(() => {
+          expect(findModal()).not.toBeNull();
+          done();
+        });
+      });
+
+      it('on mobile, shows a security warning modal', done => {
+        findRunPipelineBtnMobile().click();
+
+        vm.$nextTick(() => {
+          expect(findModal()).not.toBeNull();
+          done();
         });
       });
     });