trash-and-fire / lightweight-charts-react-wrapper

React wrapper for financial lightweight charts
Other
58 stars 5 forks source link

Setting `rightOffset` to some value causes the scrolling position to reset when the series uses `reactive` and `data` props #20

Closed luciodale closed 1 month ago

luciodale commented 2 months ago

I have a query that returns data that is then fed to my CandlestickSeries via the data prop. Every time the query returns new data the scrolling position jumps back to the initial position, when using the rightOffset prop.

This example reproduces exactly the behaviour I'm explaining: https://codesandbox.io/p/sandbox/codesandboxer-example-forked-y4c4y5?file=%2Fexample.tsx%3A101%2C1

Simply scroll to the left and you'll see that the position will always jump back to the right most value, making the navigation impossible.

Any idea why this is happening / how to fix this?

luciodale commented 2 months ago

The problem is that whenever the component the chart is in re-renders (no matter the update triggering the re-render) the chart will always lose the scroll position. Here's another example without using the onVisibleLogicalRangeChange callback: https://codesandbox.io/p/sandbox/codesandboxer-example-forked-l37crz?file=%2Fexample.tsx%3A34%2C1

luciodale commented 2 months ago

My current stab at it was to update this useLayoutEffect code:

useLayoutEffect(() => {
    // remove rightOffset from timeScale to avoid losing scrolling position
    const { timeScale, ...opts } = rest;
    const newOpts = {
      ...opts,
      timeScale: R.omit(timeScale || {}, ["rightOffset"]),
    };

    context.current().update(newOpts);
  }, [rest]);

So, I'm making sure that rightOffset only gets applied once, discarding it in the following re-renders

trash-and-fire commented 2 months ago

This is definitely a bug on my part. I've reproduced the same behavior with vanilla LWC and it works fine as long as I don't update the timescale options on every tick. I will investigate that. https://codesandbox.io/p/sandbox/lwc-6pyh82?file=%2Fsrc%2Findex.ts%3A27%2C26

I would also like to note that you should not use the ChartOptions['timeScale'] and <TimeScale> component at the same time. This may lead to uncontrolled overwriting of options, as stated in the readme.

You can use the following workaround:

      <Chart {...options}>
        <CandlestickSeries
          ref={barRef}
          reactive
          thinBars={true}
          downColor="#000"
          upColor="#000"
          data={stateData}
        />
        <TimeScale
          ref={timeScaleRef}
          rightOffset={10}
          secondsVisible={true}
          timeVisible={true}
        />
      </Chart>

The goal is to prevent unnecessary <TimeScale> updates since it is a React.memo component. If you want to use listeners, wrap them in useCallback or something like useEvent. https://codesandbox.io/p/sandbox/codesandboxer-example-forked-2t7kv3?file=%2Fexample.tsx%3A61%2C26

I will try to fix the bug as soon as possible. Thanks for the bug report.

trash-and-fire commented 1 month ago

Fixed in 2.1.1