-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
Add surfacecolor
functionality to surface traces
#347
Conversation
- so that Plotly.restyle is also kept backward compatible.
- straight as 'intensityBounds' in 'surfacecolor' case - scaled by scaleFactor in 'z' case
|
||
function mapLegacy(traceIn, oldAttr, newAttr) { | ||
if(oldAttr in traceIn && !(newAttr in traceIn)) { | ||
traceIn[newAttr] = traceIn[oldAttr]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mdtusz @cldougl @chriddyp Are you ok with this way of preserving backward compatibility?
zmin
, zmax
and zauto
are deprecated in favour of cmin
, cmax
and cauto
for surface traces.
The deprecated z
attributes are mapped to their corresponding c
attributes in the defaults step so that both plot
and restyle
call update the trace objects to the c
attributes.
If both a c
and z
attribute are present, only the c
attribute is coerced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep I'm ok with this. Ideally we would/should be able to remove the oldAttr
from traceIn
as well, but I imagine that may cause problems if there are other components that make use of it somewhere still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but I imagine that may cause problems if there are other components that make use of it somewhere still.
Exactly. That's why I took the conservative route here. We'll clean this up in v2.0.0
🎉 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok for me
zmax: colorscaleAttrs.zmax, | ||
surfacecolor: { | ||
valType: 'data_array', | ||
description: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a more descriptive description? I'm still a little unclear on how this is used.
💃 from me. |
nice image test 🌈 |
Add `surfacecolor` functionality to surface traces
resolves #308
@mdtusz @mikolalysenko
This PR coupled with gl-vis/gl-surface3d#7 take the work started in #314 to the finish line.