Add OCA.Files.Settings for Files navigation settings

[gary-kim]

Resolves#16602

Most of this is taken from #15719 so thank you @skjnldsv!

cc @ChristophWurst for Recommendations app
Example use here: nextcloud/recommendations#168 (Can be tested with this)
Also for Text: nextcloud/text#708

window.addEventListener('DOMContentLoaded',()=>{if(OCA.Files&&OCA.Files.Settings){OCA.Files.Settings.register(newOCA.Files.Settings.Setting('recommendations',{el: ()=>{// return an unmounted element.returndocument.createElement('a');},open: ()=>{// Callback called when settings is opened},close: ()=>{// Callback called when settings is closed}}))}})

Signed-off-by: Gary Kim gary@garykim.dev

[kesselb]

Cool 👍 Is the text app still able to inject or is a update required?

[skjnldsv]

You're welcome :)
Please not that we don't know what will be planned for 19, so it might conflict with some other tasks ;)

@skjnldsv whistle files to vue 😁
(a man can dream)

[gary-kim]

whistle files to vue grin

YES 😃!!!
In any case, this change is already using Vue so it shouldn't be much of a conflict.

Cool +1 Is the text app still able to inject or is a update required?

The Text app would need an update to use the new API.

[ChristophWurst]

Thanks for looking into this!

As with the sidebar, my gut feeling tells we we shouldn't make this a Vue-specific API but one that works with any framework. Yes, right now Vue is our main tool, but so was jQuery and Backbone and now we have to deal with legacy APIs that are bound to these libraries. There is no perfect API that works forever, but we can try to make it as flexible as possible.

Thus I would like to propose that we make this framework-agnostic and don't reply on a component passed. One thing that I had in mind with the sidebar as well is that if you register X (sidebar, settings, whatever) you get a mounted DOM element back. And then you simply render your UI into this element.

So, as someone who wants to adds a custom settings to files, I do something like

constel=OCA.Files.Settings.register('recommendations')constview=Vue.extend(...).mount(el)

^ that should work with most frameworks and isn't much less convenient to use. It even has the advantage that creating the component is done by your app and not the server Vue instance, hence you get $store or whatever you have registered globally. @skjnldsv ran into such an issue recently with the Talk sidebar.

Hope this makes sense. I won't be around much the next few days, but I'll happily discuss this next year :)

[skjnldsv]

constel=OCA.Files.Settings.register('recommendations')

With this you leave the mount mechanism to the app's management, but the app might not know the current settings status.
Did we changed the view of the files app, did we changed another thing (like the sidebar, you can change the file, but the sidebar doesn't change everything, they just reload the tabs and data)

I could therefore do it differently,

OCA.Files.Settings.register('recommendations',function(){returnnewVue({...}).$el})

And then every time the Settings would change it can generate the whole data itself by requesting a new instance of the registered Settings?
That's how I plan to upgrade the sidebar, but I definitely have no fixed mind on this, so I could be totally wrong ;)

[ChristophWurst]

lol, yes, that was the simplified version. I had pretty much the same in mind for cases where you need more context information. Maybe with an object, though

OCA.Files.Settings.register('recommendations',{open: ()=>{...},close: ()=>{...},})

^ allows separation of different events. A single callback would either have a single purpose for which it's called or you need to pass in this information as parameter. If you use the object style then in server you even know which callbacks are provided, should you need that information.

I'm not sure if we should return a new element every time a callback is invoked.

[gary-kim]

Alright, let's go with a more framework agnostic one!

For the sidebar, it definitely would make sense to have different events but I don't think that's necessary for the settings for which it's usually a mount then done thing. Could just make it so that, at least for settings, only el exists but it is till an object to make the API somewhat consistent, so for recommendations, it would be something like this:

OCA.Files.Settings.register('recommendations',{el: ()=>{new(Vue.extend(Settings)).$mount(document.createElement('div')).$el}});

Also, cc @juliushaertl for Text app rich workspace enabled setting.

EDIT: Just thinking, it may also make sense to have a dirChange callback for Settings. Maybe the recommendations settings should only be visible in the root directory?

[juliusknorr]

EDIT: Just thinking, it may also make sense to have a dirChange callback for Settings. Maybe the recommendations settings should only be visible in the root directory?

Let's keep the settings independent from the current directory. Otherwise it would be confusing if they are sometimes there and sometimes not. 😉

[skjnldsv]

	el: ()=>{new(Vue.extend(Settings)).$mount(document.createElement('div')).$el}

No, you don't need to mount or do anything, this is exactly what @ChristophWurst suggested :)
With the open/close. As long as the open return an unmounted dom it's fine :)

The close would just be unmounting stuff, and we could possibly need this as we might have different settings for different files views? (sharing, deleted files...)
If so, we still need to unmount the elements so they're not stacking and creating a memory leak.

Now that I think of it, why are you all using Vue.extend instead of creating a new Vue instance? I feel like I'm missing something :)

[gary-kim]

As long as the open return an unmounted dom it's fine

Oh wait,

new(Vue.extend(Settings)).$mount().$el

also works doesn't it. 🙈

The close would just be unmounting stuff, and we could possibly need this as we might have different settings for different files views?

Oh, that makes sense, although it should be possible to unmount without needing that to be handled for most cases? Still can't hurt to have it available.

It may make sense to have options el, open, close. So el should return a unmounted dom and open and close get called when first mounting and unmounting respectively.

Sorry if I'm misunderstanding something.

[skjnldsv]

It may make sense to have options el, open, close. So el should return a unmounted dom and open and close get called when first mounting and unmounting respectively.

seems about right :)
It's then just a matter of choosing the right keywords for us then, we can also use skjnldsv but not sure I'll get a consensus on this!

Oh wait,

new(Vue.extend(Settings)).$mount().$el

also works doesn't it.

Can you? I never seen the Vue.extend used in those cases.

---

I just found this: https://forum.vuejs.org/t/what-is-the-relation-or-difference-between-vue-instance-and-vue-component-instance/12439/2
So as I now understand, in the end it's basically the same.

The difference is than from 1.x to vue 2.x, they removed the option to use el in a Vue.extend

[ChristophWurst]

Looks good 👍

[jancborchardt]

Works instantly, really nice! :)

Just suggested a slight wording change over in the Recommendations pull request: nextcloud/recommendations#168

And only a small request here: Is it possible to change the order so that "Show hidden files" is last? Then it is:

• Show rich workspaces
• Show recommendations
• Show hidden files

And it corresponds to the order these elements show in the actual files view.

[rullzer]

/compile amend /