Wire marker selection to floaters #821
Conversation
… type of setter props, in preparation for wiring
| ...prevSelectedMarkers, | ||
| props.id, | ||
| ]); | ||
| const handleClick = useCallback( |
There was a problem hiding this comment.
useCallback doesn't seem essential but seems sensible given it is debounced and this may avoid the debouncing being reset unnecessarily.
Especially as we are adding selectedMarkers to the function. Previously we were using "functional update" prevSelectedMarkers
…e-marker-selection-to-floaters
|
How about just having a yellow border around the floating plot when it is showing data from selected markers? @dmfalke @moontrip @chowington I think we probably need a Map UX meeting this Thurs. |
|
Playing with the clickable markers and floaters, I'm finding it really misleading that we currently don't apply the "marker-config little filter" to the floaters. In the example screenshotted below, if you click on the bubble marker, the "study tags" floating bar plot would lead you to believe that the tick data you just clicked on was provided by the WIN or Powell lab networks (which both work on mosquitoes not ticks). What's actually happening is that the floating plot is being filtered by the geohash rectangle that the marker lies within. I think this is probably an argument in favour of having floating plots always filtered by "marker-config little filters". It's something we could trial in QA at least.
|
…butionDataPromise is ready' check; tested regular EDA - looks fine
| ), | ||
| ], | ||
| }; | ||
| const distributionDataPromise = usePromise( |
There was a problem hiding this comment.
removed the double-nested useCallback
| @@ -560,13 +552,6 @@ function HistogramViz(props: VisualizationProps<Options>) { | |||
| ) | |||
| return undefined; | |||
|
|
|||
| // wait till distributionDataPromise is ready | |||
There was a problem hiding this comment.
There was no need to wait for this - it was indirectly causing a double data request due to distributionDataPromise.* being added as dependencies.
There was a problem hiding this comment.
Though waiting for it probably avoided the runtime bug that I later fixed.
|
Sorry, will roll out new logic to all modes/viz and un-draft tomorrow. |
…er is highlighted) with a bad date format that came from the empty string fallback
… getPlotSubtitle already existed and used this instead of what I used before
|
All good for review now! |
| @@ -902,6 +903,7 @@ function BarplotViz(props: VisualizationProps<Options>) { | |||
| .every((reqdVar) => !!(vizConfig as any)[reqdVar[0]]); | |||
|
|
|||
| const LayoutComponent = options?.layoutComponent ?? PlotLayout; | |||
| const plotSubtitle = options?.getPlotSubtitle?.(); | |||
There was a problem hiding this comment.
see standaloneVisPlugins.ts
|
Hi @bobular 👋 I started looking at this and tried to test the map with Field-based vector popolation studies at first. But I found a couple of issues so far:
In the meantime, I will try to address a message for null case of binspec etc. (to be no/empty data message) as we discussed in the meeting. Update 1: nevermind I will test again as I just realized that my branch was old Update 2: all above observations worked just fine after updating my local branch, so strikethrough. Sorry for confusion! |
There was a problem hiding this comment.
Overall, it worked fine and I couldn't find specific issues after careful tests. Also I didn't notice specific issues on codes. 👍
One note is that there still exists handling null of binslider/binspec on Histogram Viz. Will try to use similar approach used in #826. But this does not block the approval of this PR as it will be handled in the other ticket/PR
Resolves #432
Currently branched off #794
First off, as suggested by Dave months ago, I have moved the event handling of "single click to unselect all" out of MapVEuMap and into SemanticMarkers. This sets the scene for wiring up selected markers to appState per marker mode (which makes sense, because some marker modes might have no markers!)
Testing the floaters is tricky - there's a performance issue with sample-level variables, and assays are affected by this VEuPathDB/EdaDataService#351 However, for PoC you can make a floating barplot on "continent" and click around to see what happens quite quickly.SOLVED ✔️