Skip to content

Template quirk #4980

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

Closed
alexcjohnson opened this issue Jul 3, 2020 · 6 comments
Closed

Template quirk #4980

alexcjohnson opened this issue Jul 3, 2020 · 6 comments
Labels
regression this used to work

Comments

@alexcjohnson
Copy link
Collaborator

A strange effect of #4904:

  • If you want to use the template to override default colors for axis lines that are hidden by default, now you need to explicitly hide those lines.
  • We decided in that PR that this behavior is more self-consistent, because that's how the regular figure attributes work: if you provide a color, it makes the line visible by default. So the template should do that too, otherwise it has a strange difference vs a regular figure.
  • EXCEPT that templates are often used just to provide new defaults but the intent is not to override other parts of the logic. In the case of axis lines this has two undesirable effects:
    • Sometimes the data itself causes the lines to show. For example bar charts by default turn on the zero line. But now if you have a template that recolors the zero line and then turns it back off, bar charts won't be able to automatically turn it back on.
    • If the user provides a new color for the line in the main figure, it won't cause the line to turn on, as it would without the template present.

Both of these are demonstrated in https://codepen.io/alexcjohnson/pen/VweyRJe?editors=1010 - without the template, both the xaxis line and the x=0 line show:
Screen Shot 2020-07-03 at 3 03 07 PM
But with the template they both disappear:
Screen Shot 2020-07-03 at 3 03 23 PM

Since there is a clear use case for both variants, I kind of think we need a new concept in the template, of a "new default" value, that doesn't count as having provided a value, so can't affect any other logic.

@archmoj @nicolaskruchten @wbrgss thoughts?

@nicolaskruchten
Copy link
Contributor

Tangentially related to #4482 I feel?

@archmoj
Copy link
Contributor

archmoj commented Jul 6, 2020

Tangentially related to #4482 I feel?

Good call. It seems so.

@wbrgss
Copy link
Contributor

wbrgss commented Jul 21, 2020

I initially understood this issue as affecting the zerolinecolor and linecolor, but I realize now it affects gridcolor as well in a Dash Enterprise product, and this is harder to work around AFAIK.

In a template, we set both the xaxis and yaxis default gridcolor. Before #4904 (v1.54.4), axis grid colors were displayed depending on the trace type. For example, scatter plots have both x and y gridlines displayed — but orientation: 'h' (horizontal) bar charts only had gridlines displayed on the x-axis, and orientation: 'v' (vertical) bar charts only displayed grid lines on the y-axis, etc. This logic applies to many other trace types as well. The advantage is that you can have a universal template, but keep the conditional logic (based on trace type and, in some cases, orientation) for displaying grid lines. I'm not too familiar with where this logic is in the code base or which traces it applies to.

Here are forks of @alexcjohnson's Codepen above to demonstrate this:

With plotly.js v1.54.3gridcolor is specified for both axes in the template

With plotly-latest (v1.54.6 at the time of this writing)

@nicolaskruchten
Copy link
Contributor

I'm increasingly thinking that we should revert to the previous behaviour.

@alexcjohnson
Copy link
Collaborator Author

I agree - let's revert #4904 and reopen the issues it closed, then we can come back to this when we have more time to develop a full solution for both flavors. I still consider those issues bugs, in as far as there are certain things you can't do with templates and other things that do not behave the same way in the template as they do in layout, but in the short term this fix - that turns out to be incomplete - causes more problems than it solves.

@gvwilson
Copy link
Contributor

gvwilson commented Jun 6, 2024

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

@gvwilson gvwilson closed this as completed Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression this used to work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants