Merge branch '25374-svg-as-prop' into 'master'

Resolve "Provide SVG as a prop instead of hiding and copy them in environments table"

## What does this MR do?
- Provides SVG as a prop instead of manually manipulate the DOM to show it 
- Uniforms all props names in environments related components, 3 formats were being used and it was error prone
- Adds tests for the new SVG props.

## Why was this MR needed?
Technical debt.

## Does this MR meet the acceptance criteria?

- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
  - [x] Added for this feature/bug
  - [ ] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?
Closes #25374

See merge request !7992

app/assets/javascripts/environments/components/environment.js.es6

@@ -74,6 +74,8 @@
         projectStoppedEnvironmentsPath: environmentsData.projectStoppedEnvironmentsPath,
         newEnvironmentPath: environmentsData.newEnvironmentPath,
         helpPagePath: environmentsData.helpPagePath,
+        commitIconSvg: environmentsData.commitIconSvg,
+        playIconSvg: environmentsData.playIconSvg,
       };
     },
 
@@ -227,7 +229,9 @@
                     :model="model"
                     :toggleRow="toggleRow.bind(model)"
                     :can-create-deployment="canCreateDeploymentParsed"
-                    :can-read-environment="canReadEnvironmentParsed"></tr>
+                    :can-read-environment="canReadEnvironmentParsed"
+                    :play-icon-svg="playIconSvg"
+                    :commit-icon-svg="commitIconSvg"></tr>
 
                   <tr v-if="model.isOpen && model.children && model.children.length > 0"
                     is="environment-item"
@@ -235,7 +239,9 @@
                     :model="children"
                     :toggleRow="toggleRow.bind(children)"
                     :can-create-deployment="canCreateDeploymentParsed"
-                    :can-read-environment="canReadEnvironmentParsed">
+                    :can-read-environment="canReadEnvironmentParsed"
+                    :play-icon-svg="playIconSvg"
+                    :commit-icon-svg="commitIconSvg">
                     </tr>
 
                 </template>

app/assets/javascripts/environments/components/environment_actions.js.es6

@@ -12,38 +12,18 @@
         required: false,
         default: () => [],
       },
-    },
-
-    /**
-     * Appends the svg icon that were render in the index page.
-     * In order to reuse the svg instead of copy and paste in this template
-     * we need to render it outside this component using =custom_icon partial.
-     *
-     * TODO: Remove this when webpack is merged.
-     *
-     */
-    mounted() {
-      const playIcon = document.querySelector('.play-icon-svg.hidden svg');
-
-      const dropdownContainer = this.$el.querySelector('.dropdown-play-icon-container');
-      const actionContainers = this.$el.querySelectorAll('.action-play-icon-container');
-      // Phantomjs does not have support to iterate a nodelist.
-      const actionsArray = [].slice.call(actionContainers);
 
-      if (playIcon && actionsArray && dropdownContainer) {
-        dropdownContainer.appendChild(playIcon.cloneNode(true));
-
-        actionsArray.forEach((element) => {
-          element.appendChild(playIcon.cloneNode(true));
-        });
-      }
+      playIconSvg: {
+        type: String,
+        required: false,
+      },
     },
 
     template: `
       <div class="inline">
         <div class="dropdown">
           <a class="dropdown-new btn btn-default" data-toggle="dropdown">
-            <span class="dropdown-play-icon-container"></span>
+            <span class="js-dropdown-play-icon-container" v-html="playIconSvg"></span>
             <i class="fa fa-caret-down"></i>
           </a>
 
@@ -53,7 +33,9 @@
                 data-method="post"
                 rel="nofollow"
                 class="js-manual-action-link">
-                <span class="action-play-icon-container"></span>
+
+                <span class="js-action-play-icon-container" v-html="playIconSvg"></span>
+
                 <span>
                   {{action.name}}
                 </span>

app/assets/javascripts/environments/components/environment_external_url.js.es6

@@ -7,14 +7,14 @@
 
   window.gl.environmentsList.ExternalUrlComponent = Vue.component('external-url-component', {
     props: {
-      external_url: {
+      externalUrl: {
         type: String,
         default: '',
       },
     },
 
     template: `
-      <a class="btn external_url" :href="external_url" target="_blank">
+      <a class="btn external_url" :href="externalUrl" target="_blank">
         <i class="fa fa-external-link"></i>
       </a>
     `,

app/assets/javascripts/environments/components/environment_item.js.es6

@@ -58,6 +58,16 @@
         required: false,
         default: false,
       },
+
+      commitIconSvg: {
+        type: String,
+        required: false,
+      },
+
+      playIconSvg: {
+        type: String,
+        required: false,
+      },
     },
 
     data() {
@@ -451,11 +461,12 @@
           <div v-if="!isFolder && hasLastDeploymentKey" class="js-commit-component">
             <commit-component
               :tag="commitTag"
-              :commit_ref="commitRef"
-              :commit_url="commitUrl"
-              :short_sha="commitShortSha"
+              :commit-ref="commitRef"
+              :commit-url="commitUrl"
+              :short-sha="commitShortSha"
               :title="commitTitle"
-              :author="commitAuthor">
+              :author="commitAuthor"
+              :commit-icon-svg="commitIconSvg">
             </commit-component>
           </div>
           <p v-if="!isFolder && !hasLastDeploymentKey" class="commit-title">
@@ -476,6 +487,7 @@
             <div v-if="hasManualActions && canCreateDeployment"
               class="inline js-manual-actions-container">
               <actions-component
+                :play-icon-svg="playIconSvg"
                 :actions="manualActions">
               </actions-component>
             </div>
@@ -483,22 +495,22 @@
             <div v-if="model.external_url && canReadEnvironment"
               class="inline js-external-url-container">
               <external-url-component
-                :external_url="model.external_url">
-              </external_url-component>
+                :external-url="model.external_url">
+              </external-url-component>
             </div>
 
             <div v-if="isStoppable && canCreateDeployment"
               class="inline js-stop-component-container">
               <stop-component
-                :stop_url="model.stop_path">
+                :stop-url="model.stop_path">
               </stop-component>
             </div>
 
             <div v-if="canRetry && canCreateDeployment"
               class="inline js-rollback-component-container">
               <rollback-component
-                :is_last_deployment="isLastDeployment"
-                :retry_url="retryUrl">
+                :is-last-deployment="isLastDeployment"
+                :retry-url="retryUrl">
                 </rollback-component>
             </div>
           </div>

app/assets/javascripts/environments/components/environment_rollback.js.es6

@@ -7,19 +7,20 @@
 
   window.gl.environmentsList.RollbackComponent = Vue.component('rollback-component', {
     props: {
-      retry_url: {
+      retryUrl: {
         type: String,
         default: '',
       },
-      is_last_deployment: {
+
+      isLastDeployment: {
         type: Boolean,
         default: true,
       },
     },
 
     template: `
-      <a class="btn" :href="retry_url" data-method="post" rel="nofollow">
-        <span v-if="is_last_deployment">
+      <a class="btn" :href="retryUrl" data-method="post" rel="nofollow">
+        <span v-if="isLastDeployment">
           Re-deploy
         </span>
         <span v-else>

app/assets/javascripts/environments/components/environment_stop.js.es6

@@ -7,7 +7,7 @@
 
   window.gl.environmentsList.StopComponent = Vue.component('stop-component', {
     props: {
-      stop_url: {
+      stopUrl: {
         type: String,
         default: '',
       },
@@ -15,7 +15,7 @@
 
     template: `
       <a class="btn stop-env-link"
-        :href="stop_url"
+        :href="stopUrl"
         data-confirm="Are you sure you want to stop this environment?"
         data-method="post"
         rel="nofollow">

app/assets/javascripts/vue_common_component/commit.js.es6

@@ -23,7 +23,7 @@
        * name
        * ref_url
        */
-      commit_ref: {
+      commitRef: {
         type: Object,
         required: false,
         default: () => ({}),
@@ -32,16 +32,16 @@
       /**
        * Used to link to the commit sha.
        */
-      commit_url: {
+      commitUrl: {
         type: String,
         required: false,
         default: '',
       },
 
       /**
-       * Used to show the commit short_sha that links to the commit url.
+       * Used to show the commit short sha that links to the commit url.
        */
-      short_sha: {
+      shortSha: {
         type: String,
         required: false,
         default: '',
@@ -68,6 +68,11 @@
         required: false,
         default: () => ({}),
       },
+
+      commitIconSvg: {
+        type: String,
+        required: false,
+      },
     },
 
     computed: {
@@ -80,7 +85,7 @@
        * @returns {Boolean}
        */
       hasCommitRef() {
-        return this.commit_ref && this.commit_ref.name && this.commit_ref.ref_url;
+        return this.commitRef && this.commitRef.name && this.commitRef.ref_url;
       },
 
       /**
@@ -110,24 +115,6 @@
       },
     },
 
-    /**
-     * In order to reuse the svg instead of copy and paste in this template
-     * we need to render it outside this component using =custom_icon partial.
-     * Make sure it has this structure:
-     * .commit-icon-svg.hidden
-     *   svg
-     *
-     * TODO: Find a better way to include SVG
-     */
-    mounted() {
-      const commitIconContainer = this.$el.querySelector('.commit-icon-container');
-      const commitIcon = document.querySelector('.commit-icon-svg.hidden svg');
-
-      if (commitIconContainer && commitIcon) {
-        commitIconContainer.appendChild(commitIcon.cloneNode(true));
-      }
-    },
-
     template: `
       <div class="branch-commit">
 
@@ -138,15 +125,15 @@
 
         <a v-if="hasCommitRef"
           class="monospace branch-name"
-          :href="commit_ref.ref_url">
-          {{commit_ref.name}}
+          :href="commitRef.ref_url">
+          {{commitRef.name}}
         </a>
 
-        <div class="icon-container commit-icon commit-icon-container"></div>
+        <div v-html="commitIconSvg" class="commit-icon js-commit-icon"></div>
 
         <a class="commit-id monospace"
-          :href="commit_url">
-          {{short_sha}}
+          :href="commitUrl">
+          {{shortSha}}
         </a>
 
         <p class="commit-title">
@@ -162,7 +149,7 @@
             </a>
 
             <a class="commit-row-message"
-              :href="commit_url">
+              :href="commitUrl">
               {{title}}
             </a>
           </span>

app/views/projects/environments/index.html.haml

@@ -17,4 +17,6 @@
   "project-stopped-environments-path" => project_environments_path(@project, scope: :stopped),
   "new-environment-path" => new_namespace_project_environment_path(@project.namespace, @project),
   "help-page-path" => help_page_path("ci/environments"),
-  "css-class" => container_class}}
+  "css-class" => container_class,
+  "commit-icon-svg" => custom_icon("icon_commit"),
+  "play-icon-svg" => custom_icon("icon_play")}}

changelogs/unreleased/25374-svg-as-prop.yml

+---
+title: Resolve "Provide SVG as a prop instead of hiding and copy them in environments table"
+merge_request: 7992
+author:

spec/features/environments_spec.rb

@@ -85,14 +85,14 @@ feature 'Environments page', :feature, :js do
         end
 
         scenario 'does show a play button' do
-          find('.dropdown-play-icon-container').click
+          find('.js-dropdown-play-icon-container').click
           expect(page).to have_content(manual.name.humanize)
         end
 
         scenario 'does allow to play manual action', js: true do
           expect(manual).to be_skipped
 
-          find('.dropdown-play-icon-container').click
+          find('.js-dropdown-play-icon-container').click
           expect(page).to have_content(manual.name.humanize)
 
           expect { click_link(manual.name.humanize) }

spec/javascripts/environments/environment_actions_spec.js.es6

@@ -8,7 +8,7 @@ describe('Actions Component', () => {
     fixture.load('environments/element.html');
   });
 
-  it('Should render a dropdown with the provided actions', () => {
+  it('should render a dropdown with the provided actions', () => {
     const actionsMock = [
       {
         name: 'bar',
@@ -24,6 +24,7 @@ describe('Actions Component', () => {
       el: document.querySelector('.test-dom-element'),
       propsData: {
         actions: actionsMock,
+        playIconSvg: '<svg></svg>',
       },
     });
 
@@ -34,4 +35,33 @@ describe('Actions Component', () => {
       component.$el.querySelector('.dropdown-menu li a').getAttribute('href'),
     ).toEqual(actionsMock[0].play_path);
   });
+
+  it('should render a dropdown with the provided svg', () => {
+    const actionsMock = [
+      {
+        name: 'bar',
+        play_path: 'https://gitlab.com/play',
+      },
+      {
+        name: 'foo',
+        play_path: '#',
+      },
+    ];
+
+    const component = new window.gl.environmentsList.ActionsComponent({
+      el: document.querySelector('.test-dom-element'),
+      propsData: {
+        actions: actionsMock,
+        playIconSvg: '<svg></svg>',
+      },
+    });
+
+    expect(
+      component.$el.querySelector('.js-dropdown-play-icon-container').children,
+    ).toContain('svg');
+
+    expect(
+      component.$el.querySelector('.js-action-play-icon-container').children,
+    ).toContain('svg');
+  });
 });

spec/javascripts/environments/environment_external_url_spec.js.es6

@@ -7,12 +7,12 @@ describe('External URL Component', () => {
     fixture.load('environments/element.html');
   });
 
-  it('should link to the provided external_url', () => {
+  it('should link to the provided externalUrl prop', () => {
     const externalURL = 'https://gitlab.com';
     const component = new window.gl.environmentsList.ExternalUrlComponent({
       el: document.querySelector('.test-dom-element'),
       propsData: {
-        external_url: externalURL,
+        externalUrl: externalURL,
       },
     });
 

spec/javascripts/environments/environment_rollback_spec.js.es6

@@ -9,24 +9,24 @@ describe('Rollback Component', () => {
     fixture.load('environments/element.html');
   });
 
-  it('Should link to the provided retry_url', () => {
+  it('Should link to the provided retryUrl', () => {
     const component = new window.gl.environmentsList.RollbackComponent({
       el: document.querySelector('.test-dom-element'),
       propsData: {
-        retry_url: retryURL,
-        is_last_deployment: true,
+        retryUrl: retryURL,
+        isLastDeployment: true,
       },
     });
 
     expect(component.$el.getAttribute('href')).toEqual(retryURL);
   });
 
-  it('Should render Re-deploy label when is_last_deployment is true', () => {
+  it('Should render Re-deploy label when isLastDeployment is true', () => {
     const component = new window.gl.environmentsList.RollbackComponent({
       el: document.querySelector('.test-dom-element'),
       propsData: {
-        retry_url: retryURL,
-        is_last_deployment: true,
+        retryUrl: retryURL,
+        isLastDeployment: true,
       },
     });
 
@@ -34,12 +34,12 @@ describe('Rollback Component', () => {
   });
 
 
-  it('Should render Rollback label when is_last_deployment is false', () => {
+  it('Should render Rollback label when isLastDeployment is false', () => {
     const component = new window.gl.environmentsList.RollbackComponent({
       el: document.querySelector('.test-dom-element'),
       propsData: {
-        retry_url: retryURL,
-        is_last_deployment: false,
+        retryUrl: retryURL,
+        isLastDeployment: false,
       },
     });
 

spec/javascripts/environments/environment_stop_spec.js.es6

@@ -13,7 +13,7 @@ describe('Stop Component', () => {
     component = new window.gl.environmentsList.StopComponent({
       el: document.querySelector('.test-dom-element'),
       propsData: {
-        stop_url: stopURL,
+        stopUrl: stopURL,
       },
     });
   });

spec/javascripts/vue_common_components/commit_spec.js.es6

@@ -10,12 +10,12 @@ describe('Commit component', () => {
       el: document.querySelector('.test-commit-container'),
       propsData: {
         tag: false,
-        commit_ref: {
+        commitRef: {
           name: 'master',
           ref_url: 'http://localhost/namespace2/gitlabhq/tree/master',
         },
-        commit_url: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067',
-        short_sha: 'b7836edd',
+        commitUrl: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067',
+        shortSha: 'b7836edd',
         title: 'Commit message',
         author: {
           avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
@@ -34,18 +34,19 @@ describe('Commit component', () => {
 
       props = {
         tag: true,
-        commit_ref: {
+        commitRef: {
           name: 'master',
           ref_url: 'http://localhost/namespace2/gitlabhq/tree/master',
         },
-        commit_url: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067',
-        short_sha: 'b7836edd',
+        commitUrl: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067',
+        shortSha: 'b7836edd',
         title: 'Commit message',
         author: {
           avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
           web_url: 'https://gitlab.com/jschatz1',
           username: 'jschatz1',
         },
+        commitIconSvg: '<svg></svg>',
       };
 
       component = new window.gl.CommitComponent({
@@ -59,20 +60,24 @@ describe('Commit component', () => {
     });
 
     it('should render a link to the ref url', () => {
-      expect(component.$el.querySelector('.branch-name').getAttribute('href')).toEqual(props.commit_ref.ref_url);
+      expect(component.$el.querySelector('.branch-name').getAttribute('href')).toEqual(props.commitRef.ref_url);
     });
 
     it('should render the ref name', () => {
-      expect(component.$el.querySelector('.branch-name').textContent).toContain(props.commit_ref.name);
+      expect(component.$el.querySelector('.branch-name').textContent).toContain(props.commitRef.name);
     });
 
     it('should render the commit short sha with a link to the commit url', () => {
-      expect(component.$el.querySelector('.commit-id').getAttribute('href')).toEqual(props.commit_url);
-      expect(component.$el.querySelector('.commit-id').textContent).toContain(props.short_sha);
+      expect(component.$el.querySelector('.commit-id').getAttribute('href')).toEqual(props.commitUrl);
+      expect(component.$el.querySelector('.commit-id').textContent).toContain(props.shortSha);
+    });
+
+    it('should render the given commitIconSvg', () => {
+      expect(component.$el.querySelector('.js-commit-icon').children).toContain('svg');
     });
 
     describe('Given commit title and author props', () => {
-      it('Should render a link to the author profile', () => {
+      it('should render a link to the author profile', () => {
         expect(
           component.$el.querySelector('.commit-title .avatar-image-container').getAttribute('href'),
         ).toEqual(props.author.web_url);
@@ -91,7 +96,7 @@ describe('Commit component', () => {
     it('should render the commit title', () => {
       expect(
         component.$el.querySelector('a.commit-row-message').getAttribute('href'),
-      ).toEqual(props.commit_url);
+      ).toEqual(props.commitUrl);
       expect(
         component.$el.querySelector('a.commit-row-message').textContent,
       ).toContain(props.title);
@@ -99,16 +104,16 @@ describe('Commit component', () => {
   });
 
   describe('When commit title is not provided', () => {
-    it('Should render default message', () => {
+    it('should render default message', () => {
       fixture.set('<div class="test-commit-container"></div>');
       props = {
         tag: false,
-        commit_ref: {
+        commitRef: {
           name: 'master',
           ref_url: 'http://localhost/namespace2/gitlabhq/tree/master',
         },
-        commit_url: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067',
-        short_sha: 'b7836edd',
+        commitUrl: 'https://gitlab.com/gitlab-org/gitlab-ce/commit/b7836eddf62d663c665769e1b0960197fd215067',
+        shortSha: 'b7836edd',
         title: null,
         author: {},
       };