From b73959a94bcace3d0af7819f7621b308980c49d9 Mon Sep 17 00:00:00 2001
From: Bryce Johnson <bryce@gitlab.com>
Date: Wed, 17 May 2017 11:22:26 -0400
Subject: Fixes per feedback on user avatar components.

---
 .../components/user_avatar/user_avatar_image.vue   | 13 ++++----
 .../user_avatar/user_avatar_size_mixin.js          | 13 --------
 .../components/user_avatar/user_avatar_svg.vue     | 11 ++++---
 .../components/user_avatar_image_spec.js           | 26 ++++++---------
 .../vue_shared/components/user_avatar_link_spec.js | 12 ++-----
 .../components/user_avatar_size_mixin_spec.js      | 37 ----------------------
 .../vue_shared/components/user_avatar_svg_spec.js  |  6 ----
 7 files changed, 26 insertions(+), 92 deletions(-)
 delete mode 100644 app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_size_mixin.js
 delete mode 100644 spec/javascripts/vue_shared/components/user_avatar_size_mixin_spec.js

diff --git a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_image.vue b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_image.vue
index 4891e7f927c..b8db6afda12 100644
--- a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_image.vue
+++ b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_image.vue
@@ -16,12 +16,11 @@
 */
 
 import defaultAvatarUrl from 'images/no_avatar.png';
-import UserAvatarSizeMixin from './user_avatar_size_mixin';
 import TooltipMixin from '../../mixins/tooltip';
 
 export default {
   name: 'UserAvatarImage',
-  mixins: [UserAvatarSizeMixin, TooltipMixin],
+  mixins: [TooltipMixin],
   props: {
     imgSrc: {
       type: String,
@@ -58,8 +57,8 @@ export default {
     tooltipContainer() {
       return this.tooltipText ? 'body' : null;
     },
-    imgCssClasses() {
-      return `avatar ${this.avatarSizeClass} ${this.cssClasses}`;
+    avatarSizeClass() {
+      return `s${this.size}`;
     },
   },
 };
@@ -67,9 +66,11 @@ export default {
 
 <template>
   <img
-    :class="imgCssClasses"
+    class="avatar"
+    :class="[avatarSizeClass, cssClasses]"
     :src="imgSrc"
-    :style="avatarSizeStylesMap"
+    :width="size"
+    :height="size"
     :alt="imgAlt"
     :data-container="tooltipContainer"
     :data-placement="tooltipPlacement"
diff --git a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_size_mixin.js b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_size_mixin.js
deleted file mode 100644
index b6155ffd28e..00000000000
--- a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_size_mixin.js
+++ /dev/null
@@ -1,13 +0,0 @@
-export default {
-  computed: {
-    avatarSizeStylesMap() {
-      return {
-        width: `${this.size}px`,
-        height: `${this.size}px`,
-      };
-    },
-    avatarSizeClass() {
-      return `s${this.size}`;
-    },
-  },
-};
diff --git a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_svg.vue b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_svg.vue
index 39b4d37c91e..d2ff2ac006e 100644
--- a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_svg.vue
+++ b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_svg.vue
@@ -14,10 +14,7 @@
 
 */
 
-import UserAvatarSizeMixin from './user_avatar_size_mixin';
-
 export default {
-  mixins: [UserAvatarSizeMixin],
   props: {
     svg: {
       type: String,
@@ -29,13 +26,19 @@ export default {
       default: 20,
     },
   },
+  computed: {
+    avatarSizeClass() {
+      return `s${this.size}`;
+    },
+  },
 };
 </script>
 
 <template>
   <svg
     :class="avatarSizeClass"
-    :style="avatarSizeStylesMap"
+    :height="size"
+    :width="size"
     v-html="svg">
   </svg>
 </template>
diff --git a/spec/javascripts/vue_shared/components/user_avatar_image_spec.js b/spec/javascripts/vue_shared/components/user_avatar_image_spec.js
index 0b5ec736b1e..8daa7610274 100644
--- a/spec/javascripts/vue_shared/components/user_avatar_image_spec.js
+++ b/spec/javascripts/vue_shared/components/user_avatar_image_spec.js
@@ -18,8 +18,6 @@ describe('User Avatar Image Component', function () {
       this.userAvatarImage = new UserAvatarImageComponent({
         propsData: this.propsData,
       }).$mount();
-
-      this.imageElement = this.userAvatarImage.$el.outerHTML;
     });
 
     it('should return a defined Vue component', function () {
@@ -27,14 +25,7 @@ describe('User Avatar Image Component', function () {
     });
 
     it('should have <img> as a child element', function () {
-      const componentImgTag = this.userAvatarImage.$el.outerHTML;
-      expect(componentImgTag).toContain('<img');
-    });
-
-    it('should return neccessary props as defined', function () {
-      _.each(this.propsData, (val, key) => {
-        expect(this.userAvatarImage[key]).toBeDefined();
-      });
+      expect(this.userAvatarImage.$el.tagName).toBe('IMG');
     });
 
     it('should properly compute tooltipContainer', function () {
@@ -42,19 +33,22 @@ describe('User Avatar Image Component', function () {
     });
 
     it('should properly render tooltipContainer', function () {
-      expect(this.imageElement).toContain('data-container="body"');
+      expect(this.userAvatarImage.$el.getAttribute('data-container')).toBe('body');
     });
 
     it('should properly compute avatarSizeClass', function () {
       expect(this.userAvatarImage.avatarSizeClass).toBe('s99');
     });
 
-    it('should properly compute imgCssClasses', function () {
-      expect(this.userAvatarImage.imgCssClasses).toBe('avatar s99 myextraavatarclass');
-    });
+    it('should properly render img css', function () {
+      const classList = this.userAvatarImage.$el.classList;
+      const containsAvatar = classList.contains('avatar');
+      const containsSizeClass = classList.contains('s99');
+      const containsCustomClass = classList.contains('myextraavatarclass');
 
-    it('should properly render imgCssClasses', function () {
-      expect(this.imageElement).toContain('avatar s99 myextraavatarclass');
+      expect(containsAvatar).toBe(true);
+      expect(containsSizeClass).toBe(true);
+      expect(containsCustomClass).toBe(true);
     });
   });
 });
diff --git a/spec/javascripts/vue_shared/components/user_avatar_link_spec.js b/spec/javascripts/vue_shared/components/user_avatar_link_spec.js
index 770daa9f0de..52e450e9ba5 100644
--- a/spec/javascripts/vue_shared/components/user_avatar_link_spec.js
+++ b/spec/javascripts/vue_shared/components/user_avatar_link_spec.js
@@ -35,13 +35,11 @@ describe('User Avatar Link Component', function () {
   });
 
   it('should render <a> as a child element', function () {
-    const componentLinkTag = this.userAvatarLink.$el.outerHTML;
-    expect(componentLinkTag).toContain('<a');
+    expect(this.userAvatarLink.$el.tagName).toBe('A');
   });
 
   it('should have <img> as a child element', function () {
-    const componentImgTag = this.userAvatarLink.$el.outerHTML;
-    expect(componentImgTag).toContain('<img');
+    expect(this.userAvatarLink.$el.querySelector('img')).not.toBeNull();
   });
 
   it('should return neccessary props as defined', function () {
@@ -49,10 +47,4 @@ describe('User Avatar Link Component', function () {
       expect(this.userAvatarLink[key]).toBeDefined();
     });
   });
-
-  it('should include props in the rendered output', function () {
-    _.each(this.propsData, (val) => {
-      expect(this.userAvatarLink.$el.outerHTML).toContain(val);
-    });
-  });
 });
diff --git a/spec/javascripts/vue_shared/components/user_avatar_size_mixin_spec.js b/spec/javascripts/vue_shared/components/user_avatar_size_mixin_spec.js
deleted file mode 100644
index b37813cdb3d..00000000000
--- a/spec/javascripts/vue_shared/components/user_avatar_size_mixin_spec.js
+++ /dev/null
@@ -1,37 +0,0 @@
-import Vue from 'vue';
-import UserAvatarSizeMixin from '~/vue_shared/components/user_avatar/user_avatar_size_mixin';
-
-describe('User Avatar Size Mixin', () => {
-  beforeEach(() => {
-    this.vueInstance = new Vue({
-      data: {
-        size: 99,
-      },
-      mixins: [UserAvatarSizeMixin],
-    });
-  });
-
-  describe('#avatarSizeClass', () => {
-    it('should be a defined computed value', () => {
-      expect(this.vueInstance.avatarSizeClass).toBeDefined();
-    });
-
-    it('should correctly transform size into the class name', () => {
-      expect(this.vueInstance.avatarSizeClass).toBe('s99');
-    });
-  });
-
-  describe('#avatarSizeStylesMap', () => {
-    it('should be a defined computed value', () => {
-      expect(this.vueInstance.avatarSizeStylesMap).toBeDefined();
-    });
-
-    it('should return a correctly formatted styles width', () => {
-      expect(this.vueInstance.avatarSizeStylesMap.width).toBe('99px');
-    });
-
-    it('should return a correctly formatted styles height', () => {
-      expect(this.vueInstance.avatarSizeStylesMap.height).toBe('99px');
-    });
-  });
-});
diff --git a/spec/javascripts/vue_shared/components/user_avatar_svg_spec.js b/spec/javascripts/vue_shared/components/user_avatar_svg_spec.js
index 809886c5dbd..b8d639ffbec 100644
--- a/spec/javascripts/vue_shared/components/user_avatar_svg_spec.js
+++ b/spec/javascripts/vue_shared/components/user_avatar_svg_spec.js
@@ -25,11 +25,5 @@ describe('User Avatar Svg Component', function () {
       expect(this.userAvatarSvg.$el.tagName).toEqual('svg');
       expect(this.userAvatarSvg.$el.innerHTML).toContain('<path');
     });
-
-    it('should return neccessary props as defined', function () {
-      _.each(this.propsData, (val, key) => {
-        expect(this.userAvatarSvg[key]).toBeDefined();
-      });
-    });
   });
 });
-- 
cgit v1.2.1