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

Implement legend.grouptitlefont and hoverlabel.grouptitlefont #6040

Merged
merged 7 commits into from Dec 10, 2021

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Nov 24, 2021

Resolves #5920.

@plotly/plotly_js

@alexcjohnson
Copy link
Contributor

Behavior looks great. My only question is about the attribute names/locations:

  • I think we should call it grouptitlefont ie no s on title - I see why you did this, because the new attributes apply to multiple titles, but if you try to construct a similar English sentence you wouldn't pluralize, like "Our team shirt color is red."
  • Instead of legendgrouptitlefont, can we do legend.grouptitlefont?

@archmoj archmoj modified the milestones: v2.7.0, v2.8.0 Dec 3, 2021
@archmoj
Copy link
Contributor Author

archmoj commented Dec 3, 2021

Behavior looks great. My only question is about the attribute names/locations:

  • I think we should call it grouptitlefont ie no s on title - I see why you did this, because the new attributes apply to multiple titles, but if you try to construct a similar English sentence you wouldn't pluralize, like "Our team shirt color is red."
  • Instead of legendgrouptitlefont, can we do legend.grouptitlefont?

Actually I did try the exact same changes but ran into some problems.

  1. For titles to title change, it appears there is some magical logic somewhere in the code that convert any titlefont to title.font. So we need to figure that out.
  2. For legend.grouptitlefont, it appears we need major changes in src/plots/plots.js while this value is needed in traces legendgrouptitle.font while the legend object still empty in the coerce process.

@alexcjohnson
Copy link
Contributor

For titles to title change, it appears there is some magical logic somewhere in the code that convert any titlefont to title.font. So we need to figure that out.

Probably just needs excluding here:

} else if(key.indexOf('titlefont') > -1) {
replace(key, key.replace('titlefont', 'title.font'));

For legend.grouptitlefont, it appears we need major changes in src/plots/plots.js while this value is needed in traces legendgrouptitle.font while the legend object still empty in the coerce process.

Ah right, because we supply legend defaults after traces, which we need to do so that we know whether to show the legend or not. I still think this is worthwhile, how about this: coerce legend.grouptitlefont inside legend/defaults.js, and then in the same place loop over traces and coerce trace.legendgrouptitle.font. We already have a loop over traces there, and we have access to the input trace and trace module attributes which I think is all you need.

@archmoj archmoj changed the title Implement legendgrouptitlesfont and hoverlabel.grouptitlesfont Implement legend.grouptitlesfont and hoverlabel.grouptitlesfont Dec 5, 2021
@archmoj archmoj changed the title Implement legend.grouptitlesfont and hoverlabel.grouptitlesfont Implement legend.grouptitlefont and hoverlabel.grouptitlefont Dec 5, 2021
@archmoj
Copy link
Contributor Author

archmoj commented Dec 5, 2021

Now we have the implentation for hoverlabel.grouptitlefont as well as legend.grouptitlefont attributes.

Copy link
Contributor

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 Thanks for the tweaks :)

@archmoj archmoj merged commit 1206902 into master Dec 10, 2021
@archmoj archmoj deleted the grouptitlesfont branch December 10, 2021 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] Set font size for legendgroup titles in layout
2 participants