Move group and filter transforms to main #536
Conversation
| if (['scatter', 'bar', 'scattergl'].indexOf(fullContainer.type) === -1) { | ||
| const transformableCharts = ['scatter', 'bar', 'scattergl']; | ||
|
|
||
| if (transformableCharts.indexOf(fullContainer.type) === -1) { |
There was a problem hiding this comment.
I think you can use includes here
| const transformBy = | ||
| container.transforms && | ||
| container.transforms.map(tr => { | ||
| let retValue = ''; |
There was a problem hiding this comment.
This could be const with a ternary operator
There was a problem hiding this comment.
retValue? I don't see how - it can have 3 values, including ''
- if groupsrc
- if targetsrc
- if neither
There was a problem hiding this comment.
what is retValue, returnValue?
would you mind giving it a name that says more what it's used for,
I guess this one's for the label, so columnLabel?
dmt0
force-pushed
the
move-transforms-to=main
branch
2 times, most recently
from
852b11d to
e4b328c
Compare
|
I see some things are not displaying in the transforms panel. I blame Percy configuration at this point. Percy snapshot also prints a bunch of stuff into the console, such as: Tried adding data sources to Percy test fixture - doesn't solve it. |
|
Ahhh interesting! We shouldn’t be using Plotly-basic!!
…On Fri, Jun 29, 2018 at 17:58 Dmitry Shvedov ***@***.***> wrote:
I see some things are not displaying in the transforms panel. I blame
Percy configuration at this point. Percy snapshot also prints a bunch of
stuff into the console, such as:
plotly-basic.js:36143 WARN: Unrecognized transform type aggregate etc.
for other transform types...
Things work OK <http://prntscr.com/k0zb9b> when I run RCE locally.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#536 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMbA3AAq0StWwFjWLxqMP1j2tRDIbeeks5uBqMWgaJpZM4U9jr_>
.
|
|
Interesting indeed. When I use plotly or plotly-with-meta, I get |
|
Perhaps. Worth talking to the plotly.js guys about approaches to testing. Perhaps there are or could be private methods that would allow us to populate |
|
try to play with various other bundles too: with/without gl support...
…On Fri, Jun 29, 2018 at 18:18 Dmitry Shvedov ***@***.***> wrote:
Interesting indeed. When I use plotly or plotly-with-meta, I get
TypeError: window.URL.createObjectURL is not a function
Perhaps plotly-basic is the only one that doesn't assume that there's a
browser window?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#536 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMbA2jkAaU7yXmjZgHpmE9S08zNqAbFks5uBqeygaJpZM4U9jr_>
.
|
|
For the new traces tests you added (histogram, histogram2d, ..), would you mind adding them with data (x or y values). There's more widgets that appear sometimes when the trace is drawn. In this case, I would have just liked seing the default trace orientation be selected. |
|
@VeraZab is data missing? what do you mean? |
dmt0
force-pushed
the
move-transforms-to=main
branch
2 times, most recently
from
74e9c3d to
c49dfa7
Compare
dmt0
force-pushed
the
move-transforms-to=main
branch
from
c49dfa7 to
97ea7b4
Compare
|
💃 |
Closes #444