plotly / plotly/plotly.js

redraw <chart type> with no changes is not a noop (svg mocks)

Open
#7,347 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug testing
Dominant language
JavaScript
Stars
18.3k
Forks
2k
Avg merge
2d 12h
Merged PRs (30d)
28

Description

  • We traced it back to 2.24.0 (https://github.com/plotly/plotly.js/releases/tag/v2.24.0), which includes a group of PR's that correspond to the exact same figure types that are failing. For example, in the sunburst PR, there's a line that updates the trace marker color.
  • From @alexcjohnson:
    • In sunburst/style.js (which is part of the plotting pipeline) we have
    if(marker.pattern) {
        if(!marker.colors || !marker.pattern.shape) marker.color = cdi.color;
    } else {
        marker.color = cdi.color;
    }
    
    • ie we’re changing _fullData during plotting. Which is a big no no. So yeah that’s where the noop is being broken, we should avoid that, which I bet in this case means creating a new mock trace object to pass into Drawing.pointStyle(s, trace, gd, pt); rather than modifying this one. But of course since this has been around for a year and a half, this is not a release blocker.
    • Also I’ll note, the code in question is in styleOne, meaning that it’ll be called once for every segment of the sunburst… so if you do go with a mock trace object, put the object creation up in style (which only happens once per trace) instead of in styleOne, and styleOne can keep reusing that same object.
      If it helps, you’re free to attach new things to the trace object during plotting as long as they start with _.
    • I’m thinking about things like here where we mock an axis in order to reuse logic from regular axis handling in 3D axes… or here where we mock the entire figure in the course of making a new shrunken version of the figure for rangesliders
  • Update after looking into it a bit: The code referenced above is now is fill_one.js. Added in this commit

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in fill_one.js and trace the style/styleOne path into Drawing.pointStyle(s, trace, gd, pt). Check how SVG mocks and fullData are handled during redraws, especially across the affected chart types. Done means redrawing an unchanged chart leaves the underlying data untouched and behaves as a noop.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
data-visualization
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.