From 3aeba04592794e15166e8e5b481552d751b39c28 Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Thu, 8 Apr 2021 13:52:18 +0200
Subject: [PATCH 01/10] feat: use oc-select to select roles

---
 .../components/Collaborators/Collaborator.vue |  5 +-
 .../CollaboratorsEditOptions.vue              | 71 +++++++++----------
 .../Collaborators/EditCollaborator.vue        |  2 +-
 .../PublicLinksSidebar/EditPublicLink.vue     | 66 +++++++++--------
 .../web-app-files/src/components/RoleItem.vue | 18 +++++
 .../src/components/Roles/RoleItem.vue         | 29 --------
 .../src/components/Roles/RolesSelect.vue      | 68 ------------------
 .../helpers/collaboratorRolesDefinition.js    | 16 ++---
 .../src/helpers/publicLinkRolesDefinition.js  | 44 ++++++------
 .../web-app-files/src/mixins/collaborators.js |  8 ++-
 10 files changed, 127 insertions(+), 200 deletions(-)
 create mode 100644 packages/web-app-files/src/components/RoleItem.vue
 delete mode 100644 packages/web-app-files/src/components/Roles/RoleItem.vue
 delete mode 100644 packages/web-app-files/src/components/Roles/RolesSelect.vue

diff --git a/packages/web-app-files/src/components/Collaborators/Collaborator.vue b/packages/web-app-files/src/components/Collaborators/Collaborator.vue
index 8d285085ae1..ce132d1aa53 100644
--- a/packages/web-app-files/src/components/Collaborators/Collaborator.vue
+++ b/packages/web-app-files/src/components/Collaborators/Collaborator.vue
@@ -266,11 +266,8 @@ export default {
     },
 
     originalRole() {
-      if (this.collaborator.role.name === 'advancedRole') {
-        return this.advancedRole
-      }
+      const role = this.displayRoles.find(r => r.name === this.collaborator.role.name)
 
-      const role = this.displayRoles[this.collaborator.role.name]
       if (role) {
         return role
       }
diff --git a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
index 12b166bd0f3..738c7e303f0 100644
--- a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
+++ b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
@@ -1,17 +1,28 @@
 <template>
   <oc-grid gutter="small" child-width="1-1">
-    <roles-select
-      mode="collaborators"
-      :roles="roles"
-      :selected-role="role"
-      @roleSelected="selectRole"
-    />
+    <label tag="label" class="oc-label">
+      {{ $gettext('Role') }}
+      <oc-select
+        id="files-people-role"
+        v-model="selectedRole"
+        :options="roles"
+        :clearable="false"
+        label="label"
+      >
+        <template v-slot:option="option">
+          <role-item :role="option" />
+        </template>
+        <template #no-options v-translate>
+          No matching role found
+        </template>
+      </oc-select>
+    </label>
     <template v-if="$_ocCollaborators_hasAdditionalPermissions">
       <label v-if="selectedRole.name !== 'advancedRole'" class="oc-label">
         <translate>Additional permissions</translate>
       </label>
       <additional-permissions
-        :available-permissions="role.additionalPermissions"
+        :available-permissions="selectedRole.additionalPermissions"
         :collaborators-permissions="collaboratorsPermissions"
         @permissionChecked="checkAdditionalPermissions"
       />
@@ -48,14 +59,14 @@ import { mapGetters } from 'vuex'
 import moment from 'moment'
 import collaboratorsMixins from '../../mixins/collaborators'
 
-import RolesSelect from '../Roles/RolesSelect.vue'
+import RoleItem from '../RoleItem.vue'
 import AdditionalPermissions from './AdditionalPermissions.vue'
 
 export default {
   name: 'CollaboratorsEditOptions',
 
   components: {
-    RolesSelect,
+    RoleItem,
     AdditionalPermissions
   },
 
@@ -110,30 +121,6 @@ export default {
       return this.selectedRole && this.selectedRole.additionalPermissions
     },
 
-    role() {
-      // Returns default role
-      if (!this.existingRole && !this.selectedRole) {
-        const defaultRole = this.roles[Object.keys(this.roles)[0]]
-        this.selectRole(defaultRole, false)
-        return defaultRole
-      }
-
-      if (
-        (this.existingRole && this.existingRole.name === 'advancedRole' && !this.selectedRole) ||
-        (this.selectedRole && this.selectedRole.name === 'advancedRole')
-      ) {
-        this.selectRole(this.advancedRole, false)
-        return this.advancedRole
-      }
-
-      if (this.existingRole && !this.selectedRole) {
-        this.selectRole(this.existingRole, false)
-        return this.existingRole
-      }
-
-      return this.selectedRole
-    },
-
     expirationSupported() {
       return this.userExpirationDate && this.groupExpirationDate
     },
@@ -236,6 +223,19 @@ export default {
     }
   },
 
+  created() {
+    if (
+      (this.existingRole && this.existingRole.name === 'advancedRole' && !this.selectedRole) ||
+      (this.selectedRole && this.selectedRole.name === 'advancedRole')
+    ) {
+      this.selectedRole = this.advancedRole
+    } else if (this.existingRole && !this.selectedRole) {
+      this.selectedRole = this.existingRole
+    } else {
+      this.selectedRole = this.roles[0]
+    }
+  },
+
   mounted() {
     if (this.expirationSupported) {
       if (this.editingUser || this.editingGroup) {
@@ -253,11 +253,6 @@ export default {
   },
 
   methods: {
-    selectRole(role) {
-      this.selectedRole = role
-      this.publishChange()
-    },
-
     checkAdditionalPermissions(permissions) {
       this.additionalPermissions = permissions
       this.publishChange()
diff --git a/packages/web-app-files/src/components/Collaborators/EditCollaborator.vue b/packages/web-app-files/src/components/Collaborators/EditCollaborator.vue
index 4810937c591..4cf2b59f592 100644
--- a/packages/web-app-files/src/components/Collaborators/EditCollaborator.vue
+++ b/packages/web-app-files/src/components/Collaborators/EditCollaborator.vue
@@ -121,7 +121,7 @@ export default {
     },
 
     $_originalRole() {
-      return this.roles[this.collaborator.role.name]
+      return this.roles.find(r => r.name === this.collaborator.role.name)
     },
 
     $_permissionsBitmask() {
diff --git a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
index 9b67af7a8dc..58318fd7fe7 100644
--- a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
+++ b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
@@ -13,14 +13,23 @@
       <div class="oc-mb">
         <oc-text-input id="oc-files-file-link-name" v-model="name" :label="$gettext('Name')" />
       </div>
-      <oc-grid child-width="1-1" gutter="small">
-        <roles-select
-          :roles="$_roles"
-          :selected-role="$_selectedRole"
-          mode="file-link"
-          @roleSelected="$_selectRole"
-        />
-      </oc-grid>
+      <label tag="label" class="oc-label">
+        {{ $gettext('Role') }}
+        <oc-select
+          id="files-link-role"
+          v-model="selectedRole"
+          :options="roles"
+          :clearable="false"
+          label="label"
+        >
+          <template v-slot:option="option">
+            <role-item :role="option" />
+          </template>
+          <template #no-options v-translate>
+            No matching role found
+          </template>
+        </oc-select>
+      </label>
       <div class="oc-mb uk-grid-small uk-flex" uk-grid>
         <div v-if="$_expirationDate" class="uk-width-1-1 uk-width-2-5@m">
           <div class="uk-position-relative">
@@ -126,15 +135,12 @@
 import { mapGetters, mapActions, mapState, mapMutations } from 'vuex'
 import mixins from '../../mixins'
 import moment from 'moment'
-import filter from 'lodash-es/filter'
-import first from 'lodash-es/first'
-import values from 'lodash-es/values'
 import publicLinkRoles from '../../helpers/publicLinkRolesDefinition'
-import RolesSelect from '../Roles/RolesSelect.vue'
+import RoleItem from '../RoleItem.vue'
 
 export default {
   components: {
-    RolesSelect
+    RoleItem
   },
   mixins: [mixins],
   data() {
@@ -149,7 +155,8 @@ export default {
       placeholder: {
         mailTo: this.$gettext('Mail recipients'),
         mailBody: this.$gettext('Personal note')
-      }
+      },
+      selectedRole: null
     }
   },
   title: $gettext => {
@@ -191,7 +198,7 @@ export default {
       return Object.keys(this.capabilities.files_sharing.public.send_mail).length > 0
     },
 
-    $_roles() {
+    roles() {
       const $gettext = this.$gettext
       return publicLinkRoles({
         $gettext,
@@ -199,17 +206,6 @@ export default {
       })
     },
 
-    $_selectedRole() {
-      const permissions = parseInt(this.permissions, 10)
-      if (permissions) {
-        const matchingRoles = filter(this.$_roles, r => r.permissions === permissions)
-        if (matchingRoles.length > 0) {
-          return first(values(matchingRoles))
-        }
-      }
-      return this.$_roles.viewer
-    },
-
     $_expirationDate() {
       const expireDate = this.capabilities.files_sharing.public.expire_date
 
@@ -294,13 +290,27 @@ export default {
     this.hasPassword = this.publicLinkInEdit.hasPassword
     this.expireDate = this.publicLinkInEdit.expireDate
     this.permissions = this.publicLinkInEdit.permissions
+
+    this.setRole()
   },
   methods: {
     ...mapActions('Files', ['addLink', 'updateLink']),
     ...mapMutations('Files', ['SET_APP_SIDEBAR_ACCORDION_CONTEXT']),
 
-    $_selectRole(role) {
-      this.permissions = role.permissions
+    setRole() {
+      const permissions = parseInt(this.permissions, 10)
+
+      if (permissions) {
+        const role = this.roles.find(r => r.permissions === permissions)
+
+        if (role) {
+          this.selectedRole = role
+
+          return
+        }
+      }
+
+      this.selectedRole = this.roles[0]
     },
 
     $_addLink() {
diff --git a/packages/web-app-files/src/components/RoleItem.vue b/packages/web-app-files/src/components/RoleItem.vue
new file mode 100644
index 00000000000..f9f5fe4eee9
--- /dev/null
+++ b/packages/web-app-files/src/components/RoleItem.vue
@@ -0,0 +1,18 @@
+<template>
+  <span class="roles-select-role-item">
+    <span class="oc-text-bold uk-display-block uk-width-1-1" v-text="role.label" />
+    <span class="oc-m-rm" v-text="role.description" />
+  </span>
+</template>
+
+<script>
+export default {
+  name: 'RoleItem',
+  props: {
+    role: {
+      type: Object,
+      required: true
+    }
+  }
+}
+</script>
diff --git a/packages/web-app-files/src/components/Roles/RoleItem.vue b/packages/web-app-files/src/components/Roles/RoleItem.vue
deleted file mode 100644
index 6cc9949027b..00000000000
--- a/packages/web-app-files/src/components/Roles/RoleItem.vue
+++ /dev/null
@@ -1,29 +0,0 @@
-<template>
-  <div class="roles-select-role-item">
-    <span class="oc-text-bold" v-text="role.label" />
-    <p v-if="showDescription" class="oc-text-muted oc-m-rm" v-text="role.description" />
-  </div>
-</template>
-
-<script>
-export default {
-  name: 'RoleItem',
-  props: {
-    showDescription: {
-      type: Boolean,
-      default: true
-    },
-    role: {
-      type: Object,
-      required: true
-    }
-  }
-}
-</script>
-
-<style>
-/* FIXME: move into ODS? */
-.roles-select-role-item {
-  text-transform: none;
-}
-</style>
diff --git a/packages/web-app-files/src/components/Roles/RolesSelect.vue b/packages/web-app-files/src/components/Roles/RolesSelect.vue
deleted file mode 100644
index 85e29ec791d..00000000000
--- a/packages/web-app-files/src/components/Roles/RolesSelect.vue
+++ /dev/null
@@ -1,68 +0,0 @@
-<template>
-  <div>
-    <label class="oc-label">
-      <translate>Role</translate>
-    </label>
-    <oc-button
-      :id="`files-${mode}-role-button`"
-      justify-content="space-between"
-      :class="`uk-width-1-1 files-${mode}-role-button`"
-    >
-      <role-item :show-description="false" :role="selectedRole" />
-      <oc-icon name="expand_more" />
-    </oc-button>
-    <oc-drop
-      close-on-click
-      :drop-id="`files-${mode}-roles-dropdown`"
-      :toggle="`#files-${mode}-role-button`"
-      mode="click"
-      position="bottom-justify"
-      :options="{ offset: 0, delayHide: 0, flip: false }"
-      class="oc-autocomplete-dropdown"
-    >
-      <ul class="oc-autocomplete-suggestion-list">
-        <li
-          v-for="role in roles"
-          :id="`files-${mode}-role-${role.name}`"
-          :key="role.name"
-          class="oc-autocomplete-suggestion"
-          :class="{ 'oc-autocomplete-suggestion-selected': role.name === selectedRole.name }"
-          @click="selectRole(role)"
-        >
-          <role-item :role="role" />
-        </li>
-      </ul>
-    </oc-drop>
-  </div>
-</template>
-
-<script>
-import RoleItem from './RoleItem.vue'
-
-export default {
-  name: 'RolesSelect',
-  components: { RoleItem },
-  props: {
-    mode: {
-      type: String,
-      required: true,
-      validator(value) {
-        return ['collaborators', 'file-link'].indexOf(value) !== -1
-      }
-    },
-    roles: {
-      type: Object,
-      required: true
-    },
-    selectedRole: {
-      type: Object,
-      required: true
-    }
-  },
-  methods: {
-    selectRole(role) {
-      this.$emit('roleSelected', role)
-    }
-  }
-}
-</script>
diff --git a/packages/web-app-files/src/helpers/collaboratorRolesDefinition.js b/packages/web-app-files/src/helpers/collaboratorRolesDefinition.js
index 9af20291ba8..cfe8bf677e2 100644
--- a/packages/web-app-files/src/helpers/collaboratorRolesDefinition.js
+++ b/packages/web-app-files/src/helpers/collaboratorRolesDefinition.js
@@ -13,8 +13,8 @@ function returnOriginal(string) {
  */
 export default ({ isFolder = false, $gettext = returnOriginal, allowSharePerm = false }) => {
   if (isFolder) {
-    return {
-      viewer: {
+    return [
+      {
         name: 'viewer',
         label: $gettext('Viewer'),
         description: allowSharePerm
@@ -22,7 +22,7 @@ export default ({ isFolder = false, $gettext = returnOriginal, allowSharePerm =
           : $gettext('Download and preview'),
         permissions: allowSharePerm ? ['read', 'share'] : ['read']
       },
-      editor: {
+      {
         name: 'editor',
         label: $gettext('Editor'),
         description: allowSharePerm
@@ -32,11 +32,11 @@ export default ({ isFolder = false, $gettext = returnOriginal, allowSharePerm =
           ? ['read', 'update', 'create', 'delete', 'share']
           : ['read', 'update', 'create', 'delete']
       }
-    }
+    ]
   }
 
-  return {
-    viewer: {
+  return [
+    {
       name: 'viewer',
       label: $gettext('Viewer'),
       description: allowSharePerm
@@ -44,7 +44,7 @@ export default ({ isFolder = false, $gettext = returnOriginal, allowSharePerm =
         : $gettext('Download and preview'),
       permissions: allowSharePerm ? ['read', 'share'] : ['read']
     },
-    editor: {
+    {
       name: 'editor',
       label: $gettext('Editor'),
       description: allowSharePerm
@@ -52,5 +52,5 @@ export default ({ isFolder = false, $gettext = returnOriginal, allowSharePerm =
         : $gettext('Edit, download and preview'),
       permissions: allowSharePerm ? ['read', 'update', 'share'] : ['read', 'update']
     }
-  }
+  ]
 }
diff --git a/packages/web-app-files/src/helpers/publicLinkRolesDefinition.js b/packages/web-app-files/src/helpers/publicLinkRolesDefinition.js
index f6c742da4c9..6f28f81bdc4 100644
--- a/packages/web-app-files/src/helpers/publicLinkRolesDefinition.js
+++ b/packages/web-app-files/src/helpers/publicLinkRolesDefinition.js
@@ -11,33 +11,35 @@ function returnOriginal(string) {
  * @returns {object}  Public link roles
  */
 export default ({ isFolder = false, $gettext = returnOriginal }) => {
-  const roles = {
-    viewer: {
+  const roles = [
+    {
       name: 'viewer',
       label: $gettext('Viewer'),
       description: $gettext('Recipients can view and download contents.'),
       permissions: 1
     }
-  }
+  ]
   if (isFolder) {
-    roles.contributor = {
-      name: 'contributor',
-      label: $gettext('Contributor'),
-      description: $gettext('Recipients can view, download and upload contents.'),
-      permissions: 5
-    }
-    roles.editor = {
-      name: 'editor',
-      label: $gettext('Editor'),
-      description: $gettext('Recipients can view, download, edit, delete and upload contents.'),
-      permissions: 15
-    }
-    roles.uploader = {
-      name: 'uploader',
-      label: $gettext('Uploader'),
-      description: $gettext('Recipients can upload but existing contents are not revealed.'),
-      permissions: 4
-    }
+    roles.push(
+      {
+        name: 'contributor',
+        label: $gettext('Contributor'),
+        description: $gettext('Recipients can view, download and upload contents.'),
+        permissions: 5
+      },
+      {
+        name: 'editor',
+        label: $gettext('Editor'),
+        description: $gettext('Recipients can view, download, edit, delete and upload contents.'),
+        permissions: 15
+      },
+      {
+        name: 'uploader',
+        label: $gettext('Uploader'),
+        description: $gettext('Recipients can upload but existing contents are not revealed.'),
+        permissions: 4
+      }
+    )
   }
   return roles
 }
diff --git a/packages/web-app-files/src/mixins/collaborators.js b/packages/web-app-files/src/mixins/collaborators.js
index 7e1c4dc0b8a..d712e1589a3 100644
--- a/packages/web-app-files/src/mixins/collaborators.js
+++ b/packages/web-app-files/src/mixins/collaborators.js
@@ -65,14 +65,16 @@ export default {
         isFolder: isFolder,
         allowSharePerm: !this.isOcis
       })
-      collaboratorRoles.advancedRole = this.advancedRole
+      collaboratorRoles.push(this.advancedRole)
 
       return collaboratorRoles
     },
+
     displayRoles() {
       const result = this.roles
-      result[this.resharerRole.name] = this.resharerRole
-      result[this.ownerRole.name] = this.ownerRole
+
+      result.push(this.resharerRole, this.ownerRole)
+
       return result
     }
   },

From 4bb085833aa178d738b1f4a09778e326f7cf3362 Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Thu, 8 Apr 2021 16:11:54 +0200
Subject: [PATCH 02/10] tests: remove wait steps from selecting share role

---
 .../Collaborators/CollaboratorsEditOptions.vue         |  2 +-
 packages/web-app-files/src/components/RoleItem.vue     |  2 +-
 .../pageObjects/FilesPageElement/sharingDialog.js      | 10 +++-------
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
index 738c7e303f0..d4953994a73 100644
--- a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
+++ b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
@@ -3,7 +3,7 @@
     <label tag="label" class="oc-label">
       {{ $gettext('Role') }}
       <oc-select
-        id="files-people-role"
+        id="files-collaborators-role-button"
         v-model="selectedRole"
         :options="roles"
         :clearable="false"
diff --git a/packages/web-app-files/src/components/RoleItem.vue b/packages/web-app-files/src/components/RoleItem.vue
index f9f5fe4eee9..c1eafde81c6 100644
--- a/packages/web-app-files/src/components/RoleItem.vue
+++ b/packages/web-app-files/src/components/RoleItem.vue
@@ -1,5 +1,5 @@
 <template>
-  <span class="roles-select-role-item">
+  <span :id="`files-collaborators-role-${role.name}`" class="roles-select-role-item">
     <span class="oc-text-bold uk-display-block uk-width-1-1" v-text="role.label" />
     <span class="oc-m-rm" v-text="role.description" />
   </span>
diff --git a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
index f7939ddaed9..22d5960287e 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
@@ -203,12 +203,7 @@ module.exports = {
         .startCase()
         .replace(/\s/g, '')
         .value()
-      return this.waitForElementPresent('@newCollaboratorSelectRoleButton')
-        .click('@newCollaboratorSelectRoleButton')
-        .waitForElementVisible('@newCollaboratorRolesDropdown')
-        .waitForElementVisible(`@newCollaboratorRole${role}`)
-        .click(`@newCollaboratorRole${role}`)
-        .waitForElementNotVisible('@newCollaboratorRolesDropdown')
+      return this.click('@newCollaboratorSelectRoleButton').click(`@newCollaboratorRole${role}`)
     },
     confirmShare: function() {
       return this.waitForElementPresent('@addShareSaveButton')
@@ -659,7 +654,8 @@ module.exports = {
       selector: '#files-collaborators-role-button'
     },
     newCollaboratorRolesDropdown: {
-      selector: '#files-collaborators-roles-dropdown'
+      selector: '//div[@id="files-collaborators-role-button"]//ul[@class="vs__dropdown-menu"]',
+      locateStrategy: 'xpath'
     },
     newCollaboratorRoleViewer: {
       selector: '#files-collaborators-role-viewer'

From c56678c4a74b2275901f28b9b03337bcede1bc95 Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Thu, 8 Apr 2021 16:44:04 +0200
Subject: [PATCH 03/10] feat: use permissions of selected role for public links

---
 .../PublicLinksSidebar/EditPublicLink.vue     | 16 +++++++---------
 .../web-app-files/src/components/RoleItem.vue |  2 +-
 .../FilesPageElement/publicLinksDialog.js     | 19 ++++++-------------
 .../FilesPageElement/sharingDialog.js         | 10 +++-------
 4 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
index 58318fd7fe7..ee7154fbe04 100644
--- a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
+++ b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
@@ -13,10 +13,10 @@
       <div class="oc-mb">
         <oc-text-input id="oc-files-file-link-name" v-model="name" :label="$gettext('Name')" />
       </div>
-      <label tag="label" class="oc-label">
+      <label tag="label" class="oc-label oc-mb">
         {{ $gettext('Role') }}
         <oc-select
-          id="files-link-role"
+          id="files-file-link-role-button"
           v-model="selectedRole"
           :options="roles"
           :clearable="false"
@@ -151,7 +151,6 @@ export default {
       name: null,
       hasPassword: false,
       expireDate: null,
-      permissions: 1,
       placeholder: {
         mailTo: this.$gettext('Mail recipients'),
         mailBody: this.$gettext('Personal note')
@@ -187,7 +186,7 @@ export default {
       return (
         expireDateNow !== expireDateBefore ||
         this.name !== this.publicLinkInEdit.name ||
-        this.permissions !== this.publicLinkInEdit.permissions ||
+        this.selectedRole.permissions !== this.publicLinkInEdit.permissions ||
         (this.publicLinkInEdit.hasPassword
           ? this.password !== null
           : this.password !== null && this.password.trim().length > 0)
@@ -253,7 +252,7 @@ export default {
     },
 
     $_passwordEnforced() {
-      const permissions = parseInt(this.permissions, 10)
+      const permissions = parseInt(this.selectedRole.permissions, 10)
       const password = this.capabilities.files_sharing.public.password.enforced_for
 
       if (permissions === 1 && password.read_only === '1') {
@@ -289,7 +288,6 @@ export default {
     this.name = this.publicLinkInEdit.name
     this.hasPassword = this.publicLinkInEdit.hasPassword
     this.expireDate = this.publicLinkInEdit.expireDate
-    this.permissions = this.publicLinkInEdit.permissions
 
     this.setRole()
   },
@@ -298,7 +296,7 @@ export default {
     ...mapMutations('Files', ['SET_APP_SIDEBAR_ACCORDION_CONTEXT']),
 
     setRole() {
-      const permissions = parseInt(this.permissions, 10)
+      const permissions = parseInt(this.publicLinkInEdit.permissions, 10)
 
       if (permissions) {
         const role = this.roles.find(r => r.permissions === permissions)
@@ -318,7 +316,7 @@ export default {
 
       const params = {
         expireDate: this.expireDate,
-        permissions: this.permissions,
+        permissions: this.selectedRole.permissions,
         name: this.name
       }
 
@@ -348,7 +346,7 @@ export default {
 
       const params = {
         expireDate: this.expireDate,
-        permissions: this.permissions,
+        permissions: this.selectedRole.permissions,
         name: this.name
       }
 
diff --git a/packages/web-app-files/src/components/RoleItem.vue b/packages/web-app-files/src/components/RoleItem.vue
index c1eafde81c6..ac7bf2dc1d5 100644
--- a/packages/web-app-files/src/components/RoleItem.vue
+++ b/packages/web-app-files/src/components/RoleItem.vue
@@ -1,5 +1,5 @@
 <template>
-  <span :id="`files-collaborators-role-${role.name}`" class="roles-select-role-item">
+  <span :id="`files-role-${role.name}`" class="roles-select-role-item">
     <span class="oc-text-bold uk-display-block uk-width-1-1" v-text="role.label" />
     <span class="oc-m-rm" v-text="role.description" />
   </span>
diff --git a/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js b/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js
index f9a25cb6389..b06c8c8c2da 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js
@@ -40,12 +40,8 @@ module.exports = {
         this.elements.publicLinkRoleSelectionDropdown.selector,
         role
       )
-      return this.waitForElementPresent('@selectRoleButton')
-        .click('@selectRoleButton')
-        .waitForElementVisible('@rolesDropdown')
-        .waitForElementVisible(`@role${role}`)
+      return this.click('@selectRoleButton')
         .click(`@role${role}`)
-        .waitForElementNotVisible('@rolesDropdown')
         .useXpath()
         .waitForElementVisible(selectedRoleDropdown)
         .useCss()
@@ -382,20 +378,17 @@ module.exports = {
     selectRoleButton: {
       selector: '#files-file-link-role-button'
     },
-    rolesDropdown: {
-      selector: '#files-file-link-roles-dropdown'
-    },
     roleViewer: {
-      selector: '#files-file-link-role-viewer'
+      selector: '#files-role-viewer'
     },
     roleContributor: {
-      selector: '#files-file-link-role-contributor'
+      selector: '#files-role-contributor'
     },
     roleEditor: {
-      selector: '#files-file-link-role-editor'
+      selector: '#files-role-editor'
     },
     roleUploader: {
-      selector: '#files-file-link-role-uploader'
+      selector: '#files-role-uploader'
     },
     errorMessageInsidePublicLinkContainer: {
       selector: '//div[contains(@class, "oc-alert-danger")]',
@@ -440,7 +433,7 @@ module.exports = {
       selector: '#files-sidebar-private-link-icon-copied'
     },
     publicLinkRoleSelectionDropdown: {
-      selector: '//button[contains(@class, "files-file-link-role-button")]//span[.="%s"]',
+      selector: '//div[@id="files-file-link-role-button"]//span[.="%s"]',
       locateStrategy: 'xpath'
     }
   }
diff --git a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
index 22d5960287e..003df103093 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
@@ -653,15 +653,11 @@ module.exports = {
     newCollaboratorSelectRoleButton: {
       selector: '#files-collaborators-role-button'
     },
-    newCollaboratorRolesDropdown: {
-      selector: '//div[@id="files-collaborators-role-button"]//ul[@class="vs__dropdown-menu"]',
-      locateStrategy: 'xpath'
-    },
     newCollaboratorRoleViewer: {
-      selector: '#files-collaborators-role-viewer'
+      selector: '#files-role-viewer'
     },
     newCollaboratorRoleEditor: {
-      selector: '#files-collaborators-role-editor'
+      selector: '#files-role-editor'
     },
     newCollaboratorItems: {
       selector:
@@ -672,7 +668,7 @@ module.exports = {
         "//button[contains(@class, 'files-collaborators-collaborator-autocomplete-item-remove')]"
     },
     newCollaboratorRoleAdvancedPermissions: {
-      selector: '#files-collaborators-role-advancedRole'
+      selector: '#files-role-advancedRole'
     },
     selectRoleButtonInCollaboratorInformation: {
       selector: '//button[contains(@class, "files-collaborators-role-button")]',

From d82a85372122b192b177c46975a2300d6ee5c680 Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Thu, 8 Apr 2021 16:46:09 +0200
Subject: [PATCH 04/10] add changelog

---
 changelog/unreleased/enhancement-select-roles | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 changelog/unreleased/enhancement-select-roles

diff --git a/changelog/unreleased/enhancement-select-roles b/changelog/unreleased/enhancement-select-roles
new file mode 100644
index 00000000000..8d1e2b1cfcb
--- /dev/null
+++ b/changelog/unreleased/enhancement-select-roles
@@ -0,0 +1,6 @@
+Enhancement: Use `oc-select` for role select
+
+We've used the new `oc-select` component from ODS for selecting role in people and public links accordions in the right sidebar.
+We are using this component to enable keyboard navigation when selecting the role.
+
+https://github.com/owncloud/web/pull/4937
\ No newline at end of file

From 2289d4f817595d81054cf965bedae18024bcb16f Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Thu, 22 Apr 2021 17:18:10 +0200
Subject: [PATCH 05/10] feat: watch selected role

---
 .../components/Collaborators/CollaboratorsEditOptions.vue   | 6 ++++++
 .../src/components/Collaborators/NewCollaborator.vue        | 6 +++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
index d4953994a73..8491142ad43 100644
--- a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
+++ b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
@@ -223,6 +223,12 @@ export default {
     }
   },
 
+  watch: {
+    selectedRole: {
+      handler: 'publishChange'
+    }
+  },
+
   created() {
     if (
       (this.existingRole && this.existingRole.name === 'advancedRole' && !this.selectedRole) ||
diff --git a/packages/web-app-files/src/components/Collaborators/NewCollaborator.vue b/packages/web-app-files/src/components/Collaborators/NewCollaborator.vue
index 62cee1b5541..37d1c182d91 100644
--- a/packages/web-app-files/src/components/Collaborators/NewCollaborator.vue
+++ b/packages/web-app-files/src/components/Collaborators/NewCollaborator.vue
@@ -64,7 +64,7 @@
           :disabled="!$_isValid"
           variation="primary"
           appearance="filled"
-          @click="$_ocCollaborators_newCollaboratorsAdd(selectedCollaborators)"
+          @click="share"
         >
           <translate>Share</translate>
         </oc-button>
@@ -203,12 +203,12 @@ export default {
       this.saving = false
       this.close()
     },
-    $_ocCollaborators_newCollaboratorsAdd(collaborators) {
+    share() {
       this.saving = true
 
       const saveQueue = new PQueue({ concurrency: 4 })
       const savePromises = []
-      collaborators.forEach(collaborator => {
+      this.selectedCollaborators.forEach(collaborator => {
         savePromises.push(
           saveQueue.add(() =>
             this.addShare({

From d4817b8b15bf076565b3b3f54add3ae3ad773377 Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Thu, 22 Apr 2021 17:45:05 +0200
Subject: [PATCH 06/10] tests: use correct selector for role select

---
 .../pageObjects/FilesPageElement/sharingDialog.js      | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
index 003df103093..a0f333b055e 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
@@ -404,9 +404,9 @@ module.exports = {
         this.elements.roleButtonInDropdown.selector,
         newRole.toLowerCase()
       )
-      return this.useXpath()
-        .waitForElementVisible('@selectRoleButtonInCollaboratorInformation')
+      return this.waitForElementVisible('@selectRoleButtonInCollaboratorInformation')
         .click('@selectRoleButtonInCollaboratorInformation')
+        .useXpath()
         .waitForElementVisible(newRoleButton)
         .click(newRoleButton)
         .useCss()
@@ -671,8 +671,7 @@ module.exports = {
       selector: '#files-role-advancedRole'
     },
     selectRoleButtonInCollaboratorInformation: {
-      selector: '//button[contains(@class, "files-collaborators-role-button")]',
-      locateStrategy: 'xpath'
+      selector: '#files-collaborators-role-button'
     },
     roleDropdownInCollaboratorInformation: {
       selector: '//div[contains(@id, "files-collaborators-roles-dropdown")]',
@@ -680,8 +679,7 @@ module.exports = {
     },
     roleButtonInDropdown: {
       // the translate bit is to make it case-insensitive
-      selector:
-        '//ul[contains(@class,"oc-autocomplete-suggestion-list")]//span[translate(.,"ABCDEFGHJIKLMNOPQRSTUVWXYZ","abcdefghjiklmnopqrstuvwxyz") ="%s"]',
+      selector: '//span[@id="files-role-%s"]',
       locateStrategy: 'xpath'
     },
     permissionCheckbox: {

From 0e2e34f40e63d3e7c6bf2ea226ebd682e2797f7e Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Thu, 22 Apr 2021 17:54:34 +0200
Subject: [PATCH 07/10] test: match advanced permissions with correct name

---
 tests/acceptance/helpers/stringHelper.js                 | 8 ++++++++
 .../pageObjects/FilesPageElement/sharingDialog.js        | 9 +++++----
 2 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 tests/acceptance/helpers/stringHelper.js

diff --git a/tests/acceptance/helpers/stringHelper.js b/tests/acceptance/helpers/stringHelper.js
new file mode 100644
index 00000000000..33ef36325c1
--- /dev/null
+++ b/tests/acceptance/helpers/stringHelper.js
@@ -0,0 +1,8 @@
+module.exports = {
+  camelize: function(str) {
+    return str.replace(/(?:^\w|[A-Z]|\b\w|\s+)/g, function(match, index) {
+      if (+match === 0) return '' // or if (/\s+/.test(match)) for white spaces
+      return index === 0 ? match.toLowerCase() : match.toUpperCase()
+    })
+  }
+}
diff --git a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
index a0f333b055e..49eda0ed88e 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js
@@ -4,6 +4,7 @@ const { COLLABORATOR_PERMISSION_ARRAY, calculateDate } = require('../../helpers/
 const { client } = require('nightwatch-api')
 const userSettings = require('../../helpers/userSettings')
 const collaboratorDialog = client.page.FilesPageElement.SharingDialog.collaboratorsDialog()
+const stringHelper = require('../../helpers/stringHelper')
 const SHARE_TYPE_STRING = {
   user: 'user',
   group: 'group',
@@ -400,10 +401,10 @@ module.exports = {
      * @returns {Promise}
      */
     changeCollaboratorRoleInDropdown: function(newRole) {
-      const newRoleButton = util.format(
-        this.elements.roleButtonInDropdown.selector,
-        newRole.toLowerCase()
-      )
+      newRole = newRole === 'Advanced permissions' ? 'advancedRole' : newRole
+      newRole = stringHelper.camelize(newRole)
+
+      const newRoleButton = util.format(this.elements.roleButtonInDropdown.selector, newRole)
       return this.waitForElementVisible('@selectRoleButtonInCollaboratorInformation')
         .click('@selectRoleButtonInCollaboratorInformation')
         .useXpath()

From ab6ea22a74bc79f64a1038499d064784078cb719 Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Fri, 23 Apr 2021 09:33:48 +0200
Subject: [PATCH 08/10] remove tag duplication

---
 .../src/components/Collaborators/CollaboratorsEditOptions.vue   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
index 8491142ad43..587c003b679 100644
--- a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
+++ b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
@@ -1,6 +1,6 @@
 <template>
   <oc-grid gutter="small" child-width="1-1">
-    <label tag="label" class="oc-label">
+    <label class="oc-label">
       {{ $gettext('Role') }}
       <oc-select
         id="files-collaborators-role-button"

From 447230fbe357cbcdb2ddd74c5a1ad7f0ee1603da Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Fri, 23 Apr 2021 09:59:35 +0200
Subject: [PATCH 09/10] refactor: move oc-select out of the label

---
 .../CollaboratorsEditOptions.vue              | 41 +++++++++----------
 .../PublicLinksSidebar/EditPublicLink.vue     | 35 ++++++++--------
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
index 587c003b679..6c0b53e33e6 100644
--- a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
+++ b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
@@ -1,33 +1,32 @@
 <template>
-  <oc-grid gutter="small" child-width="1-1">
-    <label class="oc-label">
-      {{ $gettext('Role') }}
-      <oc-select
-        id="files-collaborators-role-button"
-        v-model="selectedRole"
-        :options="roles"
-        :clearable="false"
-        label="label"
-      >
-        <template v-slot:option="option">
-          <role-item :role="option" />
-        </template>
-        <template #no-options v-translate>
-          No matching role found
-        </template>
-      </oc-select>
-    </label>
+  <div>
+    <translate tag="label" for="files-collaborators-role-button" class="oc-label">Role</translate>
+    <oc-select
+      v-model="selectedRole"
+      input-id="files-collaborators-role-button"
+      :options="roles"
+      :clearable="false"
+      label="label"
+    >
+      <template v-slot:option="option">
+        <role-item :role="option" />
+      </template>
+      <template #no-options v-translate>
+        No matching role found
+      </template>
+    </oc-select>
     <template v-if="$_ocCollaborators_hasAdditionalPermissions">
-      <label v-if="selectedRole.name !== 'advancedRole'" class="oc-label">
+      <label v-if="selectedRole.name !== 'advancedRole'" class="oc-label oc-mt-s">
         <translate>Additional permissions</translate>
       </label>
       <additional-permissions
         :available-permissions="selectedRole.additionalPermissions"
         :collaborators-permissions="collaboratorsPermissions"
+        :class="{ 'oc-mt-s': selectedRole.name === 'advancedRole' }"
         @permissionChecked="checkAdditionalPermissions"
       />
     </template>
-    <div v-if="expirationSupported">
+    <div v-if="expirationSupported" class="oc-mt-m">
       <label for="files-collaborators-collaborator-expiration-input">
         <translate>Expiration date</translate>
         <translate v-if="expirationDateEnforced" tag="em">(required)</translate>
@@ -51,7 +50,7 @@
         />
       </div>
     </div>
-  </oc-grid>
+  </div>
 </template>
 
 <script>
diff --git a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
index ee7154fbe04..8fc6e999e45 100644
--- a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
+++ b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
@@ -13,23 +13,24 @@
       <div class="oc-mb">
         <oc-text-input id="oc-files-file-link-name" v-model="name" :label="$gettext('Name')" />
       </div>
-      <label tag="label" class="oc-label oc-mb">
-        {{ $gettext('Role') }}
-        <oc-select
-          id="files-file-link-role-button"
-          v-model="selectedRole"
-          :options="roles"
-          :clearable="false"
-          label="label"
-        >
-          <template v-slot:option="option">
-            <role-item :role="option" />
-          </template>
-          <template #no-options v-translate>
-            No matching role found
-          </template>
-        </oc-select>
-      </label>
+      <translate tag="label" for="files-file-link-role-button" class="oc-label">
+        Role
+      </translate>
+      <oc-select
+        v-model="selectedRole"
+        input-id="files-file-link-role-button"
+        :options="roles"
+        :clearable="false"
+        label="label"
+        class="oc-mb"
+      >
+        <template v-slot:option="option">
+          <role-item :role="option" />
+        </template>
+        <template #no-options v-translate>
+          No matching role found
+        </template>
+      </oc-select>
       <div class="oc-mb uk-grid-small uk-flex" uk-grid>
         <div v-if="$_expirationDate" class="uk-width-1-1 uk-width-2-5@m">
           <div class="uk-position-relative">

From 0c7f6802cb8fa57a75254b499a3f097a53185132 Mon Sep 17 00:00:00 2001
From: Lukas Hirt <lukashirt@mail.com>
Date: Fri, 23 Apr 2021 10:19:08 +0200
Subject: [PATCH 10/10] tests: use select wrapper selector

---
 .../src/components/Collaborators/CollaboratorsEditOptions.vue   | 1 +
 .../src/components/PublicLinksSidebar/EditPublicLink.vue        | 2 +-
 .../pageObjects/FilesPageElement/publicLinksDialog.js           | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
index 6c0b53e33e6..e6cd537c4b0 100644
--- a/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
+++ b/packages/web-app-files/src/components/Collaborators/CollaboratorsEditOptions.vue
@@ -4,6 +4,7 @@
     <oc-select
       v-model="selectedRole"
       input-id="files-collaborators-role-button"
+      class="files-collaborators-role-button-wrapper"
       :options="roles"
       :clearable="false"
       label="label"
diff --git a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
index 8fc6e999e45..d54d2a4473e 100644
--- a/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
+++ b/packages/web-app-files/src/components/PublicLinksSidebar/EditPublicLink.vue
@@ -22,7 +22,7 @@
         :options="roles"
         :clearable="false"
         label="label"
-        class="oc-mb"
+        class="oc-mb files-file-link-role-button-wrapper"
       >
         <template v-slot:option="option">
           <role-item :role="option" />
diff --git a/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js b/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js
index b06c8c8c2da..a328bd8646d 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js
@@ -433,7 +433,7 @@ module.exports = {
       selector: '#files-sidebar-private-link-icon-copied'
     },
     publicLinkRoleSelectionDropdown: {
-      selector: '//div[@id="files-file-link-role-button"]//span[.="%s"]',
+      selector: '//div[contains(@class, "files-file-link-role-button-wrapper")]//span[.="%s"]',
       locateStrategy: 'xpath'
     }
   }