Redesign of MR header sections (CE Port)

- https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/8593 - Create mr-widget-icon to encapsulate circle icons - Decouple workflow L&F from mr-widget-heading - Decouple container L&F from mr-widget-heading
2018-11-29 16:57:30 -06:00 · 2018-11-29 16:57:30 -06:00 · 75d94c4cd0
commit 75d94c4cd0
parent 77c2677735
15 changed files with 388 additions and 118 deletions
--- a/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/deployment.vue
@ -112,7 +112,7 @@ export default {
 </script>

 <template>
-  <div class="mr-widget-heading deploy-heading append-bottom-default">
+  <div class="deploy-heading">
    <div class="ci-widget media">
      <div class="media-body">
        <div class="deploy-body">
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_container.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_container.vue
@ -0,0 +1,6 @@
+<template>
+  <div class="mr-widget-heading">
+    <div class="mr-widget-content"><slot name="default"></slot></div>
+    <slot name="footer"></slot>
+  </div>
+</template>
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue
@ -6,6 +6,7 @@ import Icon from '~/vue_shared/components/icon.vue';
 import clipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import tooltip from '~/vue_shared/directives/tooltip';
 import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
+import MrWidgetIcon from './mr_widget_icon.vue';

 export default {
  name: 'MRWidgetHeader',
@ -13,6 +14,7 @@ export default {
    Icon,
    clipboardButton,
    TooltipOnTruncate,
+    MrWidgetIcon,
  },
  directives: {
    tooltip,
@ -76,7 +78,7 @@ export default {
 </script>
 <template>
  <div class="mr-source-target append-bottom-default">
-    <div class="git-merge-icon-container append-right-default"><icon name="git-merge" /></div>
+    <mr-widget-icon name="git-merge" />
    <div class="git-merge-container d-flex">
      <div class="normal">
        <strong>
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_icon.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_icon.vue
@ -0,0 +1,17 @@
+<script>
+import Icon from '~/vue_shared/components/icon.vue';
+
+export default {
+  components: { Icon },
+  props: {
+    name: {
+      type: String,
+      required: true,
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="circle-icon-container append-right-default"><icon :name="name" /></div>
+</template>
--- 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
@ -79,67 +79,65 @@ export default {
 </script>

 <template>
-  <div v-if="hasPipeline || hasCIError" class="mr-widget-heading append-bottom-default">
-    <div class="ci-widget media">
-      <template v-if="hasCIError">
-        <div
-          class="add-border ci-status-icon ci-status-icon-failed ci-error
-          js-ci-error append-right-default"
-        >
-          <icon :size="32" name="status_failed_borderless" />
-        </div>
-        <div class="media-body" v-html="errorText"></div>
-      </template>
-      <template v-else-if="hasPipeline">
-        <a :href="status.details_path" class="align-self-start append-right-default">
-          <ci-icon :status="status" :size="32" :borderless="true" class="add-border" />
-        </a>
-        <div class="ci-widget-container d-flex">
-          <div class="ci-widget-content">
-            <div class="media-body">
-              <div class="font-weight-bold">
-                Pipeline
-                <a :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number"
-                  >#{{ pipeline.id }}</a
+  <div v-if="hasPipeline || hasCIError" class="ci-widget media">
+    <template v-if="hasCIError">
+      <div
+        class="add-border ci-status-icon ci-status-icon-failed ci-error
+        js-ci-error append-right-default"
+      >
+        <icon :size="32" name="status_failed_borderless" />
+      </div>
+      <div class="media-body" v-html="errorText"></div>
+    </template>
+    <template v-else-if="hasPipeline">
+      <a :href="status.details_path" class="align-self-start append-right-default">
+        <ci-icon :status="status" :size="32" :borderless="true" class="add-border" />
+      </a>
+      <div class="ci-widget-container d-flex">
+        <div class="ci-widget-content">
+          <div class="media-body">
+            <div class="font-weight-bold">
+              Pipeline
+              <a :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number"
+                >#{{ pipeline.id }}</a
+              >
+
+              {{ pipeline.details.status.label }}
+
+              <template v-if="hasCommitInfo">
+                for
+                <a
+                  :href="pipeline.commit.commit_path"
+                  class="commit-sha js-commit-link font-weight-normal"
                >
-
-                {{ pipeline.details.status.label }}
-
-                <template v-if="hasCommitInfo">
-                  for
-                  <a
-                    :href="pipeline.commit.commit_path"
-                    class="commit-sha js-commit-link font-weight-normal"
-                  >
-                    {{ pipeline.commit.short_id }}</a
-                  >
-                  on
-                  <tooltip-on-truncate
-                    :title="sourceBranch"
-                    truncate-target="child"
-                    class="label-branch label-truncate"
-                    v-html="sourceBranchLink"
-                  />
-                </template>
-              </div>
-              <div v-if="pipeline.coverage" class="coverage">Coverage {{ pipeline.coverage }}%</div>
+                  {{ pipeline.commit.short_id }}</a
+                >
+                on
+                <tooltip-on-truncate
+                  :title="sourceBranch"
+                  truncate-target="child"
+                  class="label-branch label-truncate"
+                  v-html="sourceBranchLink"
+                />
+              </template>
            </div>
-          </div>
-          <div>
-            <span class="mr-widget-pipeline-graph">
-              <span v-if="hasStages" class="stage-cell">
-                <div
-                  v-for="(stage, i) in pipeline.details.stages"
-                  :key="i"
-                  class="stage-container dropdown js-mini-pipeline-graph mr-widget-pipeline-stages"
-                >
-                  <pipeline-stage :stage="stage" />
-                </div>
-              </span>
-            </span>
+            <div v-if="pipeline.coverage" class="coverage">Coverage {{ pipeline.coverage }}%</div>
          </div>
        </div>
-      </template>
-    </div>
+        <div>
+          <span class="mr-widget-pipeline-graph">
+            <span v-if="hasStages" class="stage-cell">
+              <div
+                v-for="(stage, i) in pipeline.details.stages"
+                :key="i"
+                class="stage-container dropdown js-mini-pipeline-graph mr-widget-pipeline-stages"
+              >
+                <pipeline-stage :stage="stage" />
+              </div>
+            </span>
+          </span>
+        </div>
+      </div>
+    </template>
  </div>
 </template>
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline_container.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline_container.vue
@ -0,0 +1,74 @@
+<script>
+import Deployment from './deployment.vue';
+import MrWidgetContainer from './mr_widget_container.vue';
+import MrWidgetPipeline from './mr_widget_pipeline.vue';
+
+/**
+ * Renders the pipeline and related deployments from the store.
+ *
+ * | Props         | Description
+ * |---------------|-------------
+ * | `mr`          | This is the mr_widget store
+ * | `isPostMerge` | If true, show the "post merge" pipeline and deployments
+ */
+export default {
+  name: 'MrWidgetPipelineContainer',
+  components: {
+    Deployment,
+    MrWidgetContainer,
+    MrWidgetPipeline,
+  },
+  props: {
+    mr: {
+      type: Object,
+      required: true,
+    },
+    isPostMerge: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    pipeline() {
+      return this.isPostMerge ? this.mr.mergePipeline : this.mr.pipeline;
+    },
+    branch() {
+      return this.isPostMerge ? this.mr.targetBranch : this.mr.sourceBranch;
+    },
+    branchLink() {
+      return this.isPostMerge ? this.mr.targetBranch : this.mr.sourceBranchLink;
+    },
+    deployments() {
+      return this.isPostMerge ? this.mr.postMergeDeployments : this.mr.deployments;
+    },
+    deploymentClass() {
+      return this.isPostMerge ? 'js-post-deployment' : 'js-pre-deployment';
+    },
+    hasDeploymentMetrics() {
+      return this.isPostMerge;
+    },
+  },
+};
+</script>
+<template>
+  <mr-widget-container>
+    <mr-widget-pipeline
+      :pipeline="pipeline"
+      :ci-status="mr.ciStatus"
+      :has-ci="mr.hasCI"
+      :source-branch="branch"
+      :source-branch-link="branchLink"
+      :troubleshooting-docs-path="mr.troubleshootingDocsPath"
+    />
+    <div v-if="deployments.length" slot="footer" class="mr-widget-extension">
+      <deployment
+        v-for="deployment in deployments"
+        :key="deployment.id"
+        :class="deploymentClass"
+        :deployment="deployment"
+        :show-metrics="hasDeploymentMetrics"
+      />
+    </div>
+  </mr-widget-container>
+</template>
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
@ -6,7 +6,7 @@ import SmartInterval from '~/smart_interval';
 import createFlash from '../flash';
 import WidgetHeader from './components/mr_widget_header.vue';
 import WidgetMergeHelp from './components/mr_widget_merge_help.vue';
-import WidgetPipeline from './components/mr_widget_pipeline.vue';
+import MrWidgetPipelineContainer from './components/mr_widget_pipeline_container.vue';
 import Deployment from './components/deployment.vue';
 import WidgetRelatedLinks from './components/mr_widget_related_links.vue';
 import MergedState from './components/states/mr_widget_merged.vue';
@ -44,7 +44,7 @@ export default {
  components: {
    'mr-widget-header': WidgetHeader,
    'mr-widget-merge-help': WidgetMergeHelp,
-    'mr-widget-pipeline': WidgetPipeline,
+    MrWidgetPipelineContainer,
    Deployment,
    'mr-widget-related-links': WidgetRelatedLinks,
    'mr-widget-merged': MergedState,
@ -296,23 +296,12 @@ export default {
 <template>
  <div class="mr-state-widget prepend-top-default">
    <mr-widget-header :mr="mr" />
-    <mr-widget-pipeline
+    <mr-widget-pipeline-container
      v-if="shouldRenderPipelines"
-      :pipeline="mr.pipeline"
-      :ci-status="mr.ciStatus"
-      :has-ci="mr.hasCI"
-      :source-branch="mr.sourceBranch"
-      :source-branch-link="mr.sourceBranchLink"
-      :troubleshooting-docs-path="mr.troubleshootingDocsPath"
+      class="mr-widget-workflow"
+      :mr="mr"
    />
-    <deployment
-      v-for="deployment in mr.deployments"
-      :key="`pre-merge-deploy-${deployment.id}`"
-      class="js-pre-merge-deploy"
-      :deployment="deployment"
-      :show-metrics="false"
-    />
-    <div class="mr-section-container">
+    <div class="mr-section-container mr-widget-workflow">
      <grouped-test-reports-app
        v-if="mr.testResultsPath"
        class="js-reports-container"
@ -336,24 +325,11 @@ export default {
      </div>
      <div v-if="shouldRenderMergeHelp" class="mr-widget-footer"><mr-widget-merge-help /></div>
    </div>
-
-    <template v-if="shouldRenderMergedPipeline">
-      <mr-widget-pipeline
-        class="js-post-merge-pipeline prepend-top-default"
-        :pipeline="mr.mergePipeline"
-        :ci-status="mr.ciStatus"
-        :has-ci="mr.hasCI"
-        :source-branch="mr.targetBranch"
-        :source-branch-link="mr.targetBranch"
-        :troubleshooting-docs-path="mr.troubleshootingDocsPath"
-      />
-      <deployment
-        v-for="postMergeDeployment in mr.postMergeDeployments"
-        :key="`post-merge-deploy-${postMergeDeployment.id}`"
-        :deployment="postMergeDeployment"
-        :show-metrics="true"
-        class="js-post-deployment"
-      />
-    </template>
+    <mr-widget-pipeline-container
+      v-if="shouldRenderMergedPipeline"
+      class="js-post-merge-pipeline mr-widget-workflow"
+      :mr="mr"
+      :is-post-merge="true"
+    />
  </div>
 </template>
--- a/app/assets/stylesheets/framework/icons.scss
+++ b/app/assets/stylesheets/framework/icons.scss
@ -80,3 +80,15 @@
 .user-avatar-link {
  text-decoration: none;
 }
+
+.circle-icon-container {
+  $border-size: 1px;
+
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  border: $border-size solid $theme-gray-400;
+  border-radius: 50%;
+  padding: $gl-padding-8 - $border-size;
+  color: $theme-gray-700;
+}
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@ -50,9 +50,19 @@
 .mr-widget-heading {
  position: relative;
  border: 1px solid $border-color;
-  border-radius: 4px;
+  border-radius: $border-radius-default;
+}

-  &:not(.deploy-heading)::before {
+.mr-widget-extension {
+  border-top: 1px solid $border-color;
+  background-color: $gray-light;
+}
+
+.mr-widget-workflow {
+  margin-top: $gl-padding;
+  position: relative;
+
+  &::before {
    content: '';
    border-left: 1px solid $theme-gray-200;
    position: absolute;
@ -68,8 +78,8 @@
  border-top: 0;
 }

-.mr-widget-heading,
 .mr-widget-section,
+.mr-widget-content,
 .mr-widget-footer {
  padding: $gl-padding;
 }
@ -560,19 +570,6 @@
    color: $gl-text-color;
  }

-  .git-merge-icon-container {
-    border: 1px solid $theme-gray-400;
-    border-radius: 50%;
-    height: 32px;
-    width: 32px;
-    color: $theme-gray-700;
-    line-height: 28px;
-
-    .ic-git-merge {
-      vertical-align: middle;
-      width: 31px;
-    }
-  }

  .git-merge-container {
    justify-content: space-between;
@ -854,11 +851,6 @@
 }

 .deploy-heading {
-  margin-top: -19px;
-  border-top-left-radius: 0;
-  border-top-right-radius: 0;
-  background-color: $gray-light;
-
  @include media-breakpoint-up(md) {
    padding: $gl-padding-8 $gl-padding;
  }
@ -868,6 +860,10 @@
    font-size: 12px;
    margin-left: 48px;
  }
+
+  &:not(:last-child) {
+    border-bottom: 1px solid $border-color;
+  }
 }

 .deploy-body {
--- a/changelogs/unreleased/1979-redesign-mr-widget-approvals-ce.yml
+++ b/changelogs/unreleased/1979-redesign-mr-widget-approvals-ce.yml
@ -0,0 +1,5 @@
+---
+title: Redesign of MR header sections (CE)
+merge_request: 23465
+author:
+type: changed
--- a/spec/features/merge_request/user_sees_merge_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb
@ -60,7 +60,7 @@ describe 'Merge request > User sees merge widget', :js do
    it 'shows environments link' do
      wait_for_requests

-      page.within('.js-pre-merge-deploy') do
+      page.within('.js-pre-deployment') do
        expect(page).to have_content("Deployed to #{environment.name}")
        expect(find('.js-deploy-url')[:href]).to include(environment.formatted_external_url)
      end
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_container_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_container_spec.js
@ -0,0 +1,51 @@
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import MrWidgetContainer from '~/vue_merge_request_widget/components/mr_widget_container.vue';
+
+const BODY_HTML = '<div class="test-body">Hello World</div>';
+const FOOTER_HTML = '<div class="test-footer">Goodbye!</div>';
+
+describe('MrWidgetContainer', () => {
+  let wrapper;
+
+  const factory = (options = {}) => {
+    const localVue = createLocalVue();
+
+    wrapper = shallowMount(localVue.extend(MrWidgetContainer), {
+      localVue,
+      ...options,
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('has layout', () => {
+    factory();
+
+    expect(wrapper.is('.mr-widget-heading')).toBe(true);
+    expect(wrapper.contains('.mr-widget-content')).toBe(true);
+  });
+
+  it('accepts default slot', () => {
+    factory({
+      slots: {
+        default: BODY_HTML,
+      },
+    });
+
+    expect(wrapper.contains('.mr-widget-content .test-body')).toBe(true);
+  });
+
+  it('accepts footer slot', () => {
+    factory({
+      slots: {
+        default: BODY_HTML,
+        footer: FOOTER_HTML,
+      },
+    });
+
+    expect(wrapper.contains('.mr-widget-content .test-body')).toBe(true);
+    expect(wrapper.contains('.test-footer')).toBe(true);
+  });
+});
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_icon_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_icon_spec.js
@ -0,0 +1,30 @@
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import MrWidgetIcon from '~/vue_merge_request_widget/components/mr_widget_icon.vue';
+import Icon from '~/vue_shared/components/icon.vue';
+
+const TEST_ICON = 'commit';
+
+describe('MrWidgetIcon', () => {
+  let wrapper;
+
+  beforeEach(() => {
+    const localVue = createLocalVue();
+
+    wrapper = shallowMount(localVue.extend(MrWidgetIcon), {
+      propsData: {
+        name: TEST_ICON,
+      },
+      sync: false,
+      localVue,
+    });
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('renders icon and container', () => {
+    expect(wrapper.is('.circle-icon-container')).toBe(true);
+    expect(wrapper.find(Icon).props('name')).toEqual(TEST_ICON);
+  });
+});
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_container_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_container_spec.js
@ -0,0 +1,90 @@
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import MrWidgetPipelineContainer from '~/vue_merge_request_widget/components/mr_widget_pipeline_container.vue';
+import MrWidgetPipeline from '~/vue_merge_request_widget/components/mr_widget_pipeline.vue';
+import { mockStore } from '../mock_data';
+
+describe('MrWidgetPipelineContainer', () => {
+  let wrapper;
+
+  const factory = (props = {}) => {
+    const localVue = createLocalVue();
+
+    wrapper = shallowMount(localVue.extend(MrWidgetPipelineContainer), {
+      propsData: {
+        mr: Object.assign({}, mockStore),
+        ...props,
+      },
+      localVue,
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('when pre merge', () => {
+    beforeEach(() => {
+      factory();
+    });
+
+    it('renders pipeline', () => {
+      expect(wrapper.find(MrWidgetPipeline).exists()).toBe(true);
+      expect(wrapper.find(MrWidgetPipeline).props()).toEqual(
+        jasmine.objectContaining({
+          pipeline: mockStore.pipeline,
+          ciStatus: mockStore.ciStatus,
+          hasCi: mockStore.hasCI,
+          sourceBranch: mockStore.sourceBranch,
+          sourceBranchLink: mockStore.sourceBranchLink,
+        }),
+      );
+    });
+
+    it('renders deployments', () => {
+      const expectedProps = mockStore.deployments.map(dep =>
+        jasmine.objectContaining({
+          deployment: dep,
+          showMetrics: false,
+        }),
+      );
+
+      const deployments = wrapper.findAll('.mr-widget-extension .js-pre-deployment');
+
+      expect(deployments.wrappers.map(x => x.props())).toEqual(expectedProps);
+    });
+  });
+
+  describe('when post merge', () => {
+    beforeEach(() => {
+      factory({
+        isPostMerge: true,
+      });
+    });
+
+    it('renders pipeline', () => {
+      expect(wrapper.find(MrWidgetPipeline).exists()).toBe(true);
+      expect(wrapper.find(MrWidgetPipeline).props()).toEqual(
+        jasmine.objectContaining({
+          pipeline: mockStore.mergePipeline,
+          ciStatus: mockStore.ciStatus,
+          hasCi: mockStore.hasCI,
+          sourceBranch: mockStore.targetBranch,
+          sourceBranchLink: mockStore.targetBranch,
+        }),
+      );
+    });
+
+    it('renders deployments', () => {
+      const expectedProps = mockStore.postMergeDeployments.map(dep =>
+        jasmine.objectContaining({
+          deployment: dep,
+          showMetrics: true,
+        }),
+      );
+
+      const deployments = wrapper.findAll('.mr-widget-extension .js-post-deployment');
+
+      expect(deployments.wrappers.map(x => x.props())).toEqual(expectedProps);
+    });
+  });
+});
--- a/spec/javascripts/vue_mr_widget/mock_data.js
+++ b/spec/javascripts/vue_mr_widget/mock_data.js
@ -222,3 +222,16 @@ export default {
    'http://localhost:3000/root/acets-app/commit/53027d060246c8f47e4a9310fb332aa52f221775',
  troubleshooting_docs_path: 'help',
 };
+
+export const mockStore = {
+  pipeline: { id: 0 },
+  mergePipeline: { id: 1 },
+  targetBranch: 'target-branch',
+  sourceBranch: 'source-branch',
+  sourceBranchLink: 'source-branch-link',
+  deployments: [{ id: 0, name: 'bogus' }, { id: 1, name: 'bogus-docs' }],
+  postMergeDeployments: [{ id: 0, name: 'prod' }, { id: 1, name: 'prod-docs' }],
+  troubleshootingDocsPath: 'troubleshooting-docs-path',
+  ciStatus: 'ci-status',
+  hasCI: true,
+};