-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this 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,
});
},
449d2ad
to
5b4d896
Compare
9b234d7
to
ead4d98
Compare
ead4d98
to
73375c7
Compare
There was a problem hiding this 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.
Signed-off-by: Luka Trovic <luka@nextcloud.com>
73375c7
to
cacffec
Compare
There was a problem hiding this 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.
Failures unrelated. |
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