Mail: Missing close or choose button in file picker dialog since 0.7.5

Created on 22 Nov 2017 · 20Comments · Source: nextcloud/mail

Hey,
the file picker dialog is missing a close or save button:
2017-11-22-125411_swaygrab
But maybe this is related to core?
I'm using Nextcloud Beta 13 and the latest mail app (from git).

Best regards,
Jonas

bug design help wanted regression

Source

onny

All 20 comments

But maybe this is related to core?

Yup, that is part of the Nextcloud server. Could you please file a ticket in the server repo? We should fix that for the nc13 release.

Thanks for reporting.

ChristophWurst on 22 Nov 2017

As discovered in https://github.com/nextcloud/server/issues/7282, these bugs seem only to occur in this app.

@skjnldsv @jancborchardt any idea what's wrong? Could one of you have a look? Thanks.

bildschirmfoto von 2017-11-27 08-35-19

ChristophWurst on 27 Nov 2017

Same here, NC 12.0.3 after update to 0.7.5
screen shot 2017-11-27 at 8 25 27 pm

laurentiu2 on 27 Nov 2017

Updated the title since this affects both NC12 and NC13

pixelipo on 29 Nov 2017

What a pity the app can 'break' in such a way.

buttle on 12 Dec 2017

What a pity the app can 'break' in such a way.

But hey, it's open source and I encourage anybody interested in taking a look at the source code and trying to fix it so that we have a working filepicker again 😉

Do you know js?

I already gave it a shot but couldn't figure out what's been broken. Hence this will remain unfixed until someone jumps in and has a closer look.

ChristophWurst on 12 Dec 2017

I know a little js. But I don't have much time on my hands, unfortunately.

buttle on 12 Dec 2017

@ChristophWurst I can't test since I'm unable to set up Mail locally, but I see this issue popping up in console:
TypeError: this._setOptionDisabled is not a function build.js:20:4462

Since _setOption in core/js/jquery.ocdialog.js is responsible for painting missing DOM elements, it seems probable that there is some conflict there... Maybe one of the vendor JS is overriding this?

pixelipo on 12 Dec 2017

@ChristophWurst I can't test since I'm unable to set up Mail locally, but I see this issue popping up in console:

Let me know where you've got stuck. How did you set it up?

Since _setOption in core/js/jquery.ocdialog.js is responsible for painting missing DOM elements, it seems probable that there is some conflict there... Maybe one of the vendor JS is overriding this?

Good hint. We're shipping jquery ui with this app too, maybe that's the problem.

ChristophWurst on 12 Dec 2017

Since _setOption in core/js/jquery.ocdialog.js is responsible for painting missing DOM elements, it seems probable that there is some conflict there... Maybe one of the vendor JS is overriding this?

Bingo! $.ui.version returns 0.10.0 when I browse the files app, but 0.12.1 when browsing the mail app's page. So jquery-ui registers itself on the global jquery somehow. Awesome, right? ☹️

ChristophWurst on 12 Dec 2017

😕3

Can you just do away with the jquery you ship?

buttle on 12 Dec 2017

One possible solution would be to not use the global jQuery but or own. We'd have to set up the ajax core to use the nc request token but other than that it should work and prevent overriding jquery-ui in the global namespace.

ChristophWurst on 13 Dec 2017

👍1

Without knowing the code, it seems to make more sense to use the global library as it is already there, globally. (my 2 cents)

buttle on 13 Dec 2017

With knowing the code, it's necessary for apps to ship certain deps as the ones in the global namespace are pretty much outdated.

ChristophWurst on 13 Dec 2017

@buttle to give you a bit more context: the main problem here is that if you write a front-end app for Nextcloud you either rely on the libraries already available in the Nextcloud server or ship your own. The former seems easy, but the dependency of your code on those libraries become unclear. Can you expect the libs to keep a stable API, as in they don't break on upgrades? What if I pull in an additional library, that depends on one of those already available ones - how do I know those versions are compatible? Based on that, it's also really hard to update the server libraries as it might break apps. And thus, historically, those libs weren't really touched unless it was really necessary.
If you take the second approach of shipping your own libs, you're completely independent. You know which version you've got and nobody can change that. They are the exact same, no matter where you deploy the app and no matter which other apps are loading arbitrary script on a Nextcloud page. While it sounds like a bit of overhead using multiple version of the same lib, tools like webpack are really good at compressing js code and thus the app's js bundle is relatively small.

I hope that sheds some light on the situation and why I'm not blindly removing jquery-ui from this app.

ChristophWurst on 13 Dec 2017

I really hope we can ditch jquery-ui (globally) soon...

pixelipo on 13 Dec 2017

Not fully fixed yet.

ChristophWurst on 11 Jan 2018

Ok thanks :-)