From 580f5ff11442ce19a02cbe22f126d5515da8c42d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Julian=20M=C3=BCller?= <julimueller1998@gmail.com>
Date: Tue, 14 Apr 2020 09:35:23 +0200
Subject: [PATCH] Removed favorite button from file list and added it instead
 in action dropdown

---
 apps/files/src/components/AllFilesList.vue    | 19 ------
 apps/files/src/components/FileDetails.vue     | 10 ++-
 apps/files/src/components/FileList.vue        |  8 +--
 .../FilesLists/RowActionsDropdown.vue         |  6 +-
 apps/files/src/fileactions.js                 | 48 ++++++++++++---
 changelog/unreleased/3336                     |  8 +++
 .../FilesPageElement/fileActionsMenu.js       | 43 +++++++++++++
 .../pageObjects/FilesPageElement/filesList.js | 41 ++++++++++---
 .../pageObjects/FilesPageElement/filesRow.js  | 61 -------------------
 .../stepDefinitions/filesContext.js           | 10 +--
 10 files changed, 142 insertions(+), 112 deletions(-)
 create mode 100644 changelog/unreleased/3336

diff --git a/apps/files/src/components/AllFilesList.vue b/apps/files/src/components/AllFilesList.vue
index 128438b828a..06dbccb5679 100644
--- a/apps/files/src/components/AllFilesList.vue
+++ b/apps/files/src/components/AllFilesList.vue
@@ -2,10 +2,6 @@
   <file-list :fileData="fileData" id="files-list" :loading="loadingFolder" :actions="actions" :compactMode="_sidebarOpen"
       :isActionEnabled="isActionEnabled">
     <template #headerColumns>
-      <div v-if="!publicPage()">
-        <span class="oc-visually-hidden" v-text="favoritesHeaderText" />
-        <oc-star id="files-table-header-star" aria-hidden="true" class="uk-display-block uk-disabled" />
-      </div>
       <div class="uk-text-truncate uk-text-meta uk-width-expand" ref="headerNameColumn" >
         <sortable-column-header
           @click="toggleSort('name')"
@@ -44,9 +40,6 @@
       </div>
     </template>
     <template #rowColumns="{ item, index }">
-      <div v-if="!publicPage()">
-        <oc-star class="uk-display-block" @click.native.stop="toggleFileFavorite(item)" :shining="item.starred" />
-      </div>
       <div class="uk-text-truncate uk-width-expand" :ref="index === 0 ? 'firstRowNameColumn' : null">
         <file-item
           @click.native.stop="item.type === 'folder' ? navigateTo(item.path.substr(1)) : openFileActionBar(item)"
@@ -141,11 +134,6 @@ export default {
     FileActions
   ],
   props: ['fileData', 'starsEnabled', 'checkboxEnabled', 'dateEnabled', 'parentFolder'],
-  data () {
-    return {
-      favoritesHeaderText: this.$gettext('Favorites')
-    }
-  },
   computed: {
     ...mapState(['route']),
     ...mapGetters('Files', [
@@ -237,7 +225,6 @@ export default {
   methods: {
     ...mapActions('Files', [
       'loadFolder',
-      'markFavorite',
       'setHighlightedFile'
     ]),
 
@@ -289,12 +276,6 @@ export default {
         })
       })
     },
-    toggleFileFavorite (item) {
-      this.markFavorite({
-        client: this.$client,
-        file: item
-      })
-    },
 
     isActionEnabled (item, action) {
       return action.isEnabled(item, this.parentFolder)
diff --git a/apps/files/src/components/FileDetails.vue b/apps/files/src/components/FileDetails.vue
index d378a9aefb5..9fa8cb125a2 100644
--- a/apps/files/src/components/FileDetails.vue
+++ b/apps/files/src/components/FileDetails.vue
@@ -9,7 +9,7 @@
           <span id="files-sidebar-item-name" class="uk-margin-small-right uk-text-bold" v-text="highlightedFile.name" />
         </div>
         <div v-if="$route.name !== 'files-shared-with-others'">
-          <oc-star v-if="!publicPage()" class="uk-inline" :shining="highlightedFile.starred"/>
+          <oc-star v-if="!publicPage()" @click.native.stop="toggleFileFavorite(highlightedFile)" class="uk-inline" :shining="highlightedFile.starred"/>
           <template v-if="highlightedFile.size > -1">
             {{ highlightedFile.size | fileSize }},
           </template>
@@ -73,13 +73,19 @@ export default {
     this.activeTab = this.defaultTab
   },
   methods: {
-    ...mapActions('Files', ['deleteFiles']),
+    ...mapActions('Files', ['deleteFiles', 'markFavorite']),
     ...mapActions(['showMessage']),
     close () {
       this.$emit('reset')
     },
     showSidebar (app) {
       this.activeTab = app
+    },
+    toggleFileFavorite (file) {
+      this.markFavorite({
+        client: this.$client,
+        file: file
+      })
     }
   }
 }
diff --git a/apps/files/src/components/FileList.vue b/apps/files/src/components/FileList.vue
index 5096f405e32..9774f2cb1de 100644
--- a/apps/files/src/components/FileList.vue
+++ b/apps/files/src/components/FileList.vue
@@ -160,7 +160,7 @@ export default {
     }
   },
   methods: {
-    ...mapActions('Files', ['loadFolder', 'markFavorite',
+    ...mapActions('Files', ['loadFolder',
       'setHighlightedFile', 'setPublicLinkPassword',
       'resetFileSelection', 'addFileSelection', 'removeFileSelection', 'toggleFileSelection'
     ]),
@@ -168,12 +168,6 @@ export default {
     labelSelectSingleItem (item) {
       return this.$gettextInterpolate(this.labelSelectSingleItemText, { name: item.name, type: item.type }, true)
     },
-    toggleFileFavorite (item) {
-      this.markFavorite({
-        client: this.$client,
-        file: item
-      })
-    },
     $_ocFileName (item) {
       if (this.$route.name === 'files-favorites') {
         const pathSplit = item.path.substr(1).split('/')
diff --git a/apps/files/src/components/FilesLists/RowActionsDropdown.vue b/apps/files/src/components/FilesLists/RowActionsDropdown.vue
index 4f9aebeaa51..bc4060aa9a3 100644
--- a/apps/files/src/components/FilesLists/RowActionsDropdown.vue
+++ b/apps/files/src/components/FilesLists/RowActionsDropdown.vue
@@ -9,14 +9,14 @@
     class="uk-open uk-drop-stack"
   >
     <ul class="uk-list">
-      <li v-for="action in actions" :key="action.ariaLabel">
+      <li v-for="action in actions" :key="action.ariaLabel(item)">
         <oc-button
           class="uk-width-1-1"
           @click.native.stop="action.handler(item, action.handlerData); actionClicked()"
           :icon="action.icon"
-          :ariaLabel="action.ariaLabel"
+          :ariaLabel="action.ariaLabel(item)"
         >
-          {{ action.ariaLabel }}
+          {{ action.ariaLabel(item) }}
         </oc-button>
       </li>
     </ul>
diff --git a/apps/files/src/fileactions.js b/apps/files/src/fileactions.js
index cb5d6d7ce5b..c5676ba1104 100644
--- a/apps/files/src/fileactions.js
+++ b/apps/files/src/fileactions.js
@@ -7,14 +7,16 @@ export default {
       'deleteDialogOpen', 'deleteDialogSelectedFiles',
       'actionsInProgress', 'activeFiles', 'selectedFiles', 'highlightedFile'
     ]),
-    ...mapGetters(['capabilities', 'fileSideBars']),
+    ...mapGetters(['capabilities', 'fileSideBars', 'isAuthenticated']),
     // Files lists
     actions () {
       const actions = [
         {
           icon: 'edit',
-          ariaLabel: this.$gettext('Rename'),
           handler: this.promptFileRename,
+          ariaLabel: () => {
+            return this.$gettext('Rename')
+          },
           isEnabled: function (item, parent) {
             if (parent && !parent.canRename()) {
               return false
@@ -25,21 +27,38 @@ export default {
         {
           icon: 'file_download',
           handler: this.downloadFile,
-          ariaLabel: this.$gettext('Download'),
+          ariaLabel: () => {
+            return this.$gettext('Download')
+          },
           isEnabled: function (item) {
             return item.canDownload()
           }
         },
         {
           icon: 'delete',
-          ariaLabel: this.$gettext('Delete'),
           handler: this.deleteFile,
+          ariaLabel: () => {
+            return this.$gettext('Delete')
+          },
           isEnabled: function (item, parent) {
             if (parent && !parent.canBeDeleted()) {
               return false
             }
             return item.canBeDeleted()
           }
+        },
+        {
+          icon: 'star',
+          handler: this.toggleFileFavorite,
+          ariaLabel: (item) => {
+            if (item.starred) {
+              return this.$gettext('Unmark as favorite')
+            }
+            return this.$gettext('Mark as favorite')
+          },
+          isEnabled: () => {
+            return this.isAuthenticated
+          }
         }
       ]
       // FIXME: we are assuming this.fileSideBars and others are available on object
@@ -50,7 +69,9 @@ export default {
         if (sideBar.quickAccess) {
           actions.push({
             icon: sideBar.quickAccess.icon,
-            ariaLabel: sideBar.quickAccess.ariaLabel,
+            ariaLabel: (item) => {
+              return sideBar.quickAccess.ariaLabel
+            },
             handler: this.openSideBar,
             handlerData: sideBar.app,
             isEnabled: function (item) {
@@ -66,7 +87,8 @@ export default {
   methods: {
     ...mapActions('Files', [
       'renameFile', 'promptFileRename', 'closePromptFileRename',
-      'deleteFiles', 'promptFileDelete', 'closePromptFileDelete'
+      'deleteFiles', 'promptFileDelete', 'closePromptFileDelete',
+      'markFavorite'
     ]),
     ...mapActions(['showMessage']),
 
@@ -112,7 +134,19 @@ export default {
         this.closePromptFileRename()
       })
     },
-
+    toggleFileFavorite (file) {
+      this.markFavorite({
+        client: this.$client,
+        file: file
+      }).catch(() => {
+        const translated = this.$gettext('Error while starring "%{file}"')
+        const title = this.$gettextInterpolate(translated, { file: file.name }, true)
+        this.showMessage({
+          title: title,
+          status: 'danger'
+        })
+      })
+    },
     cancelDeleteFile () {
       this.closePromptFileDelete()
     },
diff --git a/changelog/unreleased/3336 b/changelog/unreleased/3336
new file mode 100644
index 00000000000..c394bc356f5
--- /dev/null
+++ b/changelog/unreleased/3336
@@ -0,0 +1,8 @@
+Change: Removed favorite button from file list and added it in the sidebar
+
+We've removed the favorite star button in the file list and added instead a functionality 
+to the before non-working star button in the file's sidebar. We also added a new action in 
+the file action dropdown menu which allows you to toggle the favorite status of your file.
+
+https://github.com/owncloud/phoenix/issues/1987
+https://github.com/owncloud/phoenix/pull/3336
diff --git a/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js b/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js
index 35507ef5de8..f374a4230f2 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/fileActionsMenu.js
@@ -7,6 +7,8 @@ module.exports = {
     FileAction: Object.freeze({
       download: 'download',
       delete: 'delete',
+      favorite: 'favorite',
+      unmarkFavorite: 'unmarkFavorite',
       restore: 'restore',
       share: 'share',
       rename: 'rename',
@@ -90,6 +92,39 @@ module.exports = {
 
       return this
     },
+    /**
+     * mark as favorite resource using fileActions 'favorite' button
+     * @returns {Promise<*>}
+     */
+    favorite: function () {
+      return this.performFileAction(this.FileAction.favorite)
+    },
+    /**
+     * unmark as favorite resource using fileActions 'favorite' button
+     * @returns {Promise<*>}
+     */
+    unmarkFavorite: function () {
+      return this.performFileAction(this.FileAction.unmarkFavorite)
+    },
+    /**
+     * checks whether unmark as favorite button of given file-row is present
+     *
+     * @returns {Promise<boolean>}
+     */
+    isUnmarkFavoriteBtnPresent: async function () {
+      const unmarkFavoriteButtonXpath = this.elements.unmarkFavoriteButtonInFileRow.selector
+      let isPresent = true
+      await this
+        .api.page.FilesPageElement.appSideBar().closeSidebar(100)
+      await this
+        .api.elements(
+          this.elements.unmarkFavoriteButtonXpath.locateStrategy,
+          unmarkFavoriteButtonXpath,
+          (result) => {
+            isPresent = result.value.length > 0
+          })
+      return isPresent
+    },
     /**
      * opens sharing dialog for given resource
      * assumes filesAction menu for the resource to be opened
@@ -172,6 +207,14 @@ module.exports = {
       selector: '//button[@aria-label="Download"]',
       locateStrategy: 'xpath'
     },
+    favoriteButtonInFileRow: {
+      selector: '//button[@aria-label="Mark as favorite"]',
+      locateStrategy: 'xpath'
+    },
+    unmarkFavoriteButtonInFileRow: {
+      selector: '//button[@aria-label="Unmark as favorite"]',
+      locateStrategy: 'xpath'
+    },
     restoreButtonInFileRow: {
       selector: '//button[@aria-label="Restore"]',
       locateStrategy: 'xpath'
diff --git a/tests/acceptance/pageObjects/FilesPageElement/filesList.js b/tests/acceptance/pageObjects/FilesPageElement/filesList.js
index a6c0122180b..9094969141b 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/filesList.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/filesList.js
@@ -66,6 +66,39 @@ module.exports = {
         .rename(toName, expectToSucceed)
       return this
     },
+    /**
+     * @param {string} path
+     * @return {Promise<module.exports.commands>}
+     */
+    markFavorite: async function (path) {
+      await this.waitForFileVisible(path)
+      await filesRow
+        .openFileActionsMenu(path)
+        .favorite()
+      return this
+    },
+    /**
+     * @param {string} path
+     * @return {Promise<module.exports.commands>}
+     */
+    unmarkFavorite: async function (path) {
+      await this.waitForFileVisible(path)
+      await filesRow
+        .openFileActionsMenu(path)
+        .unmarkFavorite()
+      return this
+    },
+    /**
+     * @param {string} path
+     * @return {Promise<module.exports.commands>}
+     */
+    isMarkedFavorite: async function (path) {
+      await this.waitForFileVisible(path)
+      await filesRow
+        .openFileActionsMenu(path)
+        .isUnmarkFavoriteBtnPresent()
+      return this
+    },
     /**
      * @param {string} element
      * @param {string} elementType
@@ -713,14 +746,6 @@ module.exports = {
       selector: '//span[contains(@class, "file-row-name") and (@data-preview-loaded="true" or not(@data-preview-loaded))]',
       locateStrategy: 'xpath'
     },
-    notMarkedFavoriteInFileRow: {
-      selector: '//span[contains(@class, "oc-star-dimm")]',
-      locateStrategy: 'xpath'
-    },
-    markedFavoriteInFileRow: {
-      selector: '//span[contains(@class, "oc-star-shining")]',
-      locateStrategy: 'xpath'
-    },
     collaboratorsInFileRow: {
       selector: '//*[contains(@class, "file-row-collaborator-name")]',
       locateStrategy: 'xpath'
diff --git a/tests/acceptance/pageObjects/FilesPageElement/filesRow.js b/tests/acceptance/pageObjects/FilesPageElement/filesRow.js
index 71e10175cc3..e74d49310bf 100644
--- a/tests/acceptance/pageObjects/FilesPageElement/filesRow.js
+++ b/tests/acceptance/pageObjects/FilesPageElement/filesRow.js
@@ -3,59 +3,6 @@ const filesList = client.page.FilesPageElement.filesList()
 
 module.exports = {
   commands: {
-
-    /**
-     *
-     * @param {string} path
-     */
-    markAsFavorite: async function (path) {
-      await filesList.waitForFileVisible(path)
-
-      const favoriteIconButton = filesList.getFileRowSelectorByFileName(path) +
-        this.elements.notMarkedFavoriteInFileRow.selector
-
-      await this.initAjaxCounters()
-        .useXpath()
-        .click(favoriteIconButton)
-        .waitForOutstandingAjaxCalls()
-        .useCss()
-
-      return this
-    },
-    /**
-     *
-     * @param {string} path
-     *
-     * @return {Promise<boolean>}
-     */
-    isMarkedFavorite: async function (path) {
-      let visible = false
-      const markedFavoriteIcon = filesList.getFileRowSelectorByFileName(path) +
-        this.elements.markedFavoriteInFileRow.selector
-      await filesList.waitForFileVisible(path)
-      await this.api
-        .element('xpath', markedFavoriteIcon, (result) => {
-          visible = !!result.value.ELEMENT
-        })
-      return visible
-    },
-    /**
-     * @param {string} path
-     */
-    unmarkFavorite: async function (path) {
-      const unFavoriteBtn = filesList.getFileRowSelectorByFileName(path) +
-        this.elements.markedFavoriteInFileRow.selector
-
-      await filesList.waitForFileVisible(path)
-
-      await this.initAjaxCounters()
-        .useXpath()
-        .click(unFavoriteBtn)
-        .waitForOutstandingAjaxCalls()
-        .useCss()
-
-      return this
-    },
     /**
      * Get Selector for File Actions expander
      *
@@ -86,14 +33,6 @@ module.exports = {
     }
   },
   elements: {
-    notMarkedFavoriteInFileRow: {
-      selector: '//span[contains(@class, "oc-star-dimm")]',
-      locateStrategy: 'xpath'
-    },
-    markedFavoriteInFileRow: {
-      selector: '//span[contains(@class, "oc-star-shining")]',
-      locateStrategy: 'xpath'
-    },
     fileActionsButtonInFileRow: {
       selector: '//button[contains(@class, "files-list-row-show-actions")]',
       locateStrategy: 'xpath'
diff --git a/tests/acceptance/stepDefinitions/filesContext.js b/tests/acceptance/stepDefinitions/filesContext.js
index 0939d68da8a..075da332583 100644
--- a/tests/acceptance/stepDefinitions/filesContext.js
+++ b/tests/acceptance/stepDefinitions/filesContext.js
@@ -319,15 +319,15 @@ When('the user renames the following file/folder using the webUI', async functio
 })
 
 Given('the user has marked file/folder {string} as favorite using the webUI', function (path) {
-  return client.page.FilesPageElement.filesRow().markAsFavorite(path)
+  return client.page.FilesPageElement.filesList().markFavorite(path)
 })
 
 When('the user marks file/folder {string} as favorite using the webUI', function (path) {
-  return client.page.FilesPageElement.filesRow().markAsFavorite(path)
+  return client.page.FilesPageElement.filesList().markFavorite(path)
 })
 
 When('the user unmarks the favorited file/folder {string} using the webUI', function (path) {
-  return client.page.FilesPageElement.filesRow().unmarkFavorite(path)
+  return client.page.FilesPageElement.filesList().unmarkFavorite(path)
 })
 
 Then('there should be no files/folders/resources listed on the webUI', async function () {
@@ -628,14 +628,14 @@ Then('as {string} these files/folders/resources should be listed in the folder {
 })
 
 Then('file/folder {string} should be marked as favorite on the webUI', function (path) {
-  return client.page.FilesPageElement.filesRow().isMarkedFavorite(path)
+  return client.page.FilesPageElement.filesList().isMarkedFavorite(path)
     .then(result => {
       assert.strictEqual(result, true, `${path} expected to be favorite but was not`)
     })
 })
 
 Then('file/folder {string} should not be marked as favorite on the webUI', function (path) {
-  return client.page.FilesPageElement.filesRow().isMarkedFavorite(path)
+  return client.page.FilesPageElement.filesList().isMarkedFavorite(path)
     .then(result => {
       assert.strictEqual(result, false, `not expected ${path} to be favorite but was`)
     })