plotly / plotly/react-plotly.js
onSelected uses stale function even when prop for onSelected changed
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 1.1k
- Forks
- 138
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 4
Description
While using react plotly I noticed that changes to the onSelected prop are ignored and the initial function used for that prop is called instead when building a lasso based chart. This is an issue when we want to listen to context changes to things like translations.
The code below shows the issue:
const translations = useTranslation<TaggingIndicatorsTranslations>(
allTranslations
);
const [currentBranch] = useBranch();
.....
.....
.....
const filterSelectedProductsWithContext = function filterSelectedProductsCB(
param: PlotSelectionEvent
) {
filterSelectedProducts(param, currentBranch, translations);
};
return (
<div style={{ height: "100%", width: "100%" }}>
<Plot
data={table}
config={{ displayModeBar: false, plotlyServerURL: currentBranch }}
layout={{
dragmode: "lasso",
xaxis: { title: xAndY.x },
yaxis: { title: xAndY.y },
autosize: true,
// title: "Helpful Indicators To Tag",
}}
useResizeHandler
style={{ width: "100%", height: "90%" }}
onSelected={filterSelectedProductsWithContext}
/>
</div>
);
In the above translations and branch are context values (custom context values) that change. We want the behavior of onSelected to change to reflect this (to use a new callback.) Even when changes to data are reflected in the render we still see the stale onSelected is being used when lasso selecting bullet points.
This code snippet was used in a react hook with react-plotly version 2.4.0 and 2.3.0.
I have tried using big arrow and the "function" keyword but neither work. Looking at the source code for factory.js (https://github.com/plotly/react-plotly.js/blob/master/src/factory.js) I do see there is some handler replacement logic in "syncEventHandlers" when handlers are not the same to the props. I was thinking though that the updatePlotly function (that calls this synchronization might) might not be getting called during componentDidUpdate because the onSelected prop wasn't checked.
As a workaround right now I have to take my contexts and place them in a global wrapper so their changes are visible. Its not a good solution but I can't think of a better one and our contexts are mostly global for the entire dom tree anyways.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/factory.js, reading syncEventHandlers and the updatePlotly path during componentDidUpdate. Reproduce the lasso-selection case with changing context values and confirm that onSelected uses the latest prop callback rather than the initial one.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, react
- Domain
- data-visualization, frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100