plotly / plotly/react-plotly.js

onSelected uses stale function even when prop for onSelected changed

Aberta
#197 0 comentários 0 reações 0 responsáveis Ver no GitHub

Ninguém assumiu esta issue ainda.

Linguagem predominante
JavaScript
Estrelas
1.1k
Forks
138
Merge médio
3d 2h
PRs com merge (30d)
4

Descrição

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.

Guia de contribuição

Nenhum guia de contribuição indexado para este repositório

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Direção de pesquisa

Comece em src/factory.js, lendo syncEventHandlers e o caminho de updatePlotly durante componentDidUpdate. Reproduza o caso de seleção por laço com valores de contexto variáveis e confirme que onSelected usa o callback de prop mais recente, em vez do inicial.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
javascript, react
Domínio
data-visualization, frontend
Tipo de issue
Bug
Dificuldade
3/5
Tempo estimado
1-2 dias
Status de atividade
Estagnada
Clareza
Razoavelmente clara
Facilidade para iniciantes
30/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.