From fd0ec1657ffed80f1e130b4f88714f1c59fb3d1b Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Mon, 28 Aug 2023 14:38:32 +0200
Subject: [PATCH] Job page: restructre page

Restructure page to align better with the new layout used in MRs.

Changelog: changed
---
 .../ci/job_details/components/job_header.vue  |  22 +--
 .../components/sidebar/sidebar.vue            |  24 +--
 .../components/sidebar/sidebar_header.vue     | 139 +++++++++++-------
 .../javascripts/ci/job_details/job_app.vue    |  29 ++--
 .../header/pipeline_details_header.vue        |   4 +-
 app/assets/stylesheets/framework/common.scss  |   7 +-
 app/assets/stylesheets/framework/mixins.scss  |   2 +-
 app/assets/stylesheets/framework/sidebar.scss |  10 +-
 .../stylesheets/page_bundles/build.scss       |  13 +-
 .../job_details/components/job_header_spec.js |   8 +-
 10 files changed, 139 insertions(+), 119 deletions(-)

diff --git a/app/assets/javascripts/ci/job_details/components/job_header.vue b/app/assets/javascripts/ci/job_details/components/job_header.vue
index e30cd3a947442c..00d15f870643e5 100644
--- a/app/assets/javascripts/ci/job_details/components/job_header.vue
+++ b/app/assets/javascripts/ci/job_details/components/job_header.vue
@@ -99,9 +99,20 @@ export default {
       <h1 class="gl-font-size-h-display gl-my-0 gl-display-inline-block" data-testid="job-name">
         {{ name }}
       </h1>
-    </div>
 
-    <section class="gl-md-display-flex gl-align-items-center gl-mr-3">
+      <div class="gl-display-flex gl-align-self-start gl-mt-n2">
+        <div class="gl-flex-grow-1 gl-flex-shrink-0 gl-text-right">
+          <gl-button
+            :aria-label="__('Toggle sidebar')"
+            category="secondary"
+            class="gl-lg-display-none gl-ml-2"
+            icon="chevron-double-lg-left"
+            @click="onClickSidebarButton"
+          />
+        </div>
+      </div>
+    </div>
+    <section class="header-main-content gl-display-flex gl-align-items-center gl-mr-3">
       <ci-badge-link class="gl-mr-3" :status="status" />
 
       <template v-if="shouldRenderTriggeredLabel">{{ __('Started') }}</template>
@@ -140,12 +151,5 @@ export default {
         </gl-avatar-link>
       </template>
     </section>
-
-    <gl-button
-      class="gl-md-display-none gl-ml-auto gl-align-self-start js-sidebar-build-toggle"
-      icon="chevron-double-lg-left"
-      :aria-label="__('Toggle sidebar')"
-      @click="onClickSidebarButton"
-    />
   </header>
 </template>
diff --git a/app/assets/javascripts/ci/job_details/components/sidebar/sidebar.vue b/app/assets/javascripts/ci/job_details/components/sidebar/sidebar.vue
index 860c4c8300973e..231f45d7ae633c 100644
--- a/app/assets/javascripts/ci/job_details/components/sidebar/sidebar.vue
+++ b/app/assets/javascripts/ci/job_details/components/sidebar/sidebar.vue
@@ -1,5 +1,4 @@
 <script>
-import { GlButton } from '@gitlab/ui';
 import { isEmpty } from 'lodash';
 // eslint-disable-next-line no-restricted-imports
 import { mapActions, mapGetters, mapState } from 'vuex';
@@ -24,7 +23,6 @@ export default {
   name: 'JobSidebar',
   forwardDeploymentFailureModalId,
   components: {
-    GlButton,
     ArtifactsBlock,
     CommitBlock,
     JobsContainer,
@@ -70,15 +68,6 @@ export default {
     externalLinks() {
       return filterAnnotations(this.job.annotations, 'external_link');
     },
-    jobHasPath() {
-      return Boolean(
-        this.job.erase_path ||
-          this.job.new_issue_path ||
-          this.job.terminal_path ||
-          this.job.retry_path ||
-          this.job.cancel_path,
-      );
-    },
   },
   watch: {
     job(value, oldValue) {
@@ -91,7 +80,7 @@ export default {
     },
   },
   methods: {
-    ...mapActions(['fetchJobsForStage', 'toggleSidebar']),
+    ...mapActions(['fetchJobsForStage']),
   },
 };
 </script>
@@ -99,18 +88,7 @@ export default {
   <aside class="right-sidebar build-sidebar" data-offset-top="101" data-spy="affix">
     <div class="sidebar-container">
       <div class="blocks-container gl-p-4 gl-pt-0">
-        <div
-          class="gl-py-4 gl-border-b gl-border-gray-50 gl-display-flex gl-md-display-none! gl-justify-content-end"
-        >
-          <gl-button
-            :aria-label="$options.i18n.toggleSidebar"
-            category="tertiary"
-            icon="chevron-double-lg-right"
-            @click="toggleSidebar"
-          />
-        </div>
         <sidebar-header
-          v-if="jobHasPath"
           class="gl-py-4 gl-border-b gl-border-gray-50"
           :rest-job="job"
           :job-id="job.id"
diff --git a/app/assets/javascripts/ci/job_details/components/sidebar/sidebar_header.vue b/app/assets/javascripts/ci/job_details/components/sidebar/sidebar_header.vue
index 2713b37e30e837..f757a3bcf00f84 100644
--- a/app/assets/javascripts/ci/job_details/components/sidebar/sidebar_header.vue
+++ b/app/assets/javascripts/ci/job_details/components/sidebar/sidebar_header.vue
@@ -1,5 +1,7 @@
 <script>
 import { GlButton, GlTooltipDirective } from '@gitlab/ui';
+// eslint-disable-next-line no-restricted-imports
+import { mapActions } from 'vuex';
 import { createAlert } from '~/alert';
 import { TYPENAME_COMMIT_STATUS } from '~/graphql_shared/constants';
 import { convertToGraphQLId } from '~/graphql_shared/utils';
@@ -80,70 +82,93 @@ export default {
     retryButtonCategory() {
       return this.restJob.status && this.restJob.recoverable ? 'primary' : 'secondary';
     },
+    jobHasPath() {
+      return Boolean(
+        this.restJob.erase_path ||
+          this.restJob.new_issue_path ||
+          this.restJob.terminal_path ||
+          this.restJob.retry_path ||
+          this.restJob.cancel_path,
+      );
+    },
+  },
+  methods: {
+    ...mapActions(['toggleSidebar']),
   },
 };
 </script>
 
 <template>
-  <div>
-    <div class="gl-display-flex gl-gap-3">
+  <div class="gl-py-3!">
+    <div class="gl-display-flex gl-justify-content-space-between gl-gap-3">
+      <div class="gl-display-flex gl-gap-3">
+        <template v-if="jobHasPath">
+          <gl-button
+            v-if="restJob.erase_path"
+            v-gl-tooltip.bottom
+            :title="$options.i18n.eraseLogButtonLabel"
+            :aria-label="$options.i18n.eraseLogButtonLabel"
+            :href="restJob.erase_path"
+            :data-confirm="$options.i18n.eraseLogConfirmText"
+            data-testid="job-log-erase-link"
+            data-confirm-btn-variant="danger"
+            data-method="post"
+            icon="remove"
+          />
+          <gl-button
+            v-if="restJob.new_issue_path"
+            v-gl-tooltip.bottom
+            :href="restJob.new_issue_path"
+            :title="$options.i18n.newIssue"
+            :aria-label="$options.i18n.newIssue"
+            category="secondary"
+            variant="confirm"
+            data-testid="job-new-issue"
+            icon="issue-new"
+          />
+          <gl-button
+            v-if="restJob.terminal_path"
+            v-gl-tooltip.bottom
+            :href="restJob.terminal_path"
+            :title="$options.i18n.debug"
+            :aria-label="$options.i18n.debug"
+            target="_blank"
+            icon="external-link"
+            data-testid="terminal-link"
+          />
+          <job-sidebar-retry-button
+            v-if="canShowJobRetryButton"
+            v-gl-tooltip.bottom
+            :title="buttonTitle"
+            :aria-label="buttonTitle"
+            :is-manual-job="isManualJob"
+            :category="retryButtonCategory"
+            :href="restJob.retry_path"
+            :modal-id="$options.forwardDeploymentFailureModalId"
+            variant="confirm"
+            data-testid="retry-button"
+            @updateVariablesClicked="$emit('updateVariables')"
+          />
+          <gl-button
+            v-if="restJob.cancel_path"
+            v-gl-tooltip.bottom
+            :title="$options.i18n.cancelJobButtonLabel"
+            :aria-label="$options.i18n.cancelJobButtonLabel"
+            :href="restJob.cancel_path"
+            variant="danger"
+            icon="cancel"
+            data-method="post"
+            data-testid="cancel-button"
+            rel="nofollow"
+          />
+        </template>
+      </div>
       <gl-button
-        v-if="restJob.erase_path"
-        v-gl-tooltip.bottom
-        :title="$options.i18n.eraseLogButtonLabel"
-        :aria-label="$options.i18n.eraseLogButtonLabel"
-        :href="restJob.erase_path"
-        :data-confirm="$options.i18n.eraseLogConfirmText"
-        data-testid="job-log-erase-link"
-        data-confirm-btn-variant="danger"
-        data-method="post"
-        icon="remove"
-      />
-      <gl-button
-        v-if="restJob.new_issue_path"
-        v-gl-tooltip.bottom
-        :href="restJob.new_issue_path"
-        :title="$options.i18n.newIssue"
-        :aria-label="$options.i18n.newIssue"
+        :aria-label="$options.i18n.toggleSidebar"
         category="secondary"
-        variant="confirm"
-        data-testid="job-new-issue"
-        icon="issue-new"
-      />
-      <gl-button
-        v-if="restJob.terminal_path"
-        v-gl-tooltip.bottom
-        :href="restJob.terminal_path"
-        :title="$options.i18n.debug"
-        :aria-label="$options.i18n.debug"
-        target="_blank"
-        icon="external-link"
-        data-testid="terminal-link"
-      />
-      <job-sidebar-retry-button
-        v-if="canShowJobRetryButton"
-        v-gl-tooltip.bottom
-        :title="buttonTitle"
-        :aria-label="buttonTitle"
-        :is-manual-job="isManualJob"
-        :category="retryButtonCategory"
-        :href="restJob.retry_path"
-        :modal-id="$options.forwardDeploymentFailureModalId"
-        variant="confirm"
-        data-testid="retry-button"
-        @updateVariablesClicked="$emit('updateVariables')"
-      />
-      <gl-button
-        v-if="restJob.cancel_path"
-        v-gl-tooltip.bottom
-        :title="$options.i18n.cancelJobButtonLabel"
-        :aria-label="$options.i18n.cancelJobButtonLabel"
-        :href="restJob.cancel_path"
-        variant="danger"
-        icon="cancel"
-        data-method="post"
-        data-testid="cancel-button"
-        rel="nofollow"
+        class="gl-lg-display-none"
+        icon="chevron-double-lg-right"
+        @click="toggleSidebar"
       />
     </div>
   </div>
diff --git a/app/assets/javascripts/ci/job_details/job_app.vue b/app/assets/javascripts/ci/job_details/job_app.vue
index 85a12a1dca93e9..119f8259be7eec 100644
--- a/app/assets/javascripts/ci/job_details/job_app.vue
+++ b/app/assets/javascripts/ci/job_details/job_app.vue
@@ -195,7 +195,7 @@ export default {
     },
     updateSidebar() {
       const breakpoint = bp.getBreakpointSize();
-      if (breakpoint === 'xs' || breakpoint === 'sm') {
+      if (breakpoint === 'xs' || breakpoint === 'sm' || breakpoint === 'md') {
         this.hideSidebar();
       } else if (!this.isSidebarOpen) {
         this.showSidebar();
@@ -290,11 +290,7 @@ export default {
           {{ __('This job is archived. Only the complete pipeline can be retried.') }}
         </div>
         <!-- job log -->
-        <div
-          v-if="hasJobLog && !showUpdateVariablesState"
-          class="build-log-container gl-relative"
-          :class="{ 'gl-mt-3': !job.archived }"
-        >
+        <div v-if="hasJobLog && !showUpdateVariablesState" class="build-log-container gl-relative">
           <log-top-bar
             :class="{
               'has-archived-block': job.archived,
@@ -332,18 +328,17 @@ export default {
         <!-- EO empty state -->
 
         <!-- EO Body Section -->
+
+        <sidebar
+          :class="{
+            'right-sidebar-expanded': isSidebarOpen,
+            'right-sidebar-collapsed': !isSidebarOpen,
+          }"
+          :artifact-help-url="artifactHelpUrl"
+          data-testid="job-sidebar"
+          @updateVariables="onUpdateVariables()"
+        />
       </div>
     </template>
-
-    <sidebar
-      v-if="shouldRenderContent"
-      :class="{
-        'right-sidebar-expanded': isSidebarOpen,
-        'right-sidebar-collapsed': !isSidebarOpen,
-      }"
-      :artifact-help-url="artifactHelpUrl"
-      data-testid="job-sidebar"
-      @updateVariables="onUpdateVariables()"
-    />
   </div>
 </template>
diff --git a/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue b/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue
index 3a6a655bfa6c77..ec7be3a3ffc235 100644
--- a/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue
+++ b/app/assets/javascripts/ci/pipeline_details/header/pipeline_details_header.vue
@@ -565,7 +565,7 @@ export default {
           </span>
         </div>
       </div>
-      <div class="gl-mt-5 gl-lg-mt-0">
+      <div class="gl-mt-5 gl-lg-mt-0 gl-display-flex gl-align-items-flex-start gl-gap-3">
         <gl-button
           v-if="canRetryPipeline"
           v-gl-tooltip
@@ -588,7 +588,6 @@ export default {
           :title="$options.BUTTON_TOOLTIP_CANCEL"
           :loading="isCanceling"
           :disabled="isCanceling"
-          class="gl-ml-3"
           variant="danger"
           data-testid="cancel-pipeline"
           @click="cancelPipeline()"
@@ -601,7 +600,6 @@ export default {
           v-gl-modal="$options.modal.id"
           :loading="isDeleting"
           :disabled="isDeleting"
-          class="gl-ml-3"
           variant="danger"
           category="secondary"
           data-testid="delete-pipeline"
diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss
index 764a4169da06d8..5dae64653ac5b7 100644
--- a/app/assets/stylesheets/framework/common.scss
+++ b/app/assets/stylesheets/framework/common.scss
@@ -43,7 +43,8 @@
   .right-sidebar-collapsed {
     --application-bar-right: #{$right-sidebar-collapsed-width};
 
-    &.is-merge-request {
+    &.is-merge-request,
+    &.build-sidebar {
       --application-bar-right: 0px;
     }
   }
@@ -51,6 +52,10 @@
   .right-sidebar-expanded {
     --application-bar-right: #{$right-sidebar-width};
   }
+
+  .build-sidebar {
+    --application-bar-right: 0px;
+  }
 }
 
 @include media-breakpoint-up(md) {
diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss
index a32663b17d320b..df107798a87149 100644
--- a/app/assets/stylesheets/framework/mixins.scss
+++ b/app/assets/stylesheets/framework/mixins.scss
@@ -254,7 +254,7 @@
 @mixin build-log-bar($height) {
   height: $height;
   min-height: $height;
-  background: var(--gray-50, $gray-50);
+  background: var(--white, $white);
   border: 1px solid var(--border-color, $border-color);
   color: var(--gl-text-color, $gl-text-color);
   padding: $grid-size;
diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss
index f2afa94e0003d0..9c31559cc0ea08 100644
--- a/app/assets/stylesheets/framework/sidebar.scss
+++ b/app/assets/stylesheets/framework/sidebar.scss
@@ -73,13 +73,21 @@
     }
   }
 
-  &:not(.is-merge-request) {
+  &:not(.is-merge-request):not(.build-sidebar) {
     @include media-breakpoint-up(md) {
       .content-wrapper {
         padding-right: $right-sidebar-width;
       }
     }
   }
+
+  &.build-sidebar {
+    @include media-breakpoint-up(lg) {
+      .content-wrapper {
+        padding-right: $right-sidebar-width;
+      }
+    }
+  }
 }
 
 .right-sidebar {
diff --git a/app/assets/stylesheets/page_bundles/build.scss b/app/assets/stylesheets/page_bundles/build.scss
index a06a1513f7d8e9..24bd8b0fd7f955 100644
--- a/app/assets/stylesheets/page_bundles/build.scss
+++ b/app/assets/stylesheets/page_bundles/build.scss
@@ -17,7 +17,7 @@
     @include build-log-top-bar(50px);
     z-index: 2;
     border-radius: $border-radius-default $border-radius-default 0 0;
-    box-shadow: 0 -2px 0 0 var(--white);
+    box-shadow: 0 -4px 0 0 var(--white);
 
     &.has-archived-block {
       top: calc(#{$calc-application-header-height} + 28px);
@@ -89,13 +89,20 @@
 }
 
 .right-sidebar.build-sidebar {
+  padding: 0;
+
+  @include media-breakpoint-up(lg) {
+    @include gl-border-l-0;
+  }
+
   &.right-sidebar-collapsed {
     display: none;
   }
 
   .sidebar-container {
-    padding-right: 100px;
-    height: 100%;
+    @include gl-sticky;
+    top: #{$top-bar-height - 1px};
+    max-height: calc(100vh - #{$top-bar-height - 1px});
     overflow-y: scroll;
     overflow-x: hidden;
     -webkit-overflow-scrolling: touch;
diff --git a/spec/frontend/ci/job_details/components/job_header_spec.js b/spec/frontend/ci/job_details/components/job_header_spec.js
index 39b130ef4144d1..609369316f553b 100644
--- a/spec/frontend/ci/job_details/components/job_header_spec.js
+++ b/spec/frontend/ci/job_details/components/job_header_spec.js
@@ -52,6 +52,10 @@ describe('Header CI Component', () => {
       createComponent();
     });
 
+    it('renders the correct job name', () => {
+      expect(findJobName().text()).toBe(defaultProps.name);
+    });
+
     it('should render status badge', () => {
       expect(findCiBadgeLink().exists()).toBe(true);
     });
@@ -63,10 +67,6 @@ describe('Header CI Component', () => {
     it('should render sidebar toggle button', () => {
       expect(findSidebarToggleBtn().exists()).toBe(true);
     });
-
-    it('renders the correct job name', () => {
-      expect(findJobName().text()).toBe(defaultProps.name);
-    });
   });
 
   describe('user avatar', () => {
-- 
GitLab