Skip to content

Contour restyle zmin zmax#1653

Merged
etpinard merged 5 commits into
masterfrom
contour-restyle-zmin-zmax
May 9, 2017
Merged

Contour restyle zmin zmax#1653
etpinard merged 5 commits into
masterfrom
contour-restyle-zmin-zmax

Conversation

@etpinard

@etpinard etpinard commented May 8, 2017

Copy link
Copy Markdown
Contributor

fixes #1649

So, turns out that restyling zmin and zmax for contour traces has been broken since v1.0.0 and nobody noticed 😑

This PR fixes this by making zmin and zmax redo calcdata as the contour calc does use it. Note that this isn't the case for heatmap traces where zmin and zmax are only used in the plot step.

cc @alexcjohnson

@etpinard etpinard added status: reviewable bug something broken labels May 8, 2017
@etpinard etpinard added this to the 1.27.0 milestone May 8, 2017
Comment thread src/plot_api/plot_api.js

// some attributes declare a 'recalc' flag
if(valObject.recalc) {
flags.docalc = true;

@etpinard etpinard May 8, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

... slowly migrating to something better than those ugly recalcAttrs lists in restyle and relayout.

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.

Down the line, this is going to require valObject.dostyle, .doplot, .doCamera etc etc... would we be better off with something like valObject.editType = 'docalc' etc? Which presumably we could use like a flaglist (and validate as such in plotschema_test)?

💯 🍻 for getting this effort going!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

valObject.editType = 'docalc'

Right. I guess that scales better. Thanks 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done in 413e545

Comment thread src/plot_api/plot_api.js
// some attributes declare an 'editType' flaglist
if(valObject.editType === 'docalc') {
flags.docalc = true;
}

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 guess later this can change to something like

if(valObject.editType) {
    valObject.editType.split('+').forEach(function(flag) {
        flags[flag] = true;
    });
}

But don't need to do that now - lets wait until we have validation of editType, perhaps in plotschema_test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But don't need to do that now - lets wait until we have validation of editType, perhaps in plotschema_test?

Yep, that's what I was thinking. Referencing #648

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Looks good! 💃

@etpinard
etpinard merged commit 4e2a866 into master May 9, 2017
@etpinard
etpinard deleted the contour-restyle-zmin-zmax branch May 9, 2017 15:18
@etpinard etpinard mentioned this pull request Jun 27, 2017
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

restyle zmin & zmax doesn't cause contour maps to redraw

2 participants