Skip to content
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

Add subtitle to plots #7012

Merged
merged 51 commits into from
Jul 18, 2024
Merged

Add subtitle to plots #7012

merged 51 commits into from
Jul 18, 2024

Conversation

emilykl
Copy link
Contributor

@emilykl emilykl commented Jun 4, 2024

Closes #6856

Adds ability to add a subtitle to plots.

Screen Shot 2024-06-04 at 5 44 29 PM
  • Able to add subtitle using the following API:
    "layout": {
        "title": {
            "text": "A simple plot",
            "subtitle": {
                "text": "With a subtitle"
            }
  • Ability to change the font options of the subtitle by passing font options to layout.title.subtitle.font
  • Ability to edit the subtitle when editable: true

@archmoj archmoj marked this pull request as ready for review June 5, 2024 13:52
@archmoj
Copy link
Contributor

archmoj commented Jun 5, 2024

  • Subtitle should wrap automatically onto new lines when too long (maybe? Ideally yes but this is complicated)

I think word wrap is out of the scope of this PR. cc: #382

@archmoj
Copy link
Contributor

archmoj commented Jun 5, 2024

@emilykl Do you think that the height of subtitle should push the margins?
In your example if I set subtitle.text: '1st<br>2nd<br>3rd<br>4th' the subtitle would go over the cartesian subplot area.

@archmoj
Copy link
Contributor

archmoj commented Jun 5, 2024

This is quite minor.
But I noticed when config.editable is set to true and having an empty subtitle I get a black subtitle instead of gray on Click to enter Plot subtitle.

        "title": {
            "text": "A simple plot",
            "subtitle": {
                "text": ""
            }
        },

@archmoj
Copy link
Contributor

archmoj commented Jun 6, 2024

Just addressed a baseline update in #7019.
Please pull master and merge it into this branch.
Thank you!

@emilykl
Copy link
Contributor Author

emilykl commented Jun 6, 2024

@emilykl Do you think that the height of subtitle should push the margins? In your example if I set subtitle.text: '1st<br>2nd<br>3rd<br>4th' the subtitle would go over the cartesian subplot area.

@archmoj I would say "Yes, obviously", except that it looks like currently, even the height of the title doesn't push the margins -- if you add too many lines to the title, it will overlap the chart (see codepen).

So I think if we make the subtitle push the margins dynamically, we need to make that the behavior for the title as well.

I guess the other option would be to push the margin by a fixed amount if there is a subtitle present. I'm not sure whether that's necessary though; the current amount of space looks OK to me if the subtitle is one or two lines.

@emilykl
Copy link
Contributor Author

emilykl commented Jun 6, 2024

This is quite minor. But I noticed when config.editable is set to true and having an empty subtitle I get a black subtitle instead of gray on Click to enter Plot subtitle.

good catch, thanks. Do you know where this styling is set in the code?

I think this is probably also related to the bug I noticed where the size of the text currently being edited is too large for the subtitle.

@archmoj
Copy link
Contributor

archmoj commented Jun 17, 2024

For subtitles please add a French (or other languages that you know) translation to lib/locales e.g. here:

'Click to enter Plot title': 'Ajouter un titre au graphique',

src/plot_api/subroutines.js Outdated Show resolved Hide resolved
emilykl and others added 3 commits July 9, 2024 14:57
@emilykl
Copy link
Contributor Author

emilykl commented Jul 9, 2024

Also a mock like 19 that has a tile.text: 'Click to enter Plot title' it does not render a subtitle unless one changes the title!

@archmoj Good catch. I've modified the logic so that the subtitle is rendered even if there is no title (or the title is just a placeholder). But the placement is a bit odd since the subtitle is positioned according to the height of the title, and the title height is 0. Would you suggest any changes here? I'm inclined to leave as-is since this is a pretty rare edge case and doing anything more complicated would be kind of tricky and involve a bunch of extra logic.

Screen Shot 2024-07-09 at 4 29 35 PM

@archmoj
Copy link
Contributor

archmoj commented Jul 9, 2024

Also a mock like 19 that has a tile.text: 'Click to enter Plot title' it does not render a subtitle unless one changes the title!

@archmoj Good catch. I've modified the logic so that the subtitle is rendered even if there is no title (or the title is just a placeholder). But the placement is a bit odd since the subtitle is positioned according to the height of the title, and the title height is 0. Would you suggest any changes here? I'm inclined to leave as-is since this is a pretty rare edge case and doing anything more complicated would be kind of tricky and involve a bunch of extra logic.

Screen Shot 2024-07-09 at 4 29 35 PM

If an empty title (title: '') renders similar to a blank title (title: ' ') then yes this should be the correct behavior IMHO.

@archmoj
Copy link
Contributor

archmoj commented Jul 9, 2024

Similarly on the worldcup mock the positioning of subtitle could be improved.

This one is fixed now.

@@ -14,6 +14,8 @@ var interactConstants = require('../../constants/interactions');

var OPPOSITE_SIDE = require('../../constants/alignment').OPPOSITE_SIDE;
var numStripRE = / [XY][0-9]* /;
var MATHJAX_PADDING_MULTIPLIER = 0.85;
var EXTRA_SPACING_BETWEEN_TITLE_AND_SUBTITLE = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I kind of like the current positioning of subtitle which is compact;
I was wondering the correct value for this constant might be 1.3

LINE_SPACING: 1.3,

This is why:
If I call

Plotly.newPlot(gd, [{y: [1, 2]}], {title: {text: 'title', subtitle: {text: 'subtittle'}}});

the subtitle render closer compared to when I call

Plotly.newPlot(gd, [{y: [1, 2]}], {title: {text: 'title<br><sub>subtittle</sub>'}}); 

So perhaps you could try using (LINE_SPACING - 1) * title.font.size in your calculations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@archmoj Do you think the subtitle looks better with additional spacing?

I'd prefer not to change the spacing just to match the appearance of <sub>, since that spacing is arbitrary.

I prefer the more compact look.

@archmoj
Copy link
Contributor

archmoj commented Jul 11, 2024

Mock 28 has a title stating with a <br>. When the subtitle is added the position of it is not under the title.

This renders OK now after the fixes.
Thanks!

@archmoj
Copy link
Contributor

archmoj commented Jul 11, 2024

@emilykl Would you try taking into account the height of subtitle text in the automargin calculations?

Comment on lines +13276 to +13282
"subtitle": {
"editType": "layoutstyle",
"font": {
"color": {
"editType": "layoutstyle",
"valType": "color"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emilykl Do any of the property values inherit from layout.title?
Also, layout.title.subtitle.font.size is 70% of layout.title.font.size unless set?

@emilykl
Copy link
Contributor Author

emilykl commented Jul 16, 2024

@emilykl Would you try taking into account the height of subtitle text in the automargin calculations?

@archmoj Yes I am working on it, it's a bit tricky as the code for the automargin is pretty convoluted

@archmoj
Copy link
Contributor

archmoj commented Jul 18, 2024

Thanks @emilykl for this great PR 🏆
💃

@archmoj archmoj merged commit 9adfc12 into master Jul 18, 2024
1 check passed
@archmoj archmoj deleted the add-subtitle branch July 18, 2024 12:13
@hannahker
Copy link
Contributor

Wow thanks @archmoj @emilykl I was literally just looking for this feature this morning! 🥳

@Priyanshi3005
Copy link

Thanks for this feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support simple subtitle parameter (like title)
6 participants