search

VEuPathDB / web-monorepo

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

Don't send little filters to `useDistributionOverlayConfig` and `useLegendData` #1060

Closed bobular closed 2 weeks ago

bobular commented 2 weeks ago

Fixes #896

The problem for both donut/bar and bubble markers was that useMarkerData was calling either useDistributionOverlayConfig or useLegendData internally using the filters that was passed to it. Problem was that we were sending filters plus "little filters for marker data" (aka viewport and timeslider). This meant that the overlayConfig (or legendData) needed for the marker requests (and/or processing) was not the same as has been used for the legend (which didn't use any little filters).

As explained in #896 this was most obvious when showing the timeslider variable on the markers. The legend and markers were telling completely different stories.

Now the code is slightly more verbose because useDistributionOverlayConfig or useLegendData are called outside useMarkerData and the result is passed into it.

The alternative was to change the useMarkerData props so there was an additional allFilters prop (or similar). I thought this approach was cleaner in the long run.

dmfalke commented 2 weeks ago

I want to make sure I understand the issue. Before, the legend and the markers were using different overlay configs, causing the color mapping to be different between the two. That is because we do not want little filters to impact the legends.

For what it's worth, I think your changes are good. I do prefer passing in the overlay config--it's a good example of inversion of control and separation of concerns.

I didn't test, but can shortly.

bobular commented 2 weeks ago

I want to make sure I understand the issue. Before, the legend and the markers were using different overlay configs, causing the color mapping to be different between the two. That is because we do not want little filters to impact the legends.

For what it's worth, I think your changes are good. I do prefer passing in the overlay config--it's a good example of inversion of control and separation of concerns.

I didn't test, but can shortly.

Well it felt right at the time! DK has kindly done extensive testing after I worked on it without assigning. oops.