DM-7462: create dispatch function, rendering style and stored status for region selection #177

cwang2016 · 2016-09-13T20:48:24Z

Implementation:

create region selection dispatch function, dispatchSelectRegion.
create status to record the text description of selected region and provide function to access the status.
add selectMode parameter into dispatchCreateRegionLayer & dispatchAddRegionEntry functions which defines the rendering style, rendering color and line width for the selected region. 5 types of rendering styles are defined, 'UprightBox', 'DottedOverlay', 'SolidOverlay', 'DottedReplace' and 'SolidReplace'.

Testing:

from localhost:8080/firefly/demo/ffapi-highlevel-test.html,
1. click 'test open'
2. find color, line width text boxes and rendering style dropdown list next to 'Create Region Layer' to set the display features for showing the selected region, then click 'Create Region Layer' to create region layer with the id shown on the box next to it.
3. go down to the bottom of the page to select the region (one region description is already put into the textarea box)
  - select the region layer from the dropdown list,
  - click 'Select the following region(s)'. (note: current version support single region selection)
    You may delete the region layer and create a new region layer to see the different rendering effect on the selected region. (set the style before clicking 'Create Region Layer')
from localhost:8080/firefly
1. using image select panelt to select two images, m51 & wise, m51 & 2mass
2. upload region file, test5.reg, from firefly_test_file/region_test_files
3. click any region to see the region is highlighted over all plots.
  (the default rendering style for selected region is applied in here).

…l select modes including rendering style, color, and line width.

cwang2016 · 2016-09-13T20:48:55Z

Please recompile firefly.

tgoldina · 2016-09-13T22:58:26Z

src/firefly/js/visualize/DrawLayerCntlr.js

+ * @param {string} fileOnServer region file name on server
+ * @param {string[]|string} regionAry  array or string of region description
+ * @param {string[]|string} plotId  array or string of plot id. If plotId is empty, all plots of the active group are applied.
+ * @param {function} dispatcher


should be last param

In general, there quite a few eslint warnings/errors (with our .eslintrc) in the changed files, like unused imports, vars, etc. It would be beneficial to fix them.

tgoldina · 2016-09-14T00:18:09Z

Select mode works great.
My major concern about REGION_SELECT action is that selectedRegion in its payload is an object. It's not meaningful in the context of user's callback function (like onRegionSelect callback below):

firefly.util.addActionListener(firefly.action.type.REGION_SELECT, onRegionSelect);

Can we have a string, which represents user region in REGION_SELECT payload?

cwang2016 · 2016-09-14T00:39:40Z

'selectedRegion' in dispatchSelectRegion does take either string, array of strings (for programmatic purpose) or region object (for the usage of UI clicking) .
Please see how dispatchSelectRegion takes strings in 'ffapi-highlevel-test.html'.

tgoldina · 2016-09-14T03:36:08Z

Yes, dispatchSelectRegion does work as expected - any of the regions added can be programmatically selected from an external script.

I am talking about different thing: an external script should also be able to react to region selection made by mouse click in the UI.

An external script can trigger UI changes by calling action dispatchers. But it can also react to any UI action by adding a listener to this action. A listener is a user-defined function, a callback, which is called whenever the action goes through the flux.

For example, some external script is using Firefly API to plot amplifiers, represented on the image by box regions. Each amplifier is described by unique name and region string, and this 1 to 1 map between name and region string is maintained by the script. When region is clicked, the script displays name.

To do it, the script defines a callback, which takes REGION_SELECT action object and uses the action payload to find which region is selected. The only representation of region the script is aware of is region string. But the payload of REGION_SELECT action (triggered by mouse click) has only selectedRegion object, which is Firefly-internal representation of the region.

cwang2016 · 2016-09-14T06:59:47Z

as we discussed in previous meeting, a utility function 'getSelectedRegion' is provided to report the status of the selected region, i.e. the region string.

Moreover, region string is actually stored in region object. If needed, one more utility besides 'getSelectedRegon' could be implemented to return the region string based on the region object.

Would that be ok for the callback provided by the external script to use either utility to get the region string? dispatchSelectRegion mainly uses region object to form the highlight result. The string(s) passed to the dispatchSelectRegion is used for discovering the region object. Adding both region object and region string to the action payload seems redundant for dispatchSelectRegion itself.

tgoldina · 2016-09-14T14:04:39Z

Sorry, I completely forgot about getSelectedRegion. Yes, the callback can
use it. We need to export it from ApiUtilImage.jsx though. Thank you for
the explanation.

2016-09-13 23:59 GMT-07:00 Cindy Wang [email protected]:

as we discussed in previous meeting, a utility function
'getSelectedRegion' is provided to report the status of the selected
region, i.e. the region string.

Moreover, region string is actually stored in region object. If needed,
one more utility besides 'getSelectedRegon' could be implemented to return
the region string based on the region object.

Would that be ok for the callback provided by the external script to use
either utility to get the region string? dispatchSelectRegion mainly uses
region object to form the highlight result. The string(s) passed to the
dispatchSelectRegion is used for discovering the region object. Adding both
region object and region string to the action payload seems redundant for
dispatchSelectRegion itself.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#177 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ALU3QHr-o4vRw1rp4QJydeWd0g4KgkZHks5qp5tkgaJpZM4J8IDH
.

tgoldina · 2016-09-14T15:14:29Z

I have tested getSelectedRegion, it works great. All that needs to be done is to export getSelectedRegion from ApiUtilImage and add it to JSDoc by adding @public annotation and including RegionPlot.js into the list of sources in jsdoc_config.json We might also want to mention it in dispatchRegionSelect JSDoc.

tgoldina · 2016-09-14T15:23:39Z

src/firefly/js/drawingLayers/RegionPlot.js

+/**
+ * @summary get the region description of the selected region from the specified region layer
+ * @param {string} drawLayerId id of the drawing layer
+ * @return {string}


I noticed this method returns null, when no region in drawLayerId is selected.

if the click occurs nowhere (no region under), then the previous selected region will become de-selected. Passing a 'null' is the way to de-select the region.

I understand, we just need to document it. Looking at the method code, I though I'd get empty string.

I will document it, but this is not public function. Is that needed in public?

If you pass a 'null' or empty array into 'dispatchSelectRegion', it will work as de-select. I will be documented on the dispatch function. Thanks.

tgoldina · 2016-09-14T18:35:12Z

getSelectedRegion needs to be public. Every method exposed via API should
have public annotation.

2016-09-14 11:20 GMT-07:00 Cindy Wang [email protected]:

@cwang2016 commented on this pull request.

In src/firefly/js/drawingLayers/RegionPlot.js
#177:
           var idx = getRegionIndex(regions, aRegion);
           if (idx >= 0) {
               prev.push(drawObjAry[idx]);
           }
       }
       return prev;
   }, []);
}

return selDrawObj.slice(0, stopIndex);
+}
+/**

* @summary get the region description of the selected region from the specified region layer

* @param {string} drawLayerId id of the drawing layer

* @return {string}
I will document it, but this is not public function. Is that needed in
public?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#177, or mute the thread
https://github.com/notifications/unsubscribe-auth/ALU3QCUQVR0HKqEYk0jHTo0YOgLPV77Mks5qqDr5gaJpZM4J8IDH
.

…AddRegionEntry. export getSelectedRegion from ApiUtilImage.jsx update jsdoc for region dispatch functions.

…n PointDataObj.js.

cwang2016 · 2016-09-14T19:09:06Z

some addition to the implementation:

change the signature of dispatchCreateRegionLayer and dispatchAddRegionEntry - add selectMode to replace the original relevant parameter, the default is empty object if no value is passed.
export getSelectedRegion from 'ApiUtilImage'
update jsdoc for region related dispatch functions.

DM-7462: create dispatch function for region selection based on sever…

7f8b003

…l select modes including rendering style, color, and line width.

tgoldina reviewed Sep 13, 2016
View reviewed changes

tgoldina reviewed Sep 14, 2016
View reviewed changes

Cindy Wang added 2 commits September 14, 2016 11:46

update selectMode paramter for dispatchCreateRegionLayer and dispatch…

9e1cc91

…AddRegionEntry. export getSelectedRegion from ApiUtilImage.jsx update jsdoc for region dispatch functions.

add color parameter while calling 'makeShapeHighlightRenderOptions' i…

c951eff

…n PointDataObj.js.

cwang2016 merged commit c85cfb8 into dev Sep 14, 2016

cwang2016 deleted the DM-7462-RegionSelection branch September 14, 2016 19:09

DM-7462: create dispatch function, rendering style and stored status for region selection #177

DM-7462: create dispatch function, rendering style and stored status for region selection #177

Uh oh!

Conversation

cwang2016 commented Sep 13, 2016

Uh oh!

cwang2016 commented Sep 13, 2016

Uh oh!

tgoldina Sep 13, 2016

Choose a reason for hiding this comment

Uh oh!

tgoldina commented Sep 14, 2016

Uh oh!

cwang2016 commented Sep 14, 2016

Uh oh!

tgoldina commented Sep 14, 2016

Uh oh!

cwang2016 commented Sep 14, 2016

Uh oh!

tgoldina commented Sep 14, 2016

Uh oh!

tgoldina commented Sep 14, 2016 via email

Uh oh!

tgoldina Sep 14, 2016

Choose a reason for hiding this comment

Uh oh!

cwang2016 Sep 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgoldina Sep 14, 2016 via email

Choose a reason for hiding this comment

Uh oh!

cwang2016 Sep 14, 2016

Choose a reason for hiding this comment

Uh oh!

cwang2016 Sep 14, 2016

Choose a reason for hiding this comment

Uh oh!

tgoldina commented Sep 14, 2016

@cwang2016 commented on this pull request.

Uh oh!

cwang2016 commented Sep 14, 2016

Uh oh!

Uh oh!

cwang2016 Sep 14, 2016 •

edited

Loading