Fix avatar loading spinner size

Refactored avatar component logic to be located only in Phoenix core. Moved spinner into the avatar component. Adjusted spinner size to match the one of the avatar to avoid layout shifting.
owncloud · Jan 29, 2020 · 5df30e4 · 5df30e4
1 parent e0df137
commit 5df30e4
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 73 deletions.
diff --git a/apps/files/src/components/Collaborators/AutocompleteItem.vue b/apps/files/src/components/Collaborators/AutocompleteItem.vue
@@ -1,12 +1,9 @@
 <template>
   <div class="uk-flex uk-flex-middle">
-    <oc-spinner v-if="loading" uk-spinner="ratio:1.6" class="uk-margin-small-right" :aria-label="$gettext('Loading avatar')" />
+    <avatar-image v-if="item.value.shareType === shareTypes.user" class="uk-margin-small-right" :width="50" :userid="item.value.shareWith" :userName="item.label" />
     <template v-else>
-      <oc-avatar v-if="item.value.shareType === 0" :src="avatar" class="uk-margin-small-right" :width="50" :userName="item.label" />
-      <template v-else>
-        <oc-icon v-if="item.value.shareType === 1" class="uk-margin-small-right" name="group" size="large" />
-        <oc-icon v-else class="uk-margin-small-right" name="person" size="large" />
-      </template>
+      <oc-icon v-if="item.value.shareType === shareTypes.group" class="uk-margin-small-right" name="group" size="large" key="avatar-group" />
+      <oc-icon v-else class="uk-margin-small-right" name="person" size="large" key="avatar-generic-person" />
     </template>
     <div class="files-collaborators-autocomplete-user-text">
       <div
@@ -21,18 +18,16 @@
 
 <script>
 import Mixins from '../../mixins/collaborators'
+import { shareTypes } from '../../helpers/shareTypes'
 
 export default {
   name: 'AutocompleteItem',
   props: ['item'],
   mixins: [Mixins],
-  mounted () {
-    this.$_ocCollaborators_loadAvatar(this.item)
-  },
   data () {
     return {
-      loading: false,
-      avatar: ''
+      shareTypes,
+      loading: false
     }
   }
 }

diff --git a/apps/files/src/components/Collaborators/Collaborator.vue b/apps/files/src/components/Collaborators/Collaborator.vue
@@ -2,12 +2,11 @@
   <div>
     <div v-if="$_reshareInformation" class="uk-text-meta uk-flex uk-flex-middle uk-margin-small-bottom"><oc-icon name="repeat" class="uk-margin-small-right" /> {{ $_reshareInformation }}</div>
     <div class="files-collaborators-collaborator-information uk-flex uk-flex-wrap uk-flex-middle">
-      <oc-spinner v-if="loading" key="collaborator-avatar-spinner" uk-spinner="ratio:1.6" class="uk-margin-small-right" :aria-label="$gettext('Loading')"/>
-      <div v-else key="collaborator-avatar-loaded">
-        <oc-avatar v-if="collaborator.info.share_type === '0'" :src="avatar" class="uk-margin-small-right" :width="50" :userName="collaborator.displayName" />
+      <div key="collaborator-avatar-loaded">
+        <avatar-image v-if="$_shareType === shareTypes.user" class="uk-margin-small-right" :width="50" :userid="collaborator.name" :userName="collaborator.displayName" />
         <div v-else key="collaborator-avatar-placeholder">
-          <oc-icon v-if="collaborator.info.share_type === '1'" class="uk-margin-small-right" name="group" size="large" />
-          <oc-icon v-else class="uk-margin-small-right" name="person" size="large" />
+          <oc-icon v-if="$_shareType === shareTypes.group" class="uk-margin-small-right" name="group" size="large" key="avatar-group" />
+          <oc-icon v-else class="uk-margin-small-right" name="person" size="large" key="avatar-generic-person" />
         </div>
       </div>
       <div class="uk-flex uk-flex-column uk-flex-center files-collaborators-collaborator-information-text">
@@ -26,6 +25,7 @@
 
 <script>
 import { mapGetters } from 'vuex'
+import { shareTypes } from '../../helpers/shareTypes'
 import Mixins from '../../mixins/collaborators'
 
 export default {
@@ -34,10 +34,9 @@ export default {
   mixins: [
     Mixins
   ],
-  data () {
+  data: function () {
     return {
-      avatar: '',
-      loading: false
+      shareTypes
     }
   },
   computed: {
@@ -51,6 +50,10 @@ export default {
       return this.collaborator.info.displayname_owner
     },
 
+    $_shareType () {
+      return parseInt(this.collaborator.info.share_type, 10)
+    },
+
     originalRole () {
       if (this.collaborator.role.name === 'advancedRole') {
         return this.advancedRole
@@ -65,9 +68,6 @@ export default {
         label: this.$gettext('Unknown Role')
       }
     }
-  },
-  mounted () {
-    this.$_ocCollaborators_loadAvatar(this.collaborator)
   }
 }
 </script>

diff --git a/apps/files/src/components/Collaborators/EditCollaborator.vue b/apps/files/src/components/Collaborators/EditCollaborator.vue
@@ -70,9 +70,6 @@ export default {
       return filterObject(permissions, (key, value) => value)
     }
   },
-  mounted () {
-    this.$_ocCollaborators_loadAvatar(this.collaborator)
-  },
   methods: {
     ...mapActions('Files', ['changeShare', 'toggleCollaboratorsEdit']),
 

diff --git a/apps/files/src/mixins/collaborators.js b/apps/files/src/mixins/collaborators.js
@@ -84,43 +84,6 @@ export default {
       this.editing = true
       this.toggleCollaboratorsEdit(true)
     },
-    $_ocCollaborators_loadAvatar (item) {
-      if ((item.value && item.value.shareType !== 0) || (item.info && item.info.share_type !== '0')) return
-
-      let name
-      if (item.value) {
-        name = item.value.shareWith
-      } else if (item.info) {
-        name = item.name
-      }
-
-      const dav = this.$client.helpers._davPath
-      const headers = new Headers()
-      const url = `${dav}/avatars/${name}/128.png`
-
-      headers.append('Authorization', 'Bearer ' + this.getToken)
-      headers.append('X-Requested-With', 'XMLHttpRequest')
-
-      this.loading = true
-
-      fetch(url, { headers })
-        .then(response => {
-          if (response.ok) {
-            return response.blob()
-          }
-          throw new Error('Network response was not ok.')
-        })
-        .then(blob => {
-          this.avatar = window.URL.createObjectURL(blob)
-        })
-        .catch(_ => {
-          this.avatar = ''
-        })
-        .finally(_ => {
-          this.loading = false
-        })
-    },
-
     collaboratorOptionChanged ({ role, permissions, propagate = true }) {
       this.selectedRole = role
       this.additionalPermissions = permissions

diff --git a/apps/files/src/store/actions.js b/apps/files/src/store/actions.js
@@ -235,7 +235,6 @@ function _buildShare (s, file) {
     case ('1'): // group share
       share.role = bitmaskToRole(s.permissions, file.type === 'folder')
       share.permissions = s.permissions
-      share.avatar = 'https://picsum.photos/64/64?image=1075' // TODO where do we get the avatar from? by uid? remote.php/dav/avatars/admin/128.png
       share.name = s.share_with // this is the recipient userid, rename to uid or subject? add separate field userName?
       share.displayName = s.share_with_displayname
       // TODO: Refactor to work with roles / prepare for roles API

diff --git a/changelog/unreleased/2921 b/changelog/unreleased/2921
@@ -0,0 +1,7 @@
+Bugfix: Prevent jumpy behavior when loading user avatars
+
+When loading a user avatar, the container size was smaller so as soon as the avatar was loaded, it resulted in jumpy behavior.
+This is fixed now by applying the same size to the loading spinner element.
+
+https://github.com/owncloud/phoenix/issues/2921
+https://github.com/owncloud/phoenix/pull/2927
diff --git a/src/components/Avatar.vue b/src/components/Avatar.vue
@@ -1,12 +1,15 @@
 <template>
   <component :is="type" v-if="enabled">
-    <oc-avatar :width="width" :loading="loading" :src="avatarSource" :userName="userName" />
+    <oc-spinner v-if="loading" size="small" key="avatar-loading" :aria-label="$gettext('Loading')"
+     :style="'width:' + width + 'px;height:' + width + 'px'" />
+    <oc-avatar v-else key="avatar-loaded" :width="width" :src="avatarSource" :userName="userName" />
   </component>
 </template>
 <script>
 import { mapGetters } from 'vuex'
 
 export default {
+  name: 'Avatar',
   data () {
     return {
       /**
@@ -40,16 +43,23 @@ export default {
           if (response.ok) {
             return response.blob()
           }
-          throw new Error('Network response was not ok.')
+          if (response.status !== 404) {
+            throw new Error(`Unexpected status code ${response.status}`)
+          }
         })
         .then(blob => {
           this.loading = false
-          this.avatarSource = window.URL.createObjectURL(blob)
+          if (blob) {
+            this.avatarSource = window.URL.createObjectURL(blob)
+          } else {
+            // 404, none found
+            this.avatarSource = ''
+          }
         })
         .catch(error => {
           this.avatarSource = ''
           this.loading = false
-          console.log('There has been a problem with your fetch operation: ', error.message)
+          console.error(`Error loading avatar image for user "${this.userid}": `, error.message)
         })
     }
   },

diff --git a/src/components/UserMenu.vue b/src/components/UserMenu.vue
@@ -2,13 +2,13 @@
   <div>
     <oc-button id="_userMenuButton" class="oc-topbar-personal uk-height-1-1" variation="primary" aria-label="$gettext('User Menu')" ref="menuButton">
       <oc-grid gutter="small">
-        <avatar class="oc-topbar-personal-avatar" :width="32" :userid="userId" :userName="userDisplayName"/>
+        <avatar-image class="oc-topbar-personal-avatar" :width="32" :userid="userId" :userName="userDisplayName"/>
         <div class="oc-topbar-personal-label">{{ userDisplayName }}</div>
       </oc-grid>
     </oc-button>
     <oc-drop toggle="#_userMenuButton" mode="click" :options="{pos:'bottom-right'}" class="uk-width-large" ref="menu">
       <div class="uk-card-body uk-flex uk-flex-middle uk-flex-column" id="account-info-container">
-        <avatar
+        <avatar-image
           :userid="userId"
           :userName="userDisplayName"
           :width="128"
@@ -28,12 +28,8 @@
 
 <script>
 import appVersionJson from '../../build/version.json'
-import Avatar from './Avatar.vue'
 
 export default {
-  components: {
-    Avatar
-  },
   props: {
     userId: {
       type: String,

diff --git a/src/phoenix.js b/src/phoenix.js
@@ -45,6 +45,8 @@ import { Drag, Drop } from 'vue-drag-drop'
 import DesignSystem from 'owncloud-design-system'
 import 'owncloud-design-system/dist/system/system.css'
 
+import Avatar from './components/Avatar.vue'
+
 import wgxpath from 'wicked-good-xpath'
 wgxpath.install()
 
@@ -64,6 +66,7 @@ Vue.use(VueMeta, {
 
 Vue.component('drag', Drag)
 Vue.component('drop', Drop)
+Vue.component('avatar-image', Avatar)
 
 // --- Router ----