Expand all for Projects

[chris-allan]

Add the capability of opening all Datasets in a Project if they are all closed and change the semantics so that expanding or collapsing a node, not selecting it, affects what is shown by Parade.

In order to provide functionality to open all Dataset children for a selected Project the component hierarchy needs to reflect what nodes of the jsTree that are open rather than just those that are selected. This PR contains refactoring to all jsTree node handling, initial state, and resulting subcomponent usage so as to provide the display components with reliable and thorough context under which to operate.

The stage is further set to enable the same type of logic for Screens once we are ready.

Note that the changes in this PR bind Parade to the base level selection_change.ome event.

[chris-allan]

Some investigation into jsTree event batching, delayed asynchronous data loading, and/or axios usage will need to be done. Due to the mount/unmount churn that occurs when many Datasets are opened programmatically, it is possible to reproduce this by doing the same quickly by hand, setState() is being called on unmounted components from AJAX call result handlers.

Some related reading:

• https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html

[snoopycrimecop]

Conflicting PR. Removed from build parade-merge#40. See the console output for more details. Possible conflicts:

• PR #19 chris-allan 'Sparklines'
  • omero_parade/static/omero_parade/css/parade.css

--conflicts

[chris-allan]

Large refactor done in 66c6879dc178b0dd4056cca6216a2ea22413b8d8. Probably at least one more round to come.

/cc @emilroz

[chris-allan]

That's everything pushed down as far as the jsTree nodes are used.

[snoopycrimecop]

Conflicting PR. Removed from build parade-merge#41. See the console output for more details. Possible conflicts:

• PR #19 chris-allan 'Sparklines'
  • omero_parade/static/omero_parade/css/parade.css

--conflicts Conflict resolved in build parade-merge#42. See the console output for more details.

[chris-allan]

Some related reading I came across during investigation for the refactoring completed in this PR :

• https://reactjs.org/docs/reconciliation.html
• reactjs/rfcs#26

In reading several StackOverflow and blog posts it seems like the consensus is that the use of component key changes to force remounting is treated as an anti-pattern. More recently there has also been a push within the React ecosystem towards refactoring the component lifecycle:

• https://reactjs.org/blog/2018/03/29/react-v-16-3.html#component-lifecycle-changes

In order to protect ourselves moving forward we might consider reviewing all our use of lifecycle methods and upgrading to 16.3.x before minting a 0.1.0 release.

/cc @jburel

[will-moore]

Looks good, and is working fine. Good to merge.

[chris-allan]

Current implementation results in a much higher number of FilterHub.render() calls resulting in sluggish performance in a variety of scenarios including selection. Need to look into this before we merge.

/cc @emilroz

[chris-allan]

Aforementioned concerns addressed. Ready for review again. Pay particular attention to performance when selecting >20 images by drag selection in the centre panel.

/cc @emilroz

[will-moore]

Certainly rendering speed has improved with those last few commits. Just to be clear - there's no new functionality in this PR (from the user's point of view)? Current functionality is working well and code looks good. Good to merge for me.

[chris-allan]

The big user interface addition here is the capability of opening all Datasets in a Project if they are all closed and that expanding or collapsing a node, not selecting it, affects what is shown by Parade. Everything else is to facilitate those changes.

[will-moore]

Ah, OK - Don't know how I missed that. Seems to be working fine :)

[chris-allan]

👍

Updated the PR description to be a little more prescriptive about all of that.

[chris-allan]

@emilroz came up with a little script to generate hierarchies for testing:

• https://gist.github.com/emilroz/9956daab41405b8b27b0ef1510893160

With 50 Datasets containing 20 Images each the open_node.jstree event churn and subsequent repeated translation of the node hierarchy to JSON is almost unbearable even on my very powerful system. I'll add something to help with that here in a second.

/cc @emilroz, @jburel

[chris-allan]

d7c25825b11c1872dd82e3618d0f2153e81ee002 does a nice job of reducing the event churn. There should probably be a progress indicator after "Open All" is clicked as opening and loading 50 Datasets is definitely not instant but that can come later. There are probably lots of cases where we should do that.

If @emilroz is happy with the performance now we can get this merged and re-focus our attention on thumbnails.

[emilroz]

All changes look good to me.