Skip to content

Adjust imaginary tickvals default to fix react - #5992

Merged
archmoj merged 4 commits into
masterfrom
fixup-smith-react-test
Oct 29, 2021
Merged

Adjust imaginary tickvals default to fix react#5992
archmoj merged 4 commits into
masterfrom
fixup-smith-react-test

Conversation

@archmoj

@archmojarchmoj commented Oct 19, 2021

Copy link
Copy Markdown
Contributor

The magical default for imaginary tickvals added in #5956 (comment) broke the react ("noCi" plotl_api_react_test).

@plotly/plotly_js

@archmojarchmoj added this to the v2.6.0 milestone Oct 20, 2021
var imagTickvalsDflt =
realTickvals.slice().reverse().map(function(x) { return -x; })
.concat([0])
.concat(realTickvals);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine the problem with this is we get a new array each time, so Plotly.react thinks this has changed and needs replotting? I do think as a default this behavior makes sense, can we fix it with a simple memoizer? Something like:

functionmemoize(fn,keyFn){varcache={};returnfunction(val){varnewKey=keyFn ? keyFn(val) : val;if(newKeyincache){returncache[newKey];}out=fn(val);cache[newKey]=out;returnout;}}varmakeImagDflt=memoizeOne(function(realTickvals){returnrealTickvals.slice().reverse().map(function(x){return-x;}).concat([0]).concat(realTickvals);},String);vara=makeImagDflt([2,4,5])varb=makeImagDflt([2,4,5])a===b// truevarc=makeImagDflt([1,2,3])vard=makeImagDflt([2,4,5])d===b// true

A memoizer might be useful in other contexts too... we use them quite a lot in Dash anyway.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a kind of magic!
Applied in 844e24c

@alexcjohnsonalexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 🧙

@archmojarchmoj changed the title Simplify imaginary tickvals default to fix reactAdjust imaginary tickvals default to fix reactOct 29, 2021
@archmoj
archmoj merged commit d38232d into masterOct 29, 2021
@archmoj
archmoj deleted the fixup-smith-react-test branch October 29, 2021 20:26
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugsomething broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@archmoj@alexcjohnson