Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update rich-workspace visibility #35474

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Conversation

luka-nextcloud
Copy link
Contributor

@luka-nextcloud luka-nextcloud commented Nov 29, 2022

With the new flow to create rich workspace Readme files described in nextcloud/text#3209 we need a hook to be able to show or hide the entry in the new files menu depending on the current folder. For that reason this PR adds a callback that can be provided to show/hide when the menu is shown.

Required for nextcloud/text#3291

Checklist

@max-nextcloud

This comment was marked as off-topic.

Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one more thing that I haven't thought of upfront that would be that we don't need an input for the folder description. The new file action should trigger directly on click.

Something like this seems to do the trick there:

diff --git a/apps/files/js/newfilemenu.js b/apps/files/js/newfilemenu.js
index e38801ac6b4..1d618a1a8c6 100644
--- a/apps/files/js/newfilemenu.js
+++ b/apps/files/js/newfilemenu.js
@@ -81,10 +81,18 @@
                        if (action === 'upload') {
                                OC.hideMenus();
                        } else {
-                               event.preventDefault();
-                               this.$el.find('.menuitem.active').removeClass('active');
-                               $target.addClass('active');
-                               this._promptFileName($target);
+                               var actionItem = _.filter(this._menuItems, function(item) {
+                                       return item.id === action
+                               }).pop();
+                               if (typeof actionItem.useInput === 'undefined' || actionItem.useInput === true) {
+                                       event.preventDefault();
+                                       this.$el.find('.menuitem.active').removeClass('active');
+                                       $target.addClass('active');
+                                       this._promptFileName($target);
+                               } else {
+                                       actionItem.actionHandler();
+                                       OC.hideMenus();
+                               }
                        }
                },

@@ -204,7 +212,8 @@
                                fileType: actionSpec.fileType,
                                actionHandler: actionSpec.actionHandler,
                                checkFilename: actionSpec.checkFilename,
-                               shouldShow: actionSpec.shouldShow || (() => true)
+                               shouldShow: actionSpec.shouldShow,
+                               useInput: actionSpec.useInput,
                        });
                },

@luka-nextcloud luka-nextcloud force-pushed the feature/rich-workspace-visibility branch from 449d2ad to 5b4d896 Compare December 15, 2022 09:57
@luka-nextcloud luka-nextcloud force-pushed the feature/rich-workspace-visibility branch from 9b234d7 to ead4d98 Compare December 22, 2022 08:56
@juliusknorr juliusknorr added enhancement 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 22, 2022
Copy link
Contributor

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all looks good. Just one confusion with the shouldShow parameter.

apps/files/js/newfilemenu.js Show resolved Hide resolved
Signed-off-by: Luka Trovic <luka@nextcloud.com>
@luka-nextcloud luka-nextcloud force-pushed the feature/rich-workspace-visibility branch from 73375c7 to cacffec Compare January 30, 2023 19:27
Copy link
Contributor

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see this finalized.
I still disagree about the way shouldShow is handled. But there's no point in arguing about this forever.

@juliusknorr
Copy link
Member

Failures unrelated.

@juliusknorr juliusknorr merged commit 9f2495c into master Jan 31, 2023
@juliusknorr juliusknorr deleted the feature/rich-workspace-visibility branch January 31, 2023 08:55
@blizzz blizzz mentioned this pull request Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants