VEuPathDB / web-monorepo

A monorepo that contains all frontend code for VEuPathDB websites
Apache License 2.0
2 stars 0 forks source link

Unintended consequences of Network work upsetting the bipartite network #1153

Closed asizemore closed 1 month ago

asizemore commented 1 month ago

I would guess this happened because of the recent updates done to Network. What should happen is that the bipartite network should scroll within it's box.

Screen Shot 2024-07-29 at 10 43 54 AM

The other unintended consequence is that all labels are hidden by default. For the bipartite network there are never issues with label overlap so let's change it so that the bipartite network has labels shown all the time.

moontrip commented 1 month ago

@asizemore I have no idea of the first one concerning scrolling, but I think I know the reason for the second one, showing labels. As you can remember, we have node label control for Network Viz, not for Bipartite network Viz. When I looked into Bipartite, I realize that it calls like: Bipartite network Viz > BipartiteNetworkPlot (BipartiteNetworkPlot.tsx) > NetworkPlot (NetworkPlot.tsx) > NodeWithLabel (Node.tsx) On the other hand Networ Viz's process is like: Network Viz > NetworkPlot > NodeWithLabel

The problem arises because we have changed NetworkPlot and NodeWithLabel components to implement node label control, more specifically, this showLabel prop (https://github.com/VEuPathDB/web-monorepo/blob/6b9bba0f55bee8b43cfa055977fa3b7d321e73c1/packages/libs/components/src/plots/NetworkPlot.tsx#L311). For Bipartite network viz that does not have node label control, showLabel prop becomes false, thus no label is shown.

To overcome this, I think we have one of the following two options: a) pass a new prop from BipartiteNetworkPlot to NetworkPlot (e.g., the name of "BipartiteNetwork") and add an additional OR (||) condition for showLabel prop to make it true for the case of Bipartite network; b) add node label control to Bipartite network Viz too.

asizemore commented 1 month ago

Thanks @moontrip ! Given the two options, i'd pick (a). The bipartite network never has trouble with overlapping labels so the labels should always be shown, and therefore we don't need controls like we do for the regular network.

I gave a quick look at the components again, thanks to your link. What do you think about having the BipartiteNetwork send in all node labels as the visibleNodeLables? Then all labels would be in visibleNodeLabels and we wouldn't need a new prop.

moontrip commented 1 month ago

Thanks @moontrip ! Given the two options, i'd pick (a). The bipartite network never has trouble with overlapping labels so the labels should always be shown, and therefore we don't need controls like we do for the regular network.

I gave a quick look at the components again, thanks to your link. What do you think about having the BipartiteNetwork send in all node labels as the visibleNodeLables? Then all labels would be in visibleNodeLabels and we wouldn't need a new prop.

@asizemore Yes, you are correct. We can use that prop, visibleNodeLables instead. I will work for that tomorrow and make a PR for that.

moontrip commented 1 month ago

@asizemore I will also see if I can find something for the first one when working with the second one 🤞

asizemore commented 1 month ago

Thanks @moontrip ! I appreciate it!

moontrip commented 1 month ago

Thanks @moontrip ! I appreciate it!

My pleasure :)