Inverse-transform text on layout transition #1616
Conversation
| @@ -17,6 +17,8 @@ var Drawing = require('../../components/drawing'); | |||
| var Axes = require('./axes'); | |||
| var axisRegex = /((x|y)([2-9]|[1-9][0-9]+)?)axis$/; | |||
|
|
|||
| var LAST_TRANSLATION_RE = /translate\([^)]*\)\s*$/; | |||
There was a problem hiding this comment.
Note: this will break or at least do something very unexpected if the last transform is not a vertical offset for the text. Not sure how to handle that more generally. The general pattern of parsing existing transforms with regular expressions is fragile but unavoidable without lots of changes to how things work.
There was a problem hiding this comment.
Can you just use Drawing.getTranslate instead?
There was a problem hiding this comment.
No, for two reasons, I think. First, the drawing function doesn't get the last translation explicitly, and second, it teases apart the numbers involved, which is unnecessary here. We only need to preserve the last transform without modification, not parse and reconstruct it.
|
Adding a class to text groups seems like a great idea - there aren't too many places this would be needed, and then we could ensure this is targeted exactly where we want it. For example, bars with text (eg test_dashboard/#bar_attrs_relative) often the text gets autosized, with a maximum font size that shrinks to fit if necessary, and even auto rotation. This seems like a disaster to replicate on the fly, so I'd vote to just omit bar text from this effect at least for now. Make sure it works with axis drag zooms and scroll zooms too! |
rreusser
force-pushed
the
text-layout-transition
branch
from
a1f2138 to
bf600bc
Compare
|
@alexcjohnson I've pushed a fix that fixes an error I'd made in applying the class and have refactored things into a method that sets the scale for a group of text points. The result works with transitions and zooming:
Last step is to 🔒 the drawing method logic with a quick test. Need to head back to conference, but will do that asap. |
|
Okay, added test. I'm 😄 |
|
💃 |
Addresses #1608. cc @alexcjohnson @n-riesco
See: http://rreusser.github.io/demos/plotly-unsupported/text-transition.html
(The hover/click behavior is frustratingly finicky, but that's a separate issue. Also the date axes just return NaN which is why things are weird with dates. That's not addressed here.)
As for this PR, I'm a little worried
.points gis too general a selector, so may need to add a class on text points to ensure only text is selected/modified by this tweak. Finally the transform logic is more than a little weird/ugly, but there's probably no avoiding that.