css visibility is overridden by plotly

[fresheneesz]

If you use css visibility:hidden on an element containing a plotly graph, you can still see the axes numbers and titles.

[rreusser]

The reason for this is that visibility of text nodes is explicitly set to visibility: 'visible'.

@etpinard, do you know for sure whether that line is strictly necessary or not? Could it be set only when necessary? Could visibility: 'inherit' lead to easier overriding? Plotly seems to work fine if I simply remove that line, though I'm sure there are many more uses than I could possibly have tested at a quick glance.

As a workaround, opacity: 0 hides the plot, though I'm not sure offhand how to set pointer-events: 'none' without just placing an empty div over the plot that intercepts pointer events.

[etpinard]

Controlling the elements inside a plotly.js graph div with CSS is a bad idea to begin with.

plotly.js graph are meant to be fully and strictly determined by the data / layout JSON serializable API.

We could set visibility: 'inherit' instead of visibility: 'visible' in this case, but I'm afraid that won't solve all your issues when pointing CSS to plotly.js elements.

So, I'd vote for type: wontfix. Any objections?

[rreusser]

I sympathize that it's nice if properties aren't hard-coded unnecessarily, but I also agree that it's not a good idea in general to expect CSS properties to meaningfully filter into an SVG. I'd vote for wontfix, particularly since there's not a more general path forward getting SVG to play well with CSS and since a combination of opacity and, if necessary, transparent divs that mask pointer events (which even more so can't be made to play well with CSS) seem to be a reasonably straightforward workaround.

[fresheneesz]

@etpinard I don't think this is an acceptable stance for a library built to be used in a browser environment. You simply aren't going to have graphs outside the context of the dom, and therefore there are a minimum of things that plotly should do to play nice with the dom, including sizing correctly within its parent element, and reacting to things like visibility and opacity of ancestor elements.

Please reopen so we can continue to discuss this for more than 23 hours, this topic deserves more than 3 opinions.

[rreusser]

Apologies on the hasty closure and for my likely-undue skepticism, @fresheneesz. I'm not particularly familiar with the text utilities, but there's quite a bit going on there, so I had a queasy feeling about changes leading to more changes leading to more changes since it's in a part of plotly that touches essentially everything. I've submitted PR #990. All of the tests pass, though there is certainly a possibility of untested regressions. If visibility: inherit is indeed compatible with visibility: visible and plays more nicely and doesn't lead to messy regressions, then perhaps it's worth reconsidering. cc @etpinard

[fresheneesz]

Thanks rreusser!

[rreusser]

For just a bit more context, @fresheneesz, I'd go as far as to say (correct me if I'm wrong!) that the fundamental design principle of plotly.js is that the JSON definition of the plot should determine effectively everything about how the plot looks. This is the property that makes the plots editable, embeddable and portable via the JSON definition only. The goal then is to eject most css interactions so that stray user styles don't break this assumption. inherit explicitly pulls in external properties, so that the JSON no longer describes what the plot looks like. Upon thinking about this more critically, I think this is probably a better explanation of why it tripped my "hmm… something's not quite right" circuit. This may be a case though where the most reasonable interaction is to simply hide the whole plot—which isn't particularly interesting as far as strange, problematic interactions go. So that's my more-carefully-considered rationale for both the pros and cons of considering the change. 😪

[etpinard]

@fresheneesz I apologise if that fast type: wontfix + close felt rude to you. I just didn't want to get your hopes up. For the foreseeable, plotly.js graph will remain fully and strictly determined by the data / layout JSON serializable API.

[fresheneesz]

"the JSON no longer describes what the plot looks like"

I don't think making the plot entirely invisible (via visibility:none or display:hidden) means that the JSON no longer describes how the plot looks. In what case would reacting to css visibility and display properties harm the portability of the JSON definition?

"plotly.js graph will remain fully and strictly determined by the data / layout JSON serializable API"

I can see the usefulness of portable JSON definitions, but I don't think what I'm suggesting in any way conflicts with that policy.

[etpinard]

@fresheneesz my mistake. I thought you were talking about applying CSS to elements inside the plotly graph <div> as opposed to the graph <div> itself as in this example: http://codepen.io/etpinard/pen/VKzKgV

This is bug. The graph <div> passed to Plotly.plot should still belong to the user.

Next time, please attach a reproducible example to make your point clearer.

[fresheneesz]

Ah ok, will do. Glad we got on the same page.

[etpinard]

fixed in #990