Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2k
Re-add support for passingtitle as string#7230
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| ].join(' ') | ||
| } | ||
| }, | ||
| _deprecated:{ |
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.
How about dropping thedeprecation 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.
Hmm.. I see then it would collide with the the title which is declared as an object.
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.
I think the best way of handing these cases today is to change the titlevalType to beany and then support bothString andObject types so that whentitle is a stringtitle.text is set to the string.
How about opening a new PR with this approach?
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.
@archmoj That sounds fine but I'm unsure about theattributes syntax for accepting both an object (with specific nested keys) and a string... this is the attribute definition right now fortitle 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 definevalType fortitle if we are also defining nested attributes undertitle. Do you have a suggestion?
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.
Let's see what@alexcjohnson would suggest 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.
How about keeping thevalType asObject with and add anextra option for this case?
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.
Or perhaps we should just handle this String option on the plotly.py side?
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.
@archmoj Do you have an example of anextra option being used elsewhere in the codebase?
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.
Something like this:https://github.com/plotly/plotly.js/pull/6990/files#r1591049384
Instead ofextra you may add another option to coerce and name itstringOk or something else.
archmoj commentedOct 17, 2024
We should possibly support this option on places e.g. |
emilykl commentedOct 17, 2024
Agreed, it makes sense to be consistent, I'll look into how much work this would be. |
archmoj commentedNov 5, 2024
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 | ||
| } |
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.
Uh oh!
There was an error while loading.Please reload this page.
titleproperty (removed inDrop support for passing a string to thetitleattribute, and drop support for deprecated attributestitlefont,titleposition,titleside, andtitleoffset#7212)titlefont,titleoffset) remain removedtitlestring totitle.textinternally