Skip to content

Plotly.react #2341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 40 commits into from
Feb 9, 2018
Merged

Plotly.react #2341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
54b6969
Plotly.react
alexcjohnson Feb 2, 2018
aab4f11
remove flagAxForDelete from _restyle
alexcjohnson Feb 6, 2018
45c56b0
allow 0 in pushUnique
alexcjohnson Feb 7, 2018
e58b74d
test Plotly.react with component arrays
alexcjohnson Feb 7, 2018
1074804
_skipSD -> _skipDefaults
alexcjohnson Feb 7, 2018
0548db3
test of polar toggle staticPlot
alexcjohnson Feb 7, 2018
698fa4f
multiline if statements for better debugging
alexcjohnson Feb 7, 2018
3c56243
only add _private attrs to legend after supplyDefaults
alexcjohnson Feb 7, 2018
f01bab9
cleanData should modify textposition in place
alexcjohnson Feb 7, 2018
2fbd97c
don't slice uneven arrays in scatter and related traces
alexcjohnson Feb 7, 2018
f873b26
stop slicing and generating fake arrays in scatter3d and surface defa…
alexcjohnson Feb 7, 2018
5aaa0df
remove old obsolete exponentformat/showexponent fallback
alexcjohnson Feb 7, 2018
7a3b075
cleaner handling of bin defaults
alexcjohnson Feb 7, 2018
5814642
test_dashboard: highlight the displayed mock in search results
alexcjohnson Feb 8, 2018
8fbe06e
store slider & updatemenu dimensions in _private attrs
alexcjohnson Feb 8, 2018
afc05c9
stop slicing data arrays in scattergeo
alexcjohnson Feb 8, 2018
898c39d
hacky handling for reshaped arrays in Plotly.react
alexcjohnson Feb 8, 2018
e9270fc
handle transforms in Plotly.react - by looking at _fullInput
alexcjohnson Feb 8, 2018
414a9b8
clean up carpet pushing stuff back to _fullTrace late
alexcjohnson Feb 8, 2018
e982ca1
annotatinons3d pdata -> _pdata
alexcjohnson Feb 8, 2018
b856ad8
fix for 3d scenes modifying axes range to linear
alexcjohnson Feb 8, 2018
88ad7f2
test a bunch of mocks for noop react redraws
alexcjohnson Feb 8, 2018
ade3cca
review comments - :hocho: commented-out code, comment on cleanData
alexcjohnson Feb 8, 2018
4b98be6
fixes for mapbox with Plotly.react
alexcjohnson Feb 8, 2018
3178b2e
update polar for Plotly.react compatibility
alexcjohnson Feb 8, 2018
564dff6
stop filling new [] in pie defaults
alexcjohnson Feb 8, 2018
be4932d
fix bugs in rangeslider with explicit range and relayout
alexcjohnson Feb 8, 2018
0752aa5
clean up rangeselector for Plotly.react
alexcjohnson Feb 8, 2018
9ba4be9
update scatterternary for Plotly.react
alexcjohnson Feb 8, 2018
9795b69
avoid NaN in (un)selected.marker.opacity
alexcjohnson Feb 8, 2018
57c7d6c
fix for transformed histograms to give consistent _fullInput
alexcjohnson Feb 8, 2018
3a11062
more complete diffing of config
alexcjohnson Feb 8, 2018
896471c
update scattergeo test
alexcjohnson Feb 8, 2018
e12ce1b
wider acceptance range for toimage test
alexcjohnson Feb 8, 2018
46c6642
more complete set of Plotly.react noop mocks to test
alexcjohnson Feb 8, 2018
4197c32
lint
alexcjohnson Feb 8, 2018
f9d6151
fix scattergl uneven array handling
alexcjohnson Feb 9, 2018
c8a07e8
updated gl3d_ribbons mock
alexcjohnson Feb 9, 2018
2a7d640
reset package-lock to master
alexcjohnson Feb 9, 2018
5d6a4a0
get the right _xlength for surface if x is 2D
alexcjohnson Feb 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions devtools/test_dashboard/devtools.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ function searchMocks(e) {

results.forEach(function(r) {
var result = document.createElement('span');
result.className = 'search-result';
result.className = getResultClass(r.name);
result.innerText = r.name;

result.addEventListener('click', function() {
Expand All @@ -212,6 +212,10 @@ function searchMocks(e) {
// Clear plots and plot selected.
Tabs.purge();
Tabs.plotMock(mockName);

mocksList.querySelectorAll('span').forEach(function(el) {
el.className = getResultClass(el.innerText);
});
});

mocksList.appendChild(result);
Expand All @@ -222,8 +226,16 @@ function searchMocks(e) {
});
}

function getNameFromHash() {
return window.location.hash.replace(/^#/, '');
}

function getResultClass(name) {
return 'search-result' + (getNameFromHash() === name ? ' search-result__selected' : '');
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI just added this so I could click through the list of mocks and not lose track of which one I'm on... Something I'd wanted to do for a long time but now that I'm trying to load EVERY mock in sequence it became important enough to actually implement.


function plotFromHash() {
var initialMock = window.location.hash.replace(/^#/, '');
var initialMock = getNameFromHash();

if(initialMock.length > 0) {
Tabs.plotMock(initialMock);
Expand Down
3 changes: 3 additions & 0 deletions devtools/test_dashboard/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ header span{
color: #fff;
background-color: #4983EC;
}
.search-result__selected{
background-color: #DDDDEE;
}
#plots{
overflow: scroll;
}
Expand Down
44 changes: 22 additions & 22 deletions src/components/annotations/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module.exports = {
valType: 'boolean',
role: 'info',
dflt: true,
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you clarify why we now need arraydraw here and in shapes/attributes.js?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

relayout uses containerArrayMatch to figure out what edits need to be handled with the special add/remove/edit syntax, and arraydraw was just a placeholder for "relayout will handle this, don't worry about it".

Here we're letting the user do their own editing, so there's much less of that complexity. So now I use editType: 'arraydraw' as the signal to try and draw components using their draw or drawOne component methods, hence it has to actually be there!

Copy link
Contributor

Choose a reason for hiding this comment

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

Very clear. Thanks 📚

description: [
'Determines whether or not this annotation is visible.'
].join(' ')
Expand All @@ -29,7 +29,7 @@ module.exports = {
text: {
valType: 'string',
role: 'info',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the text associated with this annotation.',
'Plotly uses a subset of HTML tags to do things like',
Expand All @@ -42,14 +42,14 @@ module.exports = {
valType: 'angle',
dflt: 0,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the angle at which the `text` is drawn',
'with respect to the horizontal.'
].join(' ')
},
font: fontAttrs({
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
colorEditType: 'arraydraw',
description: 'Sets the annotation text font.'
}),
Expand All @@ -58,7 +58,7 @@ module.exports = {
min: 1,
dflt: null,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets an explicit width for the text box. null (default) lets the',
'text set the box width. Wider text will be clipped.',
Expand All @@ -70,7 +70,7 @@ module.exports = {
min: 1,
dflt: null,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets an explicit height for the text box. null (default) lets the',
'text set the box height. Taller text will be clipped.'
Expand Down Expand Up @@ -131,7 +131,7 @@ module.exports = {
min: 0,
dflt: 1,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the padding (in px) between the `text`',
'and the enclosing border.'
Expand All @@ -142,7 +142,7 @@ module.exports = {
min: 0,
dflt: 1,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the width (in px) of the border enclosing',
'the annotation `text`.'
Expand All @@ -153,7 +153,7 @@ module.exports = {
valType: 'boolean',
dflt: true,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Determines whether or not the annotation is drawn with an arrow.',
'If *true*, `text` is placed near the arrow\'s tail.',
Expand Down Expand Up @@ -198,7 +198,7 @@ module.exports = {
min: 0.3,
dflt: 1,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the size of the end annotation arrow head, relative to `arrowwidth`.',
'A value of 1 (default) gives a head about 3x as wide as the line.'
Expand All @@ -209,7 +209,7 @@ module.exports = {
min: 0.3,
dflt: 1,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the size of the start annotation arrow head, relative to `arrowwidth`.',
'A value of 1 (default) gives a head about 3x as wide as the line.'
Expand All @@ -219,15 +219,15 @@ module.exports = {
valType: 'number',
min: 0.1,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: 'Sets the width (in px) of annotation arrow line.'
},
standoff: {
valType: 'number',
min: 0,
dflt: 0,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets a distance, in pixels, to move the end arrowhead away from the',
'position it is pointing at, for example to point at the edge of',
Expand All @@ -241,7 +241,7 @@ module.exports = {
min: 0,
dflt: 0,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets a distance, in pixels, to move the start arrowhead away from the',
'position it is pointing at, for example to point at the edge of',
Expand All @@ -253,7 +253,7 @@ module.exports = {
ax: {
valType: 'any',
role: 'info',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the x component of the arrow tail about the arrow head.',
'If `axref` is `pixel`, a positive (negative) ',
Expand All @@ -266,7 +266,7 @@ module.exports = {
ay: {
valType: 'any',
role: 'info',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the y component of the arrow tail about the arrow head.',
'If `ayref` is `pixel`, a positive (negative) ',
Expand Down Expand Up @@ -333,7 +333,7 @@ module.exports = {
x: {
valType: 'any',
role: 'info',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the annotation\'s x position.',
'If the axis `type` is *log*, then you must take the',
Expand All @@ -351,7 +351,7 @@ module.exports = {
values: ['auto', 'left', 'center', 'right'],
dflt: 'auto',
role: 'info',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the text box\'s horizontal position anchor',
'This anchor binds the `x` position to the *left*, *center*',
Expand All @@ -370,7 +370,7 @@ module.exports = {
valType: 'number',
dflt: 0,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Shifts the position of the whole annotation and arrow to the',
'right (positive) or left (negative) by this many pixels.'
Expand All @@ -396,7 +396,7 @@ module.exports = {
y: {
valType: 'any',
role: 'info',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the annotation\'s y position.',
'If the axis `type` is *log*, then you must take the',
Expand All @@ -414,7 +414,7 @@ module.exports = {
values: ['auto', 'top', 'middle', 'bottom'],
dflt: 'auto',
role: 'info',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Sets the text box\'s vertical position anchor',
'This anchor binds the `y` position to the *top*, *middle*',
Expand All @@ -433,7 +433,7 @@ module.exports = {
valType: 'number',
dflt: 0,
role: 'style',
editType: 'calcIfAutorange',
editType: 'calcIfAutorange+arraydraw',
description: [
'Shifts the position of the whole annotation and arrow up',
'(positive) or down (negative) by this many pixels.'
Expand Down
4 changes: 2 additions & 2 deletions src/components/annotations3d/convert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ function mockAnnAxes(ann, scene) {
Axes.setConvert(ann._xa);
ann._xa._offset = size.l + domain.x[0] * size.w;
ann._xa.l2p = function() {
return 0.5 * (1 + ann.pdata[0] / ann.pdata[3]) * size.w * (domain.x[1] - domain.x[0]);
return 0.5 * (1 + ann._pdata[0] / ann._pdata[3]) * size.w * (domain.x[1] - domain.x[0]);
};

ann._ya = {};
Lib.extendFlat(ann._ya, base);
Axes.setConvert(ann._ya);
ann._ya._offset = size.t + (1 - domain.y[1]) * size.h;
ann._ya.l2p = function() {
return 0.5 * (1 - ann.pdata[1] / ann.pdata[3]) * size.h * (domain.y[1] - domain.y[0]);
return 0.5 * (1 - ann._pdata[1] / ann._pdata[3]) * size.h * (domain.y[1] - domain.y[0]);
};
}
2 changes: 1 addition & 1 deletion src/components/annotations3d/draw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ module.exports = function draw(scene) {
.select('.annotation-' + scene.id + '[data-index="' + i + '"]')
.remove();
} else {
ann.pdata = project(scene.glplot.cameraParams, [
ann._pdata = project(scene.glplot.cameraParams, [
fullSceneLayout.xaxis.r2l(ann.x) * dataScale[0],
fullSceneLayout.yaxis.r2l(ann.y) * dataScale[1],
fullSceneLayout.zaxis.r2l(ann.z) * dataScale[2]
Expand Down
16 changes: 8 additions & 8 deletions src/components/legend/defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ var helpers = require('./helpers');


module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {
var containerIn = layoutIn.legend || {},
containerOut = layoutOut.legend = {};
var containerIn = layoutIn.legend || {};
var containerOut = {};

var visibleTraces = 0,
defaultOrder = 'normal',
defaultX,
defaultY,
defaultXAnchor,
defaultYAnchor;
var visibleTraces = 0;
var defaultOrder = 'normal';

var defaultX, defaultY, defaultXAnchor, defaultYAnchor;

for(var i = 0; i < fullData.length; i++) {
var trace = fullData[i];
Expand Down Expand Up @@ -58,6 +56,8 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {

if(showLegend === false) return;

layoutOut.legend = containerOut;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

legend's visibility is controlled by showlegend, which is outside the legend container itself, unlike most of our newer containers that have visible inside the container. Unless and until we change that (v2?) the legend container should not exist when it's not needed. This was causing me problems as relinkPrivateKeys was humorously removing newFullLayout.legend if it was empty and oldFullLayout.legend existed... so its existence would toggle on Plotly.react calls, making it look like the legend was always in need of updating 💫

Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice catch.

until we change that (v2?)

I'd vote +1 here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added to #420

Copy link

Choose a reason for hiding this comment

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

the legend container should not exist when it's not needed.

so by default, in _fullLayout, the .legend key wouldn't be there?
we read from _fullLayout to display widgets in the editor, if there's no .legend key, or a ._fullLayout.legend.visible key, then the widget won't display..

We could have a work around, but it's kind of one of the basic premises of widget visibility in the editor..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@VeraZab you must not have been dependent on the existence of a legend container up to now, as it was unreliable ^^ - showlegend is still there, but now it's reliably the case that if _fullLayout.showlegend=false there will be NO _fullLayout.legend container.


coerce('bgcolor', layoutOut.paper_bgcolor);
coerce('bordercolor');
coerce('borderwidth');
Expand Down
Loading