Re-add support for passing title as string
#7230
Conversation
| @@ -228,4 +228,10 @@ module.exports = overrideAll({ | |||
| ].join(' ') | |||
| } | |||
| }, | |||
| _deprecated: { | |||
There was a problem hiding this comment.
How about dropping the deprecation here?
There was a problem hiding this comment.
Hmm.. I see then it would collide with the the title which is declared as an object.
There was a problem hiding this comment.
I think the best way of handing these cases today is to change the title valType to be any and then support both String and Object types so that when title is a string title.text is set to the string.
How about opening a new PR with this approach?
There was a problem hiding this comment.
@archmoj That sounds fine but I'm unsure about the attributes syntax for accepting both an object (with specific nested keys) and a string... this is the attribute definition right now for title in colorbar:
title: {
text: {
valType: 'string',
description: 'Sets the title of the color bar.'
},
font: fontAttrs({
description: 'Sets this color bar\'s title font.'
}),
side: {
valType: 'enumerated',
values: ['right', 'top', 'bottom'],
description: [
'Determines the location of color bar\'s title',
'with respect to the color bar.',
'Defaults to *top* when `orientation` if *v* and ',
'defaults to *right* when `orientation` if *h*.',
].join(' ')
}
},
My point is that there doesn't seem to be a place to define valType for title if we are also defining nested attributes under title. Do you have a suggestion?
There was a problem hiding this comment.
Let's see what @alexcjohnson would suggest here.
There was a problem hiding this comment.
How about keeping the valType as Object with and add an extra option for this case?
There was a problem hiding this comment.
Or perhaps we should just handle this String option on the plotly.py side?
There was a problem hiding this comment.
@archmoj Do you have an example of an extra option being used elsewhere in the codebase?
There was a problem hiding this comment.
Something like this: https://github.com/plotly/plotly.js/pull/6990/files#r1591049384
Instead of extra you may add another option to coerce and name it stringOk or something else.
|
We should possibly support this option on places e.g. |
Agreed, it makes sense to be consistent, I'll look into how much work this would be. |
archmoj
requested review from
alexcjohnson,
ndrezn,
LiamConnors,
marthacryan and
gvwilson
|
Revised in #7262. |
| }, | ||
| { | ||
| desc: 'despite passing title only as a string (backwards-compatibility)', | ||
| update: { | ||
| title: NEW_TITLE, | ||
| xaxis: {title: NEW_XTITLE}, | ||
| yaxis: {title: NEW_YTITLE} | ||
| } | ||
| }, | ||
| { | ||
| desc: 'despite passing title only as a string using string attributes ' + | ||
| '(backwards-compatibility)', | ||
| update: { | ||
| title: NEW_TITLE, | ||
| 'xaxis.title': NEW_XTITLE, | ||
| 'yaxis.title': NEW_YTITLE | ||
| } |
titleproperty (removed in Drop support for passing a string to thetitleattribute, and drop support for deprecated attributestitlefont,titleposition,titleside, andtitleoffset#7212)titlefont,titleoffset) remain removedtitlestring totitle.textinternally