Skip to content

Change ggplot legend theme back to default#69

Merged
Faye-yufan merged 3 commits intomasterfrom
fix-legend-default-size
Aug 12, 2022
Merged

Change ggplot legend theme back to default#69
Faye-yufan merged 3 commits intomasterfrom
fix-legend-default-size

Conversation

@Faye-yufan
Copy link
Copy Markdown
Contributor

According to the code review in PR #66 , when using static ggplot without html rendering, the legend text size would become too big.

@Faye-yufan
Copy link
Copy Markdown
Contributor Author

@tdhock Hi, I just changed the default size back in theme-default.r . It shall only be 16px for the legend text in animint/html.
But I'm not sure if this can resolve the problem on the first image in the review comment, and I think I need more details on this. So is that the legend text wrapped because of the font size?
image

Comment thread inst/htmljs/animint.js Outdated
.style("fill", function(d){return d["textcolour"]||1;})
.style("text-anchor", "middle")
.attr("font-size", function(d){return d["textsize"]||16;})
.attr("font-size", function(d){return d["textsize"]||1;})
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please use consistent indentation

@tdhock
Copy link
Copy Markdown
Collaborator

tdhock commented Jul 20, 2022

I don't think the text wrapping is a problem (that depends on the size of the browser window).
As long as the text size is reverted I think it should be OK.

Comment thread inst/htmljs/animint.js
Comment on lines 2098 to 2100
.style("text-anchor", "middle")
.attr("font-size", function(d){return d["textsize"]||1;})
.attr("font-size", function(d){return d["textsize"]||1;})
.text("a");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

indentation still does not match ...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it must be something wrong with my vscode. I just fixed some of the other code I wrote, is that ok?

@tdhock
Copy link
Copy Markdown
Collaborator

tdhock commented Aug 12, 2022

hi @Faye-yufan can you please merge this PR?

@Faye-yufan Faye-yufan merged commit 37082a1 into master Aug 12, 2022
@Faye-yufan Faye-yufan deleted the fix-legend-default-size branch August 12, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants