search

OSC / cloudcmd

Cloud Commander orthodox web file manager with console and editor.
cloudcmd.io
0 stars 0 forks source link

Set the root tree via parameter #47

Open brianmcmichael opened 6 years ago

brianmcmichael commented 6 years ago

Allows for the changing of the root of the file tree when a treeroot query parameter is set.

This mechanism:

1) Grabs the query param from the URL if present 2) Sets an environment variable on the process to the URL 3) Renders the process variable into a javascript variable on the client html 4) An onload function on the client side checks for the presence of the variable, and if present 5) sets the window.name to the path 6) After the variable is rendered into the client html once, the process variable is wiped to prevent future windows from adopting the name

All-in-all, I'm not happy with this solution, as it seems quite serpentine in the way the data transfers around the app. It also the seems to be fairly brittle in it's reliance on "window.name", which could be set by the client browser elsewhere. I'm not sure of another way that we would be able to persist the custom setting across reloads, but not across browser tabs, however.

To test this, you'll need to

brianmcmichael commented 6 years ago

This has been cleaned up quite a bit and uses the json string described at https://github.com/OSC/ood-fileexplorer/issues/166#issuecomment-348974279

Also fixes https://github.com/OSC/ood-fileexplorer/issues/173

To test, follow the directions described in the top comment.

ericfranz commented 6 years ago

@brianmcmichael I set @nickjer as a reviewer. Do you still have qualms about this approach, like you mentioned in https://github.com/OSC/ood-fileexplorer/issues/166. Jeremy asked if we could just use query param and try to ensure that every link in the app itself has the query param added. One possibility would be to handle the event that bubbles up when a link is activated so that this link can be modified i.e. to add the query param if it already exists.

You already did a lot of work on this implementation though, at my request. So I'll leave it to you two to decide if this the current approach is safe and reliable, considering multiple browsers, including IE11, or if an alternative just using query params is also do-able.

brianmcmichael commented 6 years ago

This is all working pretty well. I had resolved my concerns by storing the data in a json format rather than as a string, with error handling in case it's not parseable.

nickjer commented 6 years ago

Also have you explored https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage

nickjer commented 6 years ago

I am probably doing something wrong. I followed your test directions above but I only see Home in my tree root. Should I also delete my node_modules directory?

Update: Nevermind, I was using the wrong query param.

nickjer commented 6 years ago

If I go to:

https://ondemand-dev.osc.edu/pun/sys/files/fs/etc/httpd/conf.d/?treeroot=/users/appl/jnicklas

I get this...

image

brianmcmichael commented 6 years ago

Hmm. So it's working for me. I'm wondering if you've got some old javascript files cached somewhere.

untitled

ericfranz commented 6 years ago

From reading it, sessionStorage looks like the right tool for the job. If that’s not currently being used for persistence maybe we can switch to that, while using the same payload that already works?

nickjer commented 6 years ago

Discovered a couple more issues: