-
Notifications
You must be signed in to change notification settings - Fork 252
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
Java Connector Update, Javascript Async Handling #21
Conversation
- added interface FileManagerI and abstract class AbstractFM and optional class RichFileManager - the only changes in RichFileManager are proper download handling and thumbnail setting instead preview - TODO: upload, ..? UPDATED - fixed content type handling in JSP files - in scripts/filemanager.js add async ajax calls and global object FileManager
- added interface FileManagerI and abstract class AbstractFM and optional class RichFileManager - the only changes in RichFileManager are proper download handling and thumbnail setting instead preview - TODO: upload, ..? UPDATED - fixed content type handling in JSP files - in scripts/filemanager.js add async ajax calls and global object FileManager
loadJS('/scripts/CodeMirror/lib/codemirror.js'); | ||
loadJS('/scripts/CodeMirror/addon/selection/active-line.js'); | ||
loadCSS('/scripts/CodeMirror/addon/display/fullscreen.css'); | ||
loadJS('/scripts/CodeMirror/addon/display/fullscreen.js'); | ||
loadJS('/scripts/CodeMirror/dynamic-mode.js'); | ||
} | ||
|
||
// added gk from simogeo, why was it removed? |
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.
Of the initial ideas of the current FM implementation is to divide client-side and server-side setting as much as possible. So "fileRoot" option now relative to the path returned in server response and doesn't depend on "serverRoot" option. So this part can be removed indeed.
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.
that seems a good idea, this would change probably client and server side, I ll update the master, if done.
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.
Yes, you could pull new changes from master.
Also read this topic, please. It's about new features of "Preview" and "Thumbnail" params in response to "getinfo" request. The solution, that we came to, is implemented and is on master.
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.
I read it, ok I try to provide an updated version tomorrow, with merged in changes from PR #23. CU
}, | ||
axis: "yx" | ||
}); | ||
|
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.
I tested upload and replace file(s), which seem to work now, although fileTree refresh seems not .. another TODO
Don't worry much about filetree. I'm going to replace it with another, more flexible, plugin and I have started to implement it already.
HI @gmkll , we worked on a java connector at the same time. I think my connector is in a more advanced stage, I use for now with Spring MVC. It needs little changes (noe need to specify path with JSP). Can we discuss about this and maybe merge our work? |
Of course, let´s discuss! As we are using simogeo fileManager for quite a while with JSP connector, I looked for an update, and thought I could achieve with a little effort to get it to work, i.e. just with the basic functionality (e.g. no editing). Now, I hope with this PR the JSP backend IS (almost?) working. Wouldn´t be the best idea to add yours as an another, a real "java" connector and keeping the jsp connector as a lightweighted version (question also to @servocoder)? For our purposes this is exactly, what we need, at least for the moment. |
I just checked quickly the state of the connector (to know if I continue mine or not). In my opinion, the Java connector should provide the same functionalities as the PHP one (including security settings, upload, thumbnails, edit, etc.). In the best case, people should just put the jar in their projects (no need to compile the class). I don’t think it's a good idea to have 2 different connectors (one light and one complete), but in the other hand, I use some JAVA 7's functionalities, if still people using JAVA 6 this can be a problem. (I don't know if you still work with 1.6). Maybe, before to go further in the conversation, I should show you what I have done. I will put the connector in a repo in the coming days. (It’s still not finished and I must try with a simple jsp project), but the code is very close. |
Thanks you both for your efforts on Java connector. I agree that Java connector should provide the same (or as close as possible) functionalities as the PHP connector. PHP connector is always up to date with new features and options from config file, we should consider it as a standard model of a connector. So let's try to implement Java connector trying to involve as much features as possible. Shall we create 2 Java connectors? I'm not aware of distinctions of Java versions and frameworks. You know this stuff better than I do. The only thing I can say is that it will be harder to maintain 2 connectors instead of one. Keep it in mind while taking a decision. |
(function($) { | ||
|
||
// keep variables in non global object | ||
var fm = {}; |
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.
I believe this object is redundant.
All variables defined inside (function($) { })
exist in the scope of that function and they aren't accessible in the global scope. Vice versa - variables defined inside (function($) { })
aren't going to overwrite global variables for some other scope. You could simply check it if you invoke config
or lg
variable in a debug console of your browser FM is inited. They are undefined
, in case you don't have globals defined with the same name of course.
So I can't see any benefit from the fm
object. Only the headache to add it for the each variable defined in the FM scope. Update your PR and remove it, if you don't have any reasonable objections, please.
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.
you are right. I initially thought it a good idea, to have one globalcontext, but I moved the fm-variable into function context, it would be just helpful to have it as a data holder object - to check all the variables, which FM is using and to easily switch context, of course. But if you think it´s better without (the reference chain is of course longer), I can remove it and update the PR.
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.
I have got your point. Indeed, it's more flexible to change context if you have all variables under the context of a single object. I don't care of a reference chain length actually, but I'm worry a bit not to miss any variable being added to vm
object. Well, let's postpone the decision on that.
Hi @servocoder, @fabriceci, my approach is so far to improve the connector step by step - I am aware of, that a lot is missing, e.g config is currently read from local config.properties only, not from provided connector config file, etc. The code is mainly just a fix of the old connector! Using spring is of course nice, but, if your webapp is not already using spring, you have to think about (a lot of) dependencies (spring-context is about 3MB - do you need other spring resources?). Actually server side configuration could be resolved rather elegantly, though. For now, the non servlet dependencies are commons.fileupload, and org.slf4j, which should be provided (I could add an pom.xml to automatically build a jar).
The PR is actually twofold: JSP connector and JS deferred mode/data holder object, which are almost independent from each other - I suggest to merge both (next week) and proceed from there in small(er) steps rather than to do a big shot. The PR stuff is in beta already tested positively and up to now is working positively. May be, @servocoder, you want to contact the other Java Connector Committer or ask others ?? |
Hi @gmkll, @fabriceci |
Btw, @th-schwarz have been already notified of this thread. |
You misunderstood, I said I use Java Spring MVC because I need to overload some parameters (and because I don't try with JSP). Like I said, the connector's code is very close to yours, with :
There is no dependencies related to spring, I will put my connector in a repo, we will be able then, to decide what we do, if you are agree. @servocoder given that the java version is the same, there is no need to have 2 connectors. |
I´ll update, but could you review #34 and update the Wiki, that I´ll better understand, what´s the current state? Expect some delay, until I fully grasp things and change duly. Thanks! |
The API have been updated. It's how it works now, but I am going to make it more standartized soon. I believe it won't require some global changes, so it shouldn't be an impediment for you to keep moving on your connector. Also I would ask you to remove |
FYI. All the logic to build path/url for |
UPDATE JAVA - Removed Thumbnail and Preview properties, use Path only - client path as a URL from context reading preview config, server path from doc_root conf property, both should work either relative or absolute. TODO Bugfix in fm.js: createPreviewUrl would only need to be "/" + path, no call to connector JS - removed global object, merged PHP, images - merged without changes
Thanks a lot, you choud try it on dev branch, check #41 |
JAVA
JS