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

Make file-upload.js app agnostic #19563

Closed
oparoz opened this issue Oct 4, 2015 · 13 comments
Closed

Make file-upload.js app agnostic #19563

oparoz opened this issue Oct 4, 2015 · 13 comments

Comments

@oparoz
Copy link
Contributor

oparoz commented Oct 4, 2015

It's not too far away for the drag and drop feature, but the "new" button only works in Files, so if you want people to be able to upload from Gallery, Music, Notes, PMS, etc., that section needs to be rewritten with callbacks.

List of Files specific calls

Files.isFileNameValid(filename)
FileList.inList(filename)
FileList.lastAction
FileList.getUniqueName(newname)
FileList.checkName(name, newname, true)
FileList.getCurrentDirectory()
FileList.add()

@PVince81 @rullzer @MorrisJobke @butonic

@oparoz oparoz added this to the 9.0-next milestone Oct 4, 2015
@PVince81
Copy link
Contributor

PVince81 commented Oct 5, 2015

Yes, or the upload function needs to be encapsulated into a proper class (or classes) that can be insert into the DOM of any list/app.

@DeepDiver1975
Copy link
Member

With respect to a clean separation of the code bases - I'd rater question this approach.

We are still far away from providing a public javascript api - as a result apps should implement their own client side file uplad mechanisms.

Otherwise a change to the files js code will break all apps which are re-using the the code.

@DeepDiver1975
Copy link
Member

So as long as an app does not integrate with the files app (e.g. file actions or viewers) there should be no need in sharing js code.

@oparoz
Copy link
Contributor Author

oparoz commented Oct 5, 2015

file-upload.js in files does all the hard work:

  • Already set up to talk to jquery-file-upload
  • Configures drop zone, handlers, file selector, progress bar
  • Performs basic validation
  • Offers conflict resolution. It's ridiculous to have to rewrite our own conflict resolution dialogue and it will have to match the one in Files or there will be complaints that the design changes too much when changing app
  • Maintained. That's important, especially for security, as there is a high chance changes in core won't be tracked if the code is duplicated

Also, some of the methods I've listed should be moved to js.js. Things like isFileNameValid() or checkName()

Otherwise a change to the files js code will break all apps which are re-using the the code

Yes, but that's a problem with any API. The common part should not have to change that often. It performs basic validation and uploading tasks and apps then add their own handlers for anything specific, like findFile().
As I mentioned, it's almost there for the drag and drop feature, the FileList class has the specific handlers needed to add the rows per example.

@DeepDiver1975
Copy link
Member

I have the big share code mess in mind - we are almost blocked with any kind of enhancement just because the share dialog is reused by apps.

The solution for 8.2 is to copy the mess to the apps - next step for the app developers will be to deal with it by either maintaining the mess or implement something on their own.

I want to avoid something like that in the future - as long as we cannot guarantee a stable js api: apps shall not use it.

In addition the sidebar uses backbone and handlebars - this might not fit into all apps and would require custom code anyhow.

To me the client side of an app should be self-contained and reuse as little as possible.
Integration with the ownCloud server can either be on php side using OCP or by using http requests against public apis like WebDAV or OCS (application specific endpoints which are built using AppFramwork for sure as well - but this is app internal).

@oparoz
Copy link
Contributor Author

oparoz commented Oct 5, 2015

I have the big share code mess in mind - we are almost blocked with any kind of enhancement just because the share dialog is reused by apps.

Yes, I think it's because people started to hack on top of something which was not designed to be app agnostic (view is embedded and it's designed to try and makes changes to the Files app). It was the path of least resistance, instead of duplicating the whole thing and I predict the same thing will happen with file-upload and more.

From my pov, core shouldn't be blocked by apps, especially now that they're not included in the distribution any more. If I emulate Files methods, then it's my problem if it breaks. On the other hand, there should be no app specific code added any more to these generic methods in order to facilitate the move towards a stable JS API.

In addition the sidebar uses backbone and handlebars - this might not fit into all apps and would require custom code anyhow.
To me the client side of an app should be self-contained and reuse as little as possible.

Apps would need to use the required libraries if they want to implement those functionalities and all they should have to do is register their callbacks with those classes. Well worth the effort to have a unified UI, otherwise it's going to be a UI mess and @jancborchardt is going to commit Seppuku.

This shold be part of the GUI toolkit:

  • oc-dialogs
  • oc-dropdown
  • oc-sidebar
  • oc-sharing
  • etc.

and there should be some utility methods:

  • file-upload

Just like we have the AppFramework on the PHP side.

@DeepDiver1975
Copy link
Member

Well worth the effort to have a unified UI, otherwise it's going to be a UI mess and @jancborchardt is going to commit Seppuku.

We have to make sure this is not going to happen 🔪 🏄

@DeepDiver1975
Copy link
Member

But unified UI is not related to reuse of javascript but reuse of html elements and css rules.

@DeepDiver1975
Copy link
Member

This shold be part of the GUI toolkit:

And this is really the hard part - on the client side we are more flexible compared to the server side.

Devs are free to use what ever js libs/frameworks - and providing components which fit into Angular the same as into backbone or ember or ...... is a hell of a job.

I honestly don't see the benefit - gifted js devs will come up with their own code for the app in short time and can maintain and test the code base their own way.

@oparoz
Copy link
Contributor Author

oparoz commented Oct 5, 2015

Devs are free to use what ever js libs/frameworks - and providing components which fit into Angular the same as into backbone or ember or ...... is a hell of a job.

True, but if you provide an official toolkit, it's use it as-is or leave it. If a dev needs his app to feel integrated, then he won't be able to play with alternative technologies without having to rewrite everything (or just the logic and use the templates).

gifted js devs will come up with their own code for the app in short time and can maintain and test the code base their own way.

Sure, but it's time taken away to build new stuff and a major effort every time core is making a big change.

It's your ship, so at the end of the day, we're going to follow your rules, but I'm just laying down some arguments, in case the conversation needs to be re-initiated at some point :)

@VicDeo
Copy link
Member

VicDeo commented Oct 5, 2015

cc /me

@MorrisJobke
Copy link
Contributor

cc /me

Simply click the subscribe button ;)

(I know it called "Unsubscribe" here)

bildschirmfoto am 2015-10-05 um 21 49 17

@ghost ghost modified the milestones: 9.1-next, 9.0-current Feb 22, 2016
@PVince81 PVince81 modified the milestones: 9.1-current, 9.2-next Jun 14, 2016
@PVince81
Copy link
Contributor

More progress made in 9.2, but still some dependencies on FileList unfortunately...

@PVince81 PVince81 modified the milestones: backlog, 10.0 Dec 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants