Uh oh!
There was an error while loading. Please reload this page.
Conversation
| // If in compare mode, select every point at position | ||
| if(hoverData[0].length !== 0 && | ||
| ['x', 'y'].indexOf(mode) !== -1 && |
There was a problem hiding this comment.
concerning
['x','y'].indexOf(mode)!==-1creating a dynamic array of strings just for index checking might be slow.
Could we create a constant e.g.
varXY=['x','y'];at the upper scope and reuse that here?
There was a problem hiding this comment.
or even var XY = {x: 1, y: 1} -> if(XY[mode]) 😅
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| // Remove duplicated hoverData points | ||
| // note that d3 also filters identical points in the rendering steps | ||
| // TODO: use ES6 map |
There was a problem hiding this comment.
I don't think we can make that a TODO just yet - perhaps if & when we convert the whole syntax to ES6 we can make sure we include polyfills for this and other feature, THEN we can use it whenever we wish...
alexcjohnson
left a comment
There was a problem hiding this comment.
This looks great. Just a few nonblocking comments. 💃
antoinerg
commented
Mar 19, 2020
Great! I'll merge as soon as the tests pass |
archmoj
commented
Mar 19, 2020
Nicely done! |
Fixes#4656
Codepen: https://codepen.io/antoinerg/pen/gOpKwPg
First commit simply wrap existing logic into a function. Second commit changes the existing behavior.