[SVCS-580] Collapsible directories for zip rendering

[Johnetordoff]

Ticket

https://openscience.atlassian.net/browse/SVCS-580

Purpose

Our zip render currently lists files of all directories in one big table, other implementations have a neater collapsible hierarchical structure. This PR creates a collapsible directory UI.

Changes

Uses two new JS libraries, jstree and jstree-table and method for the renderer that reconstructs a file tree from a list of paths.

Side effects

None that I know of

QA Notes

Upload a complex zip file and click through it contents.

Deployment Notes

None that I know of.

[coveralls]

Coverage increased (+1.2%) to 65.015% when pulling 48d7f19 on Johnetordoff:better-zip into d1d89e7 on CenterForOpenScience:develop.

[AddisonSchiller]

1. Style changes, line length changes.
2. You need to rework where your files live. They are everywhere and messy. Files that are only served to the zip-renderer should live under `extensions/zip/static/(css/js) etc.
3. you committed an emtpy folder under zip/static/img with only an init.py in it with no python files in it
4. Where is font-awesome being used? You committed it but I don't see it in use.
5. also, you have duplicate image files.
6. your builds are failing due to style problems (blank lines etc)

Needs a lot of clean up and polishing. back to you.

[AddisonSchiller]

Only the ziprenderer is using this. it should be either defined in the ziprenderer as a setting or built from assets_url in the ziprenderer

[AddisonSchiller]

Line length

[AddisonSchiller]

All these zip specific files should live under extensions/zip/static/(js/css), not under MFRs static directory.

[AddisonSchiller]

As mentioned on the mako template, this should live in extensions/zip/static/css

[AddisonSchiller]

should live in zip/static/js

[AddisonSchiller]

should be in zip/statics/js

[AddisonSchiller]

You should be passing in assets_url and defining these as
${base}/static/css......
and not with .../static

[coveralls]

Coverage increased (+0.4%) to 68.421% when pulling ed4ec57 on Johnetordoff:better-zip into 8bb2dd4 on CenterForOpenScience:develop.

[AddisonSchiller]

Bunch of changes and bugs to fix:

1. things based off of icons_url are not being found. See screenshot on comment. Icons on zip renderer do not show up as a result

2. remove i from enumerate(paths) and rewrite the loop

3. if not length..... node_path[..][..][..] != path line is very confusing. That logic needs heavy comments. It should probably be re-written to be more clear.

4. Similar to above, the case where you set node_path = node_path['children][-1]
   needs comments, or to be rewritten.

5. There are extra files. specifically an extra throbber.gif, 32px.png, and 40px.png lying around.
   Things in zip/static and server/static should be directories. There shouldn't be loose files sitting around.
   
   I dont think any of those files are being used. To fix this they can probably just be deleted (also dont know why there is an init.py in there since there are no python files in that directory). Check that no files are using these files listed directly under zip/static. If they are used, restructure your directories so there are no loose files under zip/static.

6. I dont see font-awesome being used anywhere, where is it being used from? I also dont see calls for it in my logs. If it is being used, and its only used by the zip renderer, it should be moved to zip/static

7. Comments comments. There are lots of confusing aspects to building your tree data that could use comments. Especially on the if/else cases. If something is confusing or it isn't apparent why its done, write a small comment explaining why. There are 1 or two areas noted that need comments, but the more the merrier.

8. anything i commented on that I missed in the list

Back to you!

[AddisonSchiller]

Im not sure what this is pointing to. There is no /img/icons folder. In the console when I open a zip file, i get this:

I checked out older versions of your branch and it was still doing the same thing.
I assume this is why the icons aren't showing up for me.

[Johnetordoff]

Works fine for me.

[AddisonSchiller]

I don't see i being used. This can be written without it.

[AddisonSchiller]

This needs a comment, or you should scrub empty strings from your paths before enumerating them.

[AddisonSchiller]

This if statement needs a comment explaining what it does.
to me it looks like
if length != 0 and the current path isn't the last child we just added

also, I assume length -1 is asking for the last child, so it can be written as
...['children'][-1]['text] !=...
make sure that is correct before changing it though.

[AddisonSchiller]

Folder size is always 0. Maybe folder sizes shouldn't be displayed. Has this been demo'd for product yet? See what they say.

[AddisonSchiller]

Okay i understand the if statement a bit better, but there has to be a better way to write this.

[AddisonSchiller]

this shoud be down with ..bootstrap.min.js and it shouldnt be defined by base, it should be defined like
<link rel="stylesheet" href="/static/css/bootstrap.min.css">

[coveralls]

Coverage increased (+0.4%) to 68.386% when pulling 84f4d6f on Johnetordoff:better-zip into 8bb2dd4 on CenterForOpenScience:develop.

[coveralls]

Coverage increased (+0.4%) to 68.386% when pulling cb4bdcc on Johnetordoff:better-zip into 8bb2dd4 on CenterForOpenScience:develop.

[AddisonSchiller]

3 things:

1. you have a line in renderer that is split, but does not need to be
2. you moved default.css to the zip directory while moving your other files. default.css is used by other extensions and renderers and should be moved back to server/static. make sure the location is change din the zip template as well.
3. There are some cases in the zip renderer that could still use comments.

[AddisonSchiller]

This line does not need to be split.

[AddisonSchiller]

This still needs a comment about why you set node_path to be the last child.

[AddisonSchiller]

default.css is a base mfr css sheet. It should be moved back to /server/static/css

other extensions (ipynb, codepygments etc) use it as well.

[coveralls]

Coverage increased (+0.3%) to 72.567% when pulling a42d3c7 on Johnetordoff:better-zip into 8a10328 on CenterForOpenScience:develop.

[cslzchen]

Works as expected and I still remember the demo you did a while ago. 🎆 🎆 However, there are a few issues we need to figure out @Johnetordoff since this PR has been sitting here for too long.

• Commit history is not pretty
• Folder/package structure
• Left-over code and files

When try to rebase you code to develop, there are conflicts after conflicts. I think this is due to many previous merges from develop which already fixed conflicts.

I want to reset all the changes and make new meaningful commits. For example, adding all the extension icons, the JavaScript libraries and CSS files, update viewer.mako and render.py, add/update tests, and update comments/docstr, etc.

I am not sure of you time but it will be great if you can do it. I don't feel comfortable stealing the commits/credits.

CC @felliott

[Johnetordoff]

@cslzchen I've brought this up to date and consolidated the history.

[cslzchen]

Thanks for the quick update! Here is a few issues left, all of which are about static assets. 🔥 🔥

Duplicated CSS Files

There are three CSS files being added:

• static/css/jstree-style.min.css
• static/jstree-theme/style.css
• static/jstree-theme/style.min.css

First, only jstree-theme/style.min.css is used in the file viewer.mako. There is no need for teh full version jstree-theme/style.css.

Second, jstree-style.min.css and jstree-theme/style.min.css looks very similar but have different sizes. Can you double check to make sure that we are using the correct one?

Two Images

There are two images that does not look right:

• static/jstree-theme/32px.png
• static/jstree-theme/40px.png

Are they correct and are they actually used? If so, does the JavaScript library crop them to get icons on the fly? I doubt it though. (I may have misunderstood how it works.)

One Script

• static/js/jstreetable.js

Is there a minimal version that we can use? Or do we have any customizations, which is usually the reason for using the full version. Please document if so.

[Johnetordoff]

Duplicated CSS Files:

• Was able to remove these.

Two Images:

• These are necessary to the render, why are they mapped into one file? I don't know, but it's not uncommon thing. 32px.png are the regular set, 40px.png for mobile.

One Script:

• We do have a customization for this, I've added a comment.

[cslzchen]

Merged into a feature branch where I will make some final tweaks. Consider this a merge and thanks for the great work 🎆.