'expanded' value is unnecessary (and causes some trouble)

[IgorRodchenkov]

Using 'expanded' state value, along with having 'collapsed', is not a good idea (this defines four states, where we want just two); 'collapsed' alone would be enough. In other words, selectors like this one here, i.e., 'node[expanded-collapsed="expanded"]' do not work if 'expanded' is not set for all expandable objects initially. Instead, using just "selector: 'node'" or ':selected' would work as well (in some places, with minor edits - to check for collapsible nodes is not an empty set), and selector: 'node[expanded-collapsed="collapsed"]'- in other places.

[metincansiper]

@IgorRodchenkov I think that this problem needs to be solved in extension (cytoscape.js-expand-collapse) level first and the line you specified (and the other similar lines if exists) is needed to be updated accordingly in SBGNViz. In deed in the related extension 'expanded' is set for all parent nodes on load. However, as you said changing the name of this state value to 'collapsed' and giving boolean values to it is a better and safer approach.

Please see ( https://github.com/iVis-at-Bilkent/cytoscape.js-expand-collapse/issues/16 )

[IgorRodchenkov]

I meant to have "collapsed" set or unset (removed); could be just one class.

Igor

On Oct 6, 2016, at 7:33 AM, metincansiper notifications@github.com wrote:

@IgorRodchenkov I think that this problem needs to be solved in extension (cytoscape.js-expand-collapse) level first and the line you specified (and the other similar lines if exists) is need to be updated accordingly in SBGNViz. In deed in the related extension 'expanded' is set for all parent nodes on load. However, as you said changing the name of this state value to 'collapsed' and giving boolean values to it is a better and safer approach.

Please see ( iVis-at-Bilkent/cytoscape.js-expand-collapse#16 )

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[metincansiper]

Yes, I think that when we can set it to true when the node is expanded (We need to set it to some specific value) and remove that data when the node is collapsed back. And in checking the state we can look for whether the value is truthy.

[IgorRodchenkov]

As it's right now, you cannot collapse a compound using context menu (there is no such item), but you can do using the small "-" in the right top corner of a node (after you collapsed a node this way and expanded back, context menu 'collapse' appears and works then). I did fix this issue in the pull request, but it re-appeared. This is related to this issue! Do not check for 'expanded' value in the filter/selector in order to collapse a node; it's not necessary. This line is wrong (should be just 'node' in the selector.)

[IgorRodchenkov]

Looks, the 'ctx-menu-collapse' should have the following (selector):

selector: 'node[expanded-collapsed!="collapsed"][sbgnclass="complex"],[expanded-collapsed!="collapsed"][sbgnclass="compartment"]'

for the context menu to show up or not with right nodes and right state!

[metincansiper]

@IgorRodchenkov you are right I missed that point. The selector that you proposed seems nice. I used it instead of the old selector.

[metincansiper]

We no more setting data('expanded-collapsed') in related extension. In that extension I added a class to the collapsed nodes to enable users to define a custom style for them. Now we check if a node has that class to see if it is collapsed.

As for the other selector any parent node should be collapseable so I replaced the following selector 'node[expanded-collapsed!="collapsed"][sbgnclass="complex"],[expanded-collapsed!="collapsed"][sbgnclass="compartment"]' with 'node:parent' and it seem to be working fine.