From 421b83886ef141bd8c64e8446b8a19d039d1dbb2 Mon Sep 17 00:00:00 2001
From: Sascha Eggenberger <seggenberger@gitlab.com>
Date: Mon, 17 Jul 2023 16:22:09 +0200
Subject: [PATCH] MR pipeline widget: cleanup
Changelog: changed
---
.../components/deployment/deployment.vue | 2 +-
.../components/mr_collapsible_extension.vue | 2 +-
.../components/mr_widget_pipeline.vue | 62 +++++++++++--------
.../page_bundles/merge_requests.scss | 26 ++++----
.../user_sees_merge_widget_spec.rb | 3 +-
.../user_sees_merge_widget_spec.rb | 9 ++-
.../components/mr_widget_pipeline_spec.js | 47 ++++++++++++--
7 files changed, 102 insertions(+), 49 deletions(-)
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment.vue b/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment.vue
index 41edbc83cdbdc..52f381201379e 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment.vue
@@ -36,7 +36,7 @@ export default {
</script>
<template>
- <div class="deploy-heading">
+ <div class="deploy-heading gl-px-5">
<div class="ci-widget media">
<div class="media-body">
<div class="deploy-body">
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_collapsible_extension.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_collapsible_extension.vue
index e435dc5650324..b7017cebda3d2 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_collapsible_extension.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_collapsible_extension.vue
@@ -48,7 +48,7 @@ export default {
<div class="d-flex align-items-center pl-3 gl-py-3">
<div v-if="hasError" class="ci-widget media">
<div class="media-body">
- <span class="gl-font-sm mr-widget-margin-left gl-line-height-24 js-error-state">
+ <span class="gl-font-sm gl-ml-7 gl-line-height-24 js-error-state">
{{ title }}
</span>
</div>
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
index 4e16b92fc05ff..b7e658f677c9f 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
@@ -194,7 +194,7 @@ export default {
</p>
</template>
<template v-else-if="hasPipeline">
- <a :href="status.details_path" class="gl-align-self-center gl-mr-3">
+ <a :href="status.details_path" class="gl-align-self-start gl-mt-2 gl-mr-3">
<ci-icon :status="status" :size="24" class="gl-display-flex" />
</a>
<div class="ci-widget-container d-flex">
@@ -203,16 +203,41 @@ export default {
<div
data-testid="pipeline-info-container"
data-qa-selector="merge_request_pipeline_info_content"
+ class="gl-display-flex gl-flex-wrap gl-align-items-center gl-justify-content-space-between"
>
- {{ pipeline.details.event_type_name }}
- <gl-link
- :href="pipeline.path"
- class="pipeline-id"
- data-testid="pipeline-id"
- data-qa-selector="pipeline_link"
- >#{{ pipeline.id }}</gl-link
+ <p
+ class="mr-pipeline-title gl-m-0! gl-mr-3! gl-font-weight-bold gl-line-height-32 gl-text-gray-900"
>
- {{ pipeline.details.status.label }}
+ {{ pipeline.details.event_type_name }}
+ <gl-link
+ :href="pipeline.path"
+ class="pipeline-id"
+ data-testid="pipeline-id"
+ data-qa-selector="pipeline_link"
+ >#{{ pipeline.id }}</gl-link
+ >
+ {{ pipeline.details.status.label }}
+ </p>
+ <div
+ class="gl-align-items-center gl-display-inline-flex gl-flex-grow-1 gl-justify-content-space-between"
+ >
+ <pipeline-mini-graph
+ v-if="pipeline.details.stages"
+ :downstream-pipelines="downstreamPipelines"
+ :is-merge-train="isMergeTrain"
+ :pipeline-path="pipeline.path"
+ :stages="pipeline.details.stages"
+ :upstream-pipeline="pipeline.triggered_by"
+ />
+ <pipeline-artifacts
+ :pipeline-id="pipeline.id"
+ :artifacts="artifacts"
+ class="gl-ml-3"
+ />
+ </div>
+ </div>
+ <p data-testid="pipeline-details-container" class="gl-font-sm gl-text-gray-500 gl-m-0">
+ {{ pipeline.details.event_type_name }} {{ pipeline.details.status.label }}
<template v-if="hasCommitInfo">
{{ s__('Pipeline|for') }}
<gl-link
@@ -228,7 +253,7 @@ export default {
v-safe-html="sourceBranchLink"
:title="sourceBranch"
truncate-target="child"
- class="label-branch label-truncate gl-font-weight-normal"
+ class="label-branch label-truncate gl-font-weight-normal gl-vertical-align-text-bottom"
/>
</template>
<template v-if="finishedAt">
@@ -238,8 +263,8 @@ export default {
data-testid="finished-at"
/>
</template>
- </div>
- <div v-if="pipeline.coverage" class="coverage" data-testid="pipeline-coverage">
+ </p>
+ <div v-if="pipeline.coverage" class="coverage gl-mt-1" data-testid="pipeline-coverage">
{{ s__('Pipeline|Test coverage') }} {{ pipeline.coverage }}%
<span
v-if="pipelineCoverageDelta"
@@ -275,19 +300,6 @@ export default {
</div>
</div>
</div>
- <div>
- <span class="gl-align-items-center gl-display-inline-flex">
- <pipeline-mini-graph
- v-if="pipeline.details.stages"
- :downstream-pipelines="downstreamPipelines"
- :is-merge-train="isMergeTrain"
- :pipeline-path="pipeline.path"
- :stages="pipeline.details.stages"
- :upstream-pipeline="pipeline.triggered_by"
- />
- <pipeline-artifacts :pipeline-id="pipeline.id" :artifacts="artifacts" class="gl-ml-3" />
- </span>
- </div>
</div>
</template>
</div>
diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss
index 5e20588dd7083..c22626c5c5cb2 100644
--- a/app/assets/stylesheets/page_bundles/merge_requests.scss
+++ b/app/assets/stylesheets/page_bundles/merge_requests.scss
@@ -1,6 +1,5 @@
@import 'mixins_and_variables_and_functions';
-$mr-widget-margin-left: 40px;
$mr-widget-min-height: 69px;
$tabs-holder-z-index: 250;
@@ -373,12 +372,13 @@ $tabs-holder-z-index: 250;
white-space: nowrap;
}
- @include media-breakpoint-down(md) {
+ /* stylelint-disable scss/at-rule-no-unknown */
+ @container mr-widget-extension (max-width: 600px) {
flex-direction: column;
align-items: flex-start;
.deployment-info {
- margin-bottom: $gl-padding;
+ margin-bottom: $gl-padding-8;
}
}
@@ -413,10 +413,7 @@ $tabs-holder-z-index: 250;
.deploy-heading,
.merge-train-position-indicator {
- padding: $gl-padding-8;
- @include media-breakpoint-up(md) {
- padding: $gl-padding-8 $gl-padding;
- }
+ padding: $gl-padding-8 $gl-padding;
.media-body {
min-width: 0;
@@ -643,6 +640,13 @@ $tabs-holder-z-index: 250;
text-transform: capitalize;
}
+ .mr-pipeline-title {
+ // NOTE: CSS Hack to make the force the pipeline
+ // to the end of the line or to force it to a
+ // new line if there is not enough space.
+ flex-grow: 999;
+ }
+
.label-branch {
@include gl-font-monospace;
font-size: 95%;
@@ -659,7 +663,7 @@ $tabs-holder-z-index: 250;
> span {
display: inline-block;
max-width: 12.5em;
- margin-bottom: -6px;
+ margin-bottom: -5px;
white-space: nowrap;
text-overflow: ellipsis;
overflow: hidden;
@@ -830,6 +834,8 @@ $tabs-holder-z-index: 250;
.mr-widget-extension {
border-top: 1px solid var(--border-color, $border-color);
background-color: var(--gray-10, $gray-10);
+ container-name: mr-widget-extension;
+ container-type: inline-size;
&.clickable:hover {
background-color: var(--gray-50, $gray-50);
@@ -881,10 +887,6 @@ $tabs-holder-z-index: 250;
padding-right: $gl-padding;
}
-.mr-widget-margin-left {
- margin-left: $mr-widget-margin-left;
-}
-
.mr-widget-section {
.code-text {
flex: 1;
diff --git a/ee/spec/features/merge_request/user_sees_merge_widget_spec.rb b/ee/spec/features/merge_request/user_sees_merge_widget_spec.rb
index c550e79ae9d71..5d94b738c02a8 100644
--- a/ee/spec/features/merge_request/user_sees_merge_widget_spec.rb
+++ b/ee/spec/features/merge_request/user_sees_merge_widget_spec.rb
@@ -60,7 +60,8 @@
visit project_merge_request_path(project, merge_request)
within '.ci-widget-content' do
- expect(page).to have_content("Merge train pipeline ##{pipeline.id} pending for #{pipeline.short_sha}")
+ expect(page).to have_content("Merge train pipeline ##{pipeline.id} pending")
+ expect(page).to have_content("Merge train pipeline pending for #{pipeline.short_sha}")
end
end
end
diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb
index 3cac24838a3ca..75df93d1a6c93 100644
--- a/spec/features/merge_request/user_sees_merge_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb
@@ -197,7 +197,8 @@ def click_expand_button
it 'shows head pipeline information' do
within '.ci-widget-content' do
- expect(page).to have_content("Pipeline ##{pipeline.id} pending " \
+ expect(page).to have_content("Pipeline ##{pipeline.id} pending")
+ expect(page).to have_content("Pipeline pending " \
"for #{pipeline.short_sha} " \
"on #{pipeline.ref}")
end
@@ -227,7 +228,8 @@ def click_expand_button
shared_examples 'pipeline widget' do
it 'shows head pipeline information', :sidekiq_might_not_need_inline do
within '.ci-widget-content' do
- expect(page).to have_content("Merge request pipeline ##{pipeline.id} pending for #{pipeline.short_sha}")
+ expect(page).to have_content("Merge request pipeline ##{pipeline.id} pending")
+ expect(page).to have_content("Merge request pipeline pending for #{pipeline.short_sha}")
end
end
end
@@ -266,7 +268,8 @@ def click_expand_button
shared_examples 'pipeline widget' do
it 'shows head pipeline information', :sidekiq_might_not_need_inline do
within '.ci-widget-content' do
- expect(page).to have_content("Merged result pipeline ##{pipeline.id} pending for #{pipeline.short_sha}")
+ expect(page).to have_content("Merged result pipeline ##{pipeline.id} pending")
+ expect(page).to have_content("Merged result pipeline pending for #{pipeline.short_sha}")
end
end
end
diff --git a/spec/frontend/vue_merge_request_widget/components/mr_widget_pipeline_spec.js b/spec/frontend/vue_merge_request_widget/components/mr_widget_pipeline_spec.js
index 820e486c13ffb..98534624b326b 100644
--- a/spec/frontend/vue_merge_request_widget/components/mr_widget_pipeline_spec.js
+++ b/spec/frontend/vue_merge_request_widget/components/mr_widget_pipeline_spec.js
@@ -28,6 +28,7 @@ describe('MRWidgetPipeline', () => {
const findCIErrorMessage = () => wrapper.findByTestId('ci-error-message');
const findPipelineID = () => wrapper.findByTestId('pipeline-id');
const findPipelineInfoContainer = () => wrapper.findByTestId('pipeline-info-container');
+ const findPipelineDetailsContainer = () => wrapper.findByTestId('pipeline-details-container');
const findCommitLink = () => wrapper.findByTestId('commit-link');
const findPipelineFinishedAt = () => wrapper.findByTestId('finished-at');
const findPipelineCoverage = () => wrapper.findByTestId('pipeline-coverage');
@@ -238,43 +239,77 @@ describe('MRWidgetPipeline', () => {
};
describe('for a branch pipeline', () => {
- it('renders a pipeline widget that reads "Pipeline <ID> <status> for <SHA> on <branch>"', () => {
+ it('renders a pipeline widget that reads "Pipeline <ID> <status>"', () => {
pipeline.ref.branch = true;
factory();
- const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id} on ${mockData.source_branch_link}`;
+ const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label}`;
const actual = trimText(findPipelineInfoContainer().text());
expect(actual).toBe(expected);
});
+
+ it('renders a pipeline widget that reads "Pipeline <status> for <SHA> on <branch>"', () => {
+ pipeline.ref.branch = true;
+
+ factory();
+
+ const expected = `Pipeline ${pipeline.details.status.label} for ${pipeline.commit.short_id} on ${mockData.source_branch_link}`;
+ const actual = trimText(findPipelineDetailsContainer().text());
+
+ expect(actual).toBe(expected);
+ });
});
describe('for a tag pipeline', () => {
- it('renders a pipeline widget that reads "Pipeline <ID> <status> for <SHA> on <branch>"', () => {
+ it('renders a pipeline widget that reads "Pipeline <ID> <status>"', () => {
pipeline.ref.tag = true;
factory();
- const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
+ const expected = `Pipeline #${pipeline.id} ${pipeline.details.status.label}`;
const actual = trimText(findPipelineInfoContainer().text());
expect(actual).toBe(expected);
});
+
+ it('renders a pipeline widget that reads "Pipeline <status> for <SHA> on <branch>"', () => {
+ pipeline.ref.tag = true;
+
+ factory();
+
+ const expected = `Pipeline ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
+ const actual = trimText(findPipelineDetailsContainer().text());
+
+ expect(actual).toBe(expected);
+ });
});
describe('for a detached merge request pipeline', () => {
- it('renders a pipeline widget that reads "Merge request pipeline <ID> <status> for <SHA>"', () => {
+ it('renders a pipeline widget that reads "Merge request pipeline <ID> <status>"', () => {
pipeline.details.event_type_name = 'Merge request pipeline';
pipeline.merge_request_event_type = 'detached';
factory();
- const expected = `Merge request pipeline #${pipeline.id} ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
+ const expected = `Merge request pipeline #${pipeline.id} ${pipeline.details.status.label}`;
const actual = trimText(findPipelineInfoContainer().text());
expect(actual).toBe(expected);
});
+
+ it('renders a pipeline widget that reads "Merge request pipeline <status> for <SHA>"', () => {
+ pipeline.details.event_type_name = 'Merge request pipeline';
+ pipeline.merge_request_event_type = 'detached';
+
+ factory();
+
+ const expected = `Merge request pipeline ${pipeline.details.status.label} for ${pipeline.commit.short_id}`;
+ const actual = trimText(findPipelineDetailsContainer().text());
+
+ expect(actual).toBe(expected);
+ });
});
});
});
--
GitLab