Merge branch '247897-properly-style-the-container-registry-metadata' into 'master'

Create and use registry/metadata_item

See merge request gitlab-org/gitlab!42202

app/assets/javascripts/packages/details/components/package_title.vue

 <script>
 import { mapState, mapGetters } from 'vuex';
-import { GlIcon, GlLink, GlSprintf, GlTooltipDirective } from '@gitlab/ui';
+import { GlIcon, GlSprintf, GlTooltipDirective } from '@gitlab/ui';
 import PackageTags from '../../shared/components/package_tags.vue';
 import { numberToHumanSize } from '~/lib/utils/number_utils';
 import timeagoMixin from '~/vue_shared/mixins/timeago';
 import TitleArea from '~/vue_shared/components/registry/title_area.vue';
+import MetadataItem from '~/vue_shared/components/registry/metadata_item.vue';
 import { __ } from '~/locale';
 
 export default {
@@ -12,9 +13,9 @@ export default {
   components: {
     TitleArea,
     GlIcon,
-    GlLink,
     GlSprintf,
     PackageTags,
+    MetadataItem,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -54,35 +55,24 @@ export default {
     </template>
 
     <template v-if="packageTypeDisplay" #metadata_type>
-      <gl-icon name="package" class="gl-text-gray-500 gl-mr-3" />
-      <span data-testid="package-type" class="gl-font-weight-bold">{{ packageTypeDisplay }}</span>
+      <metadata-item data-testid="package-type" icon="package" :text="packageTypeDisplay" />
     </template>
 
     <template #metadata_size>
-      <gl-icon name="disk" class="gl-text-gray-500 gl-mr-3" />
-      <span data-testid="package-size" class="gl-font-weight-bold">{{ totalSize }}</span>
+      <metadata-item data-testid="package-size" icon="disk" :text="totalSize" />
     </template>
 
     <template v-if="packagePipeline" #metadata_pipeline>
-      <gl-icon name="review-list" class="gl-text-gray-500 gl-mr-3" />
-      <gl-link
+      <metadata-item
         data-testid="pipeline-project"
-        :href="packagePipeline.project.web_url"
-        class="gl-font-weight-bold gl-str-truncated"
-      >
-        {{ packagePipeline.project.name }}
-      </gl-link>
+        icon="review-list"
+        :text="packagePipeline.project.name"
+        :link="packagePipeline.project.web_url"
+      />
     </template>
 
     <template v-if="packagePipeline" #metadata_ref>
-      <gl-icon name="branch" data-testid="package-ref-icon" class="gl-text-gray-500 gl-mr-3" />
-      <span
-        v-gl-tooltip
-        data-testid="package-ref"
-        class="gl-font-weight-bold gl-str-truncated mw-xs"
-        :title="packagePipeline.ref"
-        >{{ packagePipeline.ref }}</span
-      >
+      <metadata-item data-testid="package-ref" icon="branch" :text="packagePipeline.ref" />
     </template>
 
     <template v-if="hasTagsToDisplay" #metadata_tags>

app/assets/javascripts/registry/explorer/components/list_page/registry_header.vue

 <script>
-import { GlSprintf, GlLink, GlIcon } from '@gitlab/ui';
+import { GlSprintf, GlLink } from '@gitlab/ui';
 import TitleArea from '~/vue_shared/components/registry/title_area.vue';
-import { n__ } from '~/locale';
+import MetadataItem from '~/vue_shared/components/registry/metadata_item.vue';
+import { n__, sprintf } from '~/locale';
 import { approximateDuration, calculateRemainingMilliseconds } from '~/lib/utils/datetime_utility';
 
 import {
@@ -14,10 +15,10 @@ import {
 
 export default {
   components: {
-    GlIcon,
     GlSprintf,
     GlLink,
     TitleArea,
+    MetadataItem,
   },
   props: {
     expirationPolicy: {
@@ -58,11 +59,12 @@ export default {
   },
   computed: {
     imagesCountText() {
-      return n__(
+      const pluralisedString = n__(
         'ContainerRegistry|%{count} Image repository',
         'ContainerRegistry|%{count} Image repositories',
         this.imagesCount,
       );
+      return sprintf(pluralisedString, { count: this.imagesCount });
     },
     timeTillRun() {
       const difference = calculateRemainingMilliseconds(this.expirationPolicy?.next_run_at);
@@ -73,7 +75,7 @@ export default {
     },
     expirationPolicyText() {
       return this.expirationPolicyEnabled
-        ? EXPIRATION_POLICY_WILL_RUN_IN
+        ? sprintf(EXPIRATION_POLICY_WILL_RUN_IN, { time: this.timeTillRun })
         : EXPIRATION_POLICY_DISABLED_TEXT;
     },
     showExpirationPolicyTip() {
@@ -92,24 +94,21 @@ export default {
         <slot name="commands"></slot>
       </template>
       <template #metadata_count>
-        <span v-if="imagesCount" data-testid="images-count">
-          <gl-icon class="gl-mr-1" name="container-image" />
-          <gl-sprintf :message="imagesCountText">
-            <template #count>
-              {{ imagesCount }}
-            </template>
-          </gl-sprintf>
-        </span>
+        <metadata-item
+          v-if="imagesCount"
+          data-testid="images-count"
+          icon="container-image"
+          :text="imagesCountText"
+        />
       </template>
       <template #metadata_exp_policies>
-        <span v-if="!hideExpirationPolicyData" data-testid="expiration-policy">
-          <gl-icon class="gl-mr-1" name="expire" />
-          <gl-sprintf :message="expirationPolicyText">
-            <template #time>
-              {{ timeTillRun }}
-            </template>
-          </gl-sprintf>
-        </span>
+        <metadata-item
+          v-if="!hideExpirationPolicyData"
+          data-testid="expiration-policy"
+          icon="expire"
+          :text="expirationPolicyText"
+          size="xl"
+        />
       </template>
     </title-area>
 

app/assets/javascripts/vue_shared/components/registry/metadata_item.vue

+<script>
+import { GlIcon, GlLink } from '@gitlab/ui';
+import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
+
+export default {
+  name: 'MetadataItem',
+  components: {
+    GlIcon,
+    GlLink,
+    TooltipOnTruncate,
+  },
+  props: {
+    icon: {
+      type: String,
+      required: false,
+      default: null,
+    },
+    text: {
+      type: String,
+      required: true,
+    },
+    link: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    size: {
+      type: String,
+      required: false,
+      default: 's',
+      validator(value) {
+        return !value || ['xs', 's', 'm', 'l', 'xl'].includes(value);
+      },
+    },
+  },
+  computed: {
+    sizeClass() {
+      return `mw-${this.size}`;
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="gl-display-inline-flex gl-align-items-center">
+    <gl-icon v-if="icon" :name="icon" class="gl-text-gray-500 gl-mr-3" />
+    <tooltip-on-truncate v-if="link" :title="text" class="gl-text-truncate" :class="sizeClass">
+      <gl-link :href="link" class="gl-font-weight-bold">
+        {{ text }}
+      </gl-link>
+    </tooltip-on-truncate>
+    <div
+      v-else
+      data-testid="metadata-item-text"
+      class="gl-font-weight-bold gl-display-inline-flex"
+      :class="sizeClass"
+    >
+      <tooltip-on-truncate :title="text" class="gl-text-truncate">
+        {{ text }}
+      </tooltip-on-truncate>
+    </div>
+  </div>
+</template>

changelogs/unreleased/247897-properly-style-the-container-registry-metadata.yml

+---
+title: Update visual styling of container registry metadata
+merge_request: 42202
+author:
+type: changed

spec/frontend/packages/details/components/__snapshots__/package_title_spec.js.snap

@@ -45,34 +45,24 @@ exports[`PackageTitle renders with tags 1`] = `
       <div
         class="gl-display-flex gl-align-items-center gl-mr-5"
       >
-        <gl-icon-stub
-          class="gl-text-gray-500 gl-mr-3"
-          name="package"
-          size="16"
-        />
-         
-        <span
-          class="gl-font-weight-bold"
+        <metadata-item-stub
           data-testid="package-type"
-        >
-          maven
-        </span>
+          icon="package"
+          link=""
+          size="s"
+          text="maven"
+        />
       </div>
       <div
         class="gl-display-flex gl-align-items-center gl-mr-5"
       >
-        <gl-icon-stub
-          class="gl-text-gray-500 gl-mr-3"
-          name="disk"
-          size="16"
-        />
-         
-        <span
-          class="gl-font-weight-bold"
+        <metadata-item-stub
           data-testid="package-size"
-        >
-          300 bytes
-        </span>
+          icon="disk"
+          link=""
+          size="s"
+          text="300 bytes"
+        />
       </div>
       <div
         class="gl-display-flex gl-align-items-center gl-mr-5"
@@ -135,34 +125,24 @@ exports[`PackageTitle renders without tags 1`] = `
       <div
         class="gl-display-flex gl-align-items-center gl-mr-5"
       >
-        <gl-icon-stub
-          class="gl-text-gray-500 gl-mr-3"
-          name="package"
-          size="16"
-        />
-         
-        <span
-          class="gl-font-weight-bold"
+        <metadata-item-stub
           data-testid="package-type"
-        >
-          maven
-        </span>
+          icon="package"
+          link=""
+          size="s"
+          text="maven"
+        />
       </div>
       <div
         class="gl-display-flex gl-align-items-center gl-mr-5"
       >
-        <gl-icon-stub
-          class="gl-text-gray-500 gl-mr-3"
-          name="disk"
-          size="16"
-        />
-         
-        <span
-          class="gl-font-weight-bold"
+        <metadata-item-stub
           data-testid="package-size"
-        >
-          300 bytes
-        </span>
+          icon="disk"
+          link=""
+          size="s"
+          text="300 bytes"
+        />
       </div>
     </div>
   </div>

spec/frontend/packages/details/components/package_title_spec.js

@@ -52,7 +52,6 @@ describe('PackageTitle', () => {
   const packageSize = () => wrapper.find('[data-testid="package-size"]');
   const pipelineProject = () => wrapper.find('[data-testid="pipeline-project"]');
   const packageRef = () => wrapper.find('[data-testid="package-ref"]');
-  const packageRefIcon = () => wrapper.find('[data-testid="package-ref-icon"]');
   const packageTags = () => wrapper.find(PackageTags);
 
   afterEach(() => {
@@ -98,16 +97,16 @@ describe('PackageTitle', () => {
   });
 
   describe.each`
-    packageEntity   | expectedResult
+    packageEntity   | text
     ${conanPackage} | ${'conan'}
     ${mavenPackage} | ${'maven'}
     ${npmPackage}   | ${'npm'}
     ${nugetPackage} | ${'nuget'}
-  `(`package type`, ({ packageEntity, expectedResult }) => {
+  `(`package type`, ({ packageEntity, text }) => {
     beforeEach(() => createComponent({ packageEntity }));
 
-    it(`${packageEntity.package_type} should render from Vuex getters ${expectedResult}`, () => {
-      expect(packageType().text()).toBe(expectedResult);
+    it(`${packageEntity.package_type} should render from Vuex getters ${text}`, () => {
+      expect(packageType().props()).toEqual(expect.objectContaining({ text, icon: 'package' }));
     });
   });
 
@@ -115,13 +114,13 @@ describe('PackageTitle', () => {
     it('correctly calculates when there is only 1 file', async () => {
       await createComponent({ packageEntity: npmPackage, packageFiles: npmFiles });
 
-      expect(packageSize().text()).toBe('200 bytes');
+      expect(packageSize().props()).toMatchObject({ text: '200 bytes', icon: 'disk' });
     });
 
     it('correctly calulates when there are multiple files', async () => {
       await createComponent();
 
-      expect(packageSize().text()).toBe('300 bytes');
+      expect(packageSize().props('text')).toBe('300 bytes');
     });
   });
 
@@ -153,8 +152,10 @@ describe('PackageTitle', () => {
 
     it('correctly shows the package ref if there is one', async () => {
       await createComponent({ packageEntity: npmPackage });
-      expect(packageRefIcon().exists()).toBe(true);
-      expect(packageRef().text()).toBe(npmPackage.pipeline.ref);
+      expect(packageRef().props()).toMatchObject({
+        text: npmPackage.pipeline.ref,
+        icon: 'branch',
+      });
     });
   });
 
@@ -168,8 +169,11 @@ describe('PackageTitle', () => {
     it('correctly shows the pipeline project if there is one', async () => {
       await createComponent({ packageEntity: npmPackage });
 
-      expect(pipelineProject().text()).toBe(npmPackage.pipeline.project.name);
-      expect(pipelineProject().attributes('href')).toBe(npmPackage.pipeline.project.web_url);
+      expect(pipelineProject().props()).toMatchObject({
+        text: npmPackage.pipeline.project.name,
+        icon: 'review-list',
+        link: npmPackage.pipeline.project.web_url,
+      });
     });
   });
 });

spec/frontend/registry/explorer/components/list_page/registry_header_spec.js

@@ -7,7 +7,6 @@ import {
   LIST_INTRO_TEXT,
   EXPIRATION_POLICY_DISABLED_MESSAGE,
   EXPIRATION_POLICY_DISABLED_TEXT,
-  EXPIRATION_POLICY_WILL_RUN_IN,
 } from '~/registry/explorer/constants';
 
 jest.mock('~/lib/utils/datetime_utility', () => ({
@@ -68,13 +67,16 @@ describe('registry_header', () => {
         it('when there is one image', async () => {
           await mountComponent({ imagesCount: 1 });
 
-          expect(findImagesCountSubHeader().text()).toMatchInterpolatedText('1 Image repository');
+          expect(findImagesCountSubHeader().props()).toMatchObject({
+            text: '1 Image repository',
+            icon: 'container-image',
+          });
         });
 
         it('when there is more than one image', async () => {
           await mountComponent({ imagesCount: 3 });
 
-          expect(findImagesCountSubHeader().text()).toMatchInterpolatedText('3 Image repositories');
+          expect(findImagesCountSubHeader().props('text')).toBe('3 Image repositories');
         });
       });
 
@@ -88,7 +90,11 @@ describe('registry_header', () => {
 
           const text = findExpirationPolicySubHeader();
           expect(text.exists()).toBe(true);
-          expect(text.text()).toMatchInterpolatedText(EXPIRATION_POLICY_DISABLED_TEXT);
+          expect(text.props()).toMatchObject({
+            text: EXPIRATION_POLICY_DISABLED_TEXT,
+            icon: 'expire',
+            size: 'xl',
+          });
         });
 
         it('when is enabled', async () => {
@@ -100,7 +106,7 @@ describe('registry_header', () => {
 
           const text = findExpirationPolicySubHeader();
           expect(text.exists()).toBe(true);
-          expect(text.text()).toMatchInterpolatedText(EXPIRATION_POLICY_WILL_RUN_IN);
+          expect(text.props('text')).toBe('Expiration policy will run in ');
         });
         it('when the expiration policy is completely disabled', async () => {
           await mountComponent({

spec/frontend/vue_shared/components/registry/metadata_item_spec.js

+import { shallowMount } from '@vue/test-utils';
+import { GlIcon, GlLink } from '@gitlab/ui';
+import component from '~/vue_shared/components/registry/metadata_item.vue';
+import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue';
+
+describe('Metadata Item', () => {
+  let wrapper;
+  const defaultProps = {
+    text: 'foo',
+  };
+
+  const mountComponent = (propsData = defaultProps) => {
+    wrapper = shallowMount(component, {
+      propsData,
+    });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  const findIcon = () => wrapper.find(GlIcon);
+  const findLink = (w = wrapper) => w.find(GlLink);
+  const findText = () => wrapper.find('[data-testid="metadata-item-text"]');
+  const findTooltipOnTruncate = (w = wrapper) => w.find(TooltipOnTruncate);
+
+  describe.each(['xs', 's', 'm', 'l', 'xl'])('size class', size => {
+    const className = `mw-${size}`;
+
+    it(`${size} is assigned correctly to text`, () => {
+      mountComponent({ ...defaultProps, size });
+
+      expect(findText().classes()).toContain(className);
+    });
+
+    it(`${size} is assigned correctly to link`, () => {
+      mountComponent({ ...defaultProps, link: 'foo', size });
+
+      expect(findTooltipOnTruncate().classes()).toContain(className);
+    });
+  });
+
+  describe('text', () => {
+    it('display a proper text', () => {
+      mountComponent();
+
+      expect(findText().text()).toBe(defaultProps.text);
+    });
+
+    it('uses tooltip_on_truncate', () => {
+      mountComponent();
+
+      const tooltip = findTooltipOnTruncate(findText());
+      expect(tooltip.exists()).toBe(true);
+      expect(tooltip.attributes('title')).toBe(defaultProps.text);
+    });
+  });
+
+  describe('link', () => {
+    it('if a link prop is passed shows a link and hides the text', () => {
+      mountComponent({ ...defaultProps, link: 'bar' });
+
+      expect(findLink().exists()).toBe(true);
+      expect(findText().exists()).toBe(false);
+
+      expect(findLink().attributes('href')).toBe('bar');
+    });
+
+    it('uses tooltip_on_truncate', () => {
+      mountComponent({ ...defaultProps, link: 'bar' });
+
+      const tooltip = findTooltipOnTruncate();
+      expect(tooltip.exists()).toBe(true);
+      expect(tooltip.attributes('title')).toBe(defaultProps.text);
+      expect(findLink(tooltip).exists()).toBe(true);
+    });
+
+    it('hides the link and shows the test if a link prop is not passed', () => {
+      mountComponent();
+
+      expect(findText().exists()).toBe(true);
+      expect(findLink().exists()).toBe(false);
+    });
+  });
+
+  describe('icon', () => {
+    it('if a icon prop is passed shows a icon', () => {
+      mountComponent({ ...defaultProps, icon: 'pencil' });
+
+      expect(findIcon().exists()).toBe(true);
+      expect(findIcon().props('name')).toBe('pencil');
+    });
+
+    it('if a icon prop is not passed hides the icon', () => {
+      mountComponent();
+
+      expect(findIcon().exists()).toBe(false);
+    });
+  });
+});