Map - add forward backward step buttons to easy timeslider - add date pickers too (animation on the cheap)

[bobular]

As per title.

It's not trivial, however, because of the issue below. The Brush component will need to be controlled/controllable. If/when it's controllable, we can add date pickers (date inputs) in the expanded version of the component to allowed precise settting of the window.

After a lot of back and forth between controlled and uncontrolled behaviour of the visx Brush sub-component of the time-slider, we decided to leave it uncontroled. The main problem was with a drift/offset being introduced in the "round trip" between appState and component state as described in this comment:

web-monorepo/packages/libs/components/src/components/plotControls/TimeSlider.tsx

Lines 159 to 162 in a942e92

	// If we reenable the fake controlled behaviour of the <Brush> component using the key prop
	// then we'll need to figure out why both brush handles drift every time you adjust one of them.
	// The issue is something to do with the round-trip conversion of pixel/date/millisecond positions.
	// A good place to start looking is here.

[bobular]

UPDATE: we can hopefully control the brush directly using the innerRef tricks mentioned here airbnb/visx#934 and in this sandbox: https://codesandbox.io/s/github/airbnb/visx/tree/master/packages/visx-demo/src/sandboxes/visx-brush?file=/Example.tsx:4287-4295

It's still a medium cost ticket but at least this part should be straightforward.

[dmfalke]

Has this been discussed in a ux meeting? Just wondering what sort of consensus we have.

[moontrip]

@bobular As a part of this ticket, I have extensively investigated the drift/offset issue. In summary, I found that somehow there exist additional 2 pixels in the first and last bounds: for whatever reason Brush results in adding (-2) pixel in the first bound and (+2) pixel in the last bound. This can be checked via x0 and x1 at onBrushChange function (

web-monorepo/packages/libs/components/src/components/plotControls/TimeSlider.tsx

Line 98 in a942e92

const { x0, x1 } = domain;

) when selecting full range: technically it should be [0, 700] pixels in the current timeSlider configuration (for full range) but it actually has the range of [-2, 702] pixels (more exactly [-1.999999999, 701.99999999]).

Thus, I think that we need to consider this inherent drift when changing range at onBrushChange. For example,
a) compute inherent domain value (drift) from range (pixel) using d3.invert() function, e.g., const drift = xBrushScale.invert(2)
b) in the onBrushChange(), compensate x0 and x1 (current domain values) with the above drift before calculating startDate/endDate, e.g., [x0+drift, x1-drift]

[bobular]

Hi @moontrip - did you see my comment above #775 (comment) ?

I think we can control the brush without fudge factors? It's a while since I looked at this though.

[moontrip]

@bobular I didn't see that carefully now (but saw that before) as I just focused on the drift issue. I am not quite sure if that updateBrush works better than the current approach: I saw a developer's comment that it is not for fully controlled brush and it is not done yet. The updateBrush approach existed before when I used Reset button: Brush example code used it that way to reset range. Anyway I feel that we would still need to offset the drift either way because the Brush itself seems to have such an inherent drift.

[bobular]

Ah, I see.

I thought the drift arose from the round-trip of setting the brush position from our props, and then reading the brush position back from the visx component, or something. This would be avoided if the component was fully controlled. I'll have to look at it in more detail.

[moontrip]

@bobular From my observation, it is not a kind of numerical error as I can see drift at initial condition: well the term drift may not be appropriate as it looks like Brush just adds additional 2 pixels at start and end (for Brush handle or bound edges? IDK).

[dmfalke]

Not sure if either of you have looked at this, but it could account for the offset: https://airbnb.io/visx/docs/brush#Brush_brushRegion. I'm not sure what value we're using, but might be worth playing with it.

[moontrip]

Not sure if either of you have looked at this, but it could account for the offset: https://airbnb.io/visx/docs/brush#Brush_brushRegion. I'm not sure what value we're using, but might be worth playing with it.

It looks like the option is just to choose what will be filtered via Brush, e.g., chart, xAxis, or yAxis, not range: default value is chart. The Brush is originally intended to use Visx Chart together but we use it as a UI only.

[dmfalke]

This part of the description caught my eye:

[...] used for margin subtraction

[moontrip]

This part of the description caught my eye:

[...] used for margin subtraction

@dmfalke Yeah it's indeed confusing 😅

[moontrip]

@bobular I have tested to consider the inherent(?) offset of 2 pixels as I mentioned earlier. For the example of the Geolocation visualizations of field-based studies and time slider's variable of 'species collection start date', such a 2 pixel results in about 4 months, because the data ranges widely (from 1907 to 2024). I made a draft PR just in case: #1076

[bobular]

Hi @moontrip
I was reading through the Brush code today for #1075 and it's definitely inherent in the code...

https://github.com/airbnb/visx/blob/86a851cb3bf622b013b186f02f955bcd6548a87f/packages/visx-brush/src/Brush.tsx#L14

I've tested your PR but will comment over there. Thanks!

[bobular]

So - after #1076 there is probably no need to use the updateBrush approach via the innerRef. Yay 🎉