Closed moontrip closed 1 year ago
Even if we do the same for lineplot, I think Lineplot.tsx should be where the data is modified. Both components can use the same utility function of course.
I also wonder if we should have the additional text-box entry for the opacity slider?
Reasons to remove:
The spec in https://github.com/VEuPathDB/web-eda/issues/1487 isn't fully implemented. The markers should only have an outline when opacity == 0. I think that's achievable by setting
marker: { line: { width: 0 } }
when needed.
First of all, thank you for your detailed reviews and tests, @bobular 👍 For this one, I am afraid that I cannot understand. Yes, the marker should only have an outline when opacity == 0, which actually means marker.line should always has width (not zero). If it is zero, then marker is not visiable when opacity == 0.
First of all, thank you for your detailed reviews and tests, @bobular +1 For this one, I am afraid that I cannot understand. Yes, the marker should only have an outline when opacity == 0, which actually means marker.line should always has width (not zero). If it is zero, then marker is not visiable when opacity == 0.
Oh, sorry, I got that the wrong way round! I wrote it first time round in a complicated way, and then edited it to make it completely incomprehensible, sorry!
So the width should be set to zero when the opacity is greater than zero.
So the width should be set to zero when the opacity is greater than zero.
@bobular Aha now I see what you are suggesting now. Yes it may be the case, but one disadvantage is that if opacity is low (e.g., 0.1 or even < 0.5), then marker may not be that visible.
So the width should be set to zero when the opacity is greater than zero.
@bobular Aha now I see what you are suggesting now. Yes it may be the case, but one disadvantage is that if opacity is low (e.g., 0.1 or even < 0.5), then marker may not be that visible.
That's the idea! Actually I tried it. Even with 0.1 opacity you can still see the points.
The real problem (that is not part of this ticket), is the order that Plotly draws the markers in. Here's a GEMS1 example where we are very lucky that the green points get drawn last!
So the width should be set to zero when the opacity is greater than zero.
@bobular Aha now I see what you are suggesting now. Yes it may be the case, but one disadvantage is that if opacity is low (e.g., 0.1 or even < 0.5), then marker may not be that visible.
That's the idea! Actually I tried it. Even with 0.1 opacity you can still see the points.
The real problem (that is not part of this ticket), is the order that Plotly draws the markers in. Here's a GEMS1 example where we are very lucky that the green points get drawn last!
I see. Then I will try to change the line width too 👍 Unless it is a gradient color map, perhaps it can be alleviated by user to show/hide markers via legend, I suppose.
I also wonder if we should have the additional text-box entry for the opacity slider?
Reasons to remove:
- It is large and distracting
- The up/down step buttons currently take it out of range immediately
- The restricted step-range of the slider makes it easy to reproduce the desired setting.
Not quite sure if there exists hiding text input form at the current slider: I guess you want to keep it for other slider like bin. I will check the slider option.
Here is a summary for additional works to do based on @bobular's suggestions:
a) handle opacity at Scatterplot component, not PlotlyPlot b) set marker.line.width = 0 when opacity > 0 c) use color-math instead of rgba d) check if hiding text input form at the current slider is doable
@bobular I have addressed all of your suggestions summarized above. In addition, added a gradient color track for slider so that it would be more clear (maybe? 😃) Screenshots are available at https://github.com/VEuPathDB/web-eda/pull/1494
This is required for resolving https://github.com/VEuPathDB/web-eda/issues/1487. Relevant PR will be made at web-eda too (https://github.com/VEuPathDB/web-eda/pull/1494). Screenshots can be found the corresponding web-eda PR.