jstree upgrade (Functionality Review Only)#3141
jstree upgrade (Functionality Review Only)#3141dpwrussell wants to merge 38 commits intoome:developfrom dpwrussell:jquery_jstree
Conversation
|
--breaking |
|
Travis failure. |
|
Congratulations on getting travis to pass! |
|
@mtbc Thanks, I think that was the easy bit! |
|
Could we add escape patch some how? vakata/jstree#756 |
|
@aleksandra-tarkowska Not sure what you mean. You want to avoid having this patch? At the moment we do not use force_text because we rely on html attributes inside the names to achieve the child counts. Potentially this could be changed to use a custom plugin which modifies what jstree renders. |
|
@dpwrussell I would like this patch to be added. We should escape HTML code from within a object name displayed in a tree. |
|
The patch itself is already in the released version that is in this PR. I will look into redoing the child display as a plugin so that force_text On 3 November 2014 17:04, Aleksandra Tarkowska notifications@github.com
|
|
This will need to have origin/develop merged in at your convenience, @dpwrussell, though waiting for the m2/units tag might make sense as well. |
|
@joshmoore Done the first bit, I'll go again after units is merged. |
|
It's already merged, just not tagged. |
|
@aleksandra-tarkowska force_text enabled |
|
@dpwrussell @pwalczysko @will-moore @aleksandra-tarkowska Tested using http://trout.openmicroscopy.org/breaking/webclient/ user-3 read-only-1 - Mac - Safari and Firefox - accessing by broadband from home Safari: Stopped using Safari and proceeded with Firefox: When creating a new container, I am getting the tag “span class=“children_count”" appended to the name in the data tree (but not in the right hand pane name field). On login the default data tree display shows all members of the default group with current user toggled open. Long image names are not truncated in the data tree as they are in current. Strange behaviour with viewing a screen - in user-10 data tree - toggled a screen closed, name and icon changed to “Orphaned images” - withTest image 05.png in it. This is current user - user-3’s orphaned images folder. After refreshing data tree all returned to normal. While moving quite quickly through data tree selecting various elements, often getting a jump to a completely different element and user. Also seems to freeze up - can select and open containers - cannot select images/runs in containers. {{{ Calling it a day after this - see if Petr comes up with anything more.
|
|
All my tests were done on win7, using Firefox. user-4, group read-write Cannot expand dataset of another user (discussed already with @dpwrussell , happens also when logged in as user-10 who owns the dataset).
|
|
Right-click context menu is hidden by central pane now. (it was not before)
|
|
I have one image in the Orphaned forlder highlighted in the data tree. The same image is not present in the central pane. ID 8452
|
|
I cannot show any plates. Instead, after a click on a Plate Run in tree, the central pane reloads the main frame of the webclient.
|
Safari CrashI have not been able to replicate the Safari crashing problem. Can anyone confirm this? children_count htmlThis is leftover from the recent work to move the children count to a jstree plugin to enable force_text to be enabled. Fixed in a1d16ec13c20ab8cde6d6c0b749c17ada7109be6 TruncationFixed in fed910f9f6a561eddbcbd28c877ca9c82b6500a9. Now also works on containers as well as images and is tightly integrated with jstree. Weird behaviour with screens and later intermittent failuresI suspect this might have occured after running into the plate rendering error that I mentioned this morning. That is now fixed, so if you could try and recreate these problems again tomorrow and then we can take it from there. |
|
I suggest I try and replicate when I am in Dundee tomorrow - it might be a clue as to whether it is a network timeout error or similar. As I said it seemed to be when the browser had been idle for a minute or two. |
|
@gusferguson Sounds good. If it is network related though then it is a very odd thing to happen just because something times out or similar. |
Hotkeys is now built into jstree so remove that plugin jstree + Everything else Added appropriate HTTP Response Codes to REST API Changed default paging and no-arguments for tree.py functions Functions will now return as much data as possible by default, it is then filtered by some or all of the other options. Added capability to customize the page size on REST API views and tree.py Small tidyup remocing comments and whitespace changes Removed leftover old '-locked' check Some whitespace and comment changes also Whitespace and comments Fixed extra parameter in marshalling plate acquisitions Expand node on selection by default Added select on up/down keys in jstree Changed expand-on-open jstree to use click not select This is to allow keypress navigation without opening everything Also, enable double click on a jstree image to open the full-viewer Load, but not open nodes on select with key navigation as that is what the current client does Restore selection intelligently upon refresh Added child count indicator and ensure it is updated when its number of children change Correctly update the child count of containers within the tree when changing one of their identical brethren Fix re-load on selection change on already loaded jstree nodes Fix slight issue with duplicate node child count on insertion in jstree Added conditional selection to jstree to prevent selection of different types simultaneously Comment on group-dropdown behaviour Remove group-user drowndown enhancements from branch Improvements to public share/discussion interface REST API now offers the capability to return all shares/discussions, optionally filter by those that a user is a member of and/or the owner of. Webclient updated to use new API interface Webclient public interface now responds correctly to dblclick Webclient public interface now correctly updates the right panel Webclient public interface now correctly handles the case where a shared image has been deleted Remove old functions Added 400 Bad Request to handling of ajax errors Also added a 400 for ValidationException for the containers api to test with initially Added more exception handling to REST API Fixed double-loading when expand-on-select in jstree Fix rename of P/D/I updates jsTree node name jsTree up/down hotkeys deselect_all() then select_node() Handle left & right jsTree hotkeys Slight readability improvement to hotkeys Handle renaming with identical nodes in jstree Added comment on renaming nodes in jstree on change Disallowed selection of multiple of the same item simultaneously in the jstree Handle all the possible exceptions in REST API and return appropriate HTTP Response Codes Minor PEP8 corrections Fix area select Handled filtering the centre panel results Correctly update the jstree toolbar when updating the tree selection in response to filtering or icon selection Updated webclient area selection to only update dom and jstree once instead of once per icon selected Fix bug in REST API view where group context was not being taken into account when listing tagged objects Added DELETE list of tags capability to the REST API Fix toolbar and deleting tags in webclient Tags panel Comment and minor fix Fixed buttons toggle on tags panel when selecting in middle panel in webclient Disabled multiple selection of the same object Enabled load on select, open in click and double click open for images Remove 'annotation' button config from webclient as it doens't seem to exist Fix share/discussion REST API that was returning duplicate rows in some cases Integration tests for marshalling of shares/discussions Added crosslinked object checks to tree tests Correctly disable paste button after emptying paste buffer Updated jstree to latest standard release (3.0.8) Add missing locate functionality when nodes are created Reinstated 5.0 cut behaviour Context menu 5.0 cut behaviour Very nearly completed reworking of thumbnail update + conventional cut support. Committing for safety reasons Fixed numerous issues with updating thumbs/tree on copy/cut/move/delete/dnd including some to do with updating other identical nodes and/or child counts Fixed missing plugin in tags/public Fixed missing data on copied nodes
|
@dpwrussell There's a minor bug with pagination: If I pick 2 datasets with paginated images, expand ds-1 then expand ds-2, go to second page of ds-2, then click an image in jsTree from ds-1, ds-1 loads on page 2 so that the clicked image is not on that page and not highlighted. |
|
Tested using http://trout.openmicroscopy.org/breaking/webclient/ user-3 read-only 1 Mac Safari and Firefox - Windows IE10. Retests: Crashing after being idle
Children count tag being displayed
New Issues: Mac and Windows: Right-click menu:
After clicking on a screen got this in the Firefox console - don’t know if it is significant as there was no problem with viewing at the time: TypeError: sel_obj.rel is undefined jquery-1.11.1.js line 338 > eval:183
|
…ing in a variety of situations
|
@gusferguson I've tried with Safari to replicate your crashes with no luck, even against trout. When you say it crashes, the browser crashes completely? Do you think this could be related somehow to the weird force log out thing which seems to happen on trout (with or without my PR) a lot? Oddly, I do not see the force log out thing with safari at all, but I see it with Chrome all the time. @pwalczysko Did you use Safari? |
also causing the disabling on some context menus
|
@will-moore Pagination bug should be fixed in fc83185 |
|
@gusferguson @pwalczysko Context Menu being partially hidden should be fixed in 49724a2 |
|
@gusferguson Context menu disabling on right-click of experimenter should be fixed in 03f8a0b |
|
@dpwrussell - it is the forced logout - not browser crashing. |
|
@dpwrussell : I did use Safari and had no problems. But I know that the forced logout is a problem, I reported it some time ago, then could not repeat. It happens on Safari as well as Firefox (at least these I have tried back then). It was there before this PR was in. it seems to be connected to the fact that many users are logged in (some of them several times, from different machines). |
|
@gusferguson @pwalczysko Excellent, as it's pre-existing and happens to me on all trout builds it seems unrelated to this PR. With that, I think I've addressed all your comments. |
involvement of rwrwr- groups.
|
Contacted @dpwrussell. This work is being passed off to @will-moore. They decided that a reworking/cleanup of the branch won't necessarily make the next steps any easier, and so there's nothing else coming here. Closing. NB: I've added the |
9 participants
This is a huge PR addressing the update of jstree which had far reaching consequences. Because much of the code needed to be rewritten, I have also fixed a number of other bugs in the webclient at the same time and switched to a fully JSON REST API powered tree. Obviously it would be preferable to have done that in separate PRs, but that would have required coding a lot of broken behaviour from scratch only to then fix it.
As it's such a large PR, I am looking to evaluate the functionality first, fix any obvious problems, and then I will refactor this PR into logical chunks for what will probably be a hefty code/full functionality review.
Trello Link: https://trello.com/c/tly4JAme/63-jquery-and-jstree-3
Testing instructions
Basically just compare existing webclient to this.
There are differences, most of which are upgrades (e.g. child numbers now update when copying/moving objects) or bug fixes (e.g. tree now supports having the same object in multiple places without unpredictable behaviour), but if there are mistakes, it would be good to find them now.
I am aware that webtagging does not work yet, this is because it was relying on webclient internals (unavoidable in current plugins realistically and also unavoidable to break it here) and I have yet to update the public version.
Known Issues
Permissions have not yet been updated with the hack required for http://trac.openmicroscopy.org.uk/ome/ticket/9800
The All-Users display is not consistent with current webclient. However, it is much more consistent with Insight which might be more desirable.