-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Trace meta text #3865
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
Trace meta text #3865
Conversation
- to start accepting `meta: {key: value}` settings
... and shared colorbar use fullLayout._meta.
This looks good to me. I had forgotten that in 1.47, an un-prefixed Either way, this PR looks like we need, thanks! |
That will still work. The So, trace = {
meta: {yname: 'Y'},
name: '%{meta.yname} - %{meta[0]}'
}
layout = {
meta: ['Company B']
} will print |
I guess if there isn't a |
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.
Very nice implementation. Thanks @etpinard.
Please find my question below. Also two tests are failed on the CI.
@@ -97,8 +99,10 @@ function draw(gd, titleClass, options) { | |||
if(!editable) txt = ''; | |||
} | |||
|
|||
if(fullLayout.meta) { | |||
txt = Lib.templateString(txt, {meta: fullLayout.meta}); | |||
if(options._meta) { |
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.
Wondering if we need to bypass [ ]
or { }
here?
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.
No need. Lib.termplateString
handles those cases correctly.
The downside of falling back is that if today I have
And then I set It's not a huge concern either way. I think there are probably zero users of |
... during trace templateString calls.
"interesting" concern...
What will show up in the hover? What if I don't set |
Yeah, that's what I figured. OK for now, and probably forever, given the crazy recursion possibilities. Wouldn't want to make our hovertemplate system Turing-complete. |
Is everyone happy with the behaviour here? If so, @antoinerg would you mind reviewing this at some point today? Thanks! |
I’m happy! |
I was really happy until I realized the following test fails: Plotly.update(gd, {
meta: {pizza: 'is yummy'},
hovertemplate: 'TRACE -- %{meta.yname}<extra>%{meta.xname}</extra>'
}, {
meta: {yname: 'Yy', xname: 'Xx'}
})
.then(function() {
Fx.hover('graph', evt, 'xy');
assertHoverLabelContent({
nums: 'TRACE -- Yy',
name: 'Xx',
axis: '0.388'
});
}) If Anyway, I think my suggested behavior could be implemented by making Let me know what you think @nicolaskruchten @plotly/plotly_js |
I really think we should drop access to |
Oh, I see @nicolaskruchten! I didn't realize this was a conscious decision. @alexcjohnson can you confirm that you are OK with this? If so, then the PR would be good to go since its new behavior is well 🔒 down by the mock. |
Confirmed, I'm happy with this. |
💃 💃 |
Well, that's not what we went with here, right? here it uses |
Yes it does @nicolaskruchten.
Ok good. I'll let @etpinard decide on this one. |
well it's merged, so @etpinard already decided. Works for me, just wanted to make sure we're all on the same page. No further work or discussion required on this today :) |
a follow-up #3439 and #3548 - where we:
meta
to be set as an{}
(not just as an[]
) andmeta
info in each trace container.To make it easier for users to grab trace
meta
info within the trace wheremeta
is set, we proposed the following precedence rules:Given
Setting
but
%{meta.yname}
gives a blank string in layout fields (e.g.layout.xaxis.title.text
).One could still use
layout.meta
within the traces asMoreover, one can target trace
meta
info in layout fields asI hope @plotly/plotly_js will like it.