Bug fix. Account for multi_line=FALSE when facet rows/cols are missing. Closes #2341. by trekonom · Pull Request #2373 · plotly/plotly.R · GitHub
Skip to content

Bug fix. Account for multi_line=FALSE when facet rows/cols are missing. Closes #2341.#2373

Open
trekonom wants to merge 3 commits into
plotly:masterfrom
trekonom:issue-2341
Open

Bug fix. Account for multi_line=FALSE when facet rows/cols are missing. Closes #2341.#2373
trekonom wants to merge 3 commits into
plotly:masterfrom
trekonom:issue-2341

Conversation

@trekonom

@trekonom trekonom commented Aug 5, 2024

Copy link
Copy Markdown
Contributor

This PR proposes a fix to account for multi_line=FALSE in facet labeller functions, thereby closing #2341 .

With the proposed fix, the reprex from #2341 renders correctly without the undesired, additional strip texts for the rows:

library(plotly, warn = FALSE)
#> Loading required package: ggplot2

mtcars$cyl2 <- factor(
  mtcars$cyl,
  labels = c("alpha", "beta", "gamma")
)

ggplot(mtcars, aes(wt, mpg)) +
  geom_point() +
  facet_wrap(. ~ cyl2,
    labeller = label_wrap_gen(
      width = 25, multi_line = FALSE
    )
  )

ggplotly()

Created on 2024-08-24 with reprex v2.1.1

@trekonom

Copy link
Copy Markdown
Contributor Author

Comment thread R/ggplotly.R Outdated
Comment thread R/ggplotly.R Outdated
Comment on lines +943 to +952
row_txt <- if (!length(names(plot$facet$params$rows)) == 0) {
paste(
plot$facet$params$labeller(
lay[names(plot$facet$params$rows)]
),
collapse = br()
)
} else {
""
}

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.

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.

Thx for the review and the suggestions. Just committed both suggestions.

@cpsievert

Copy link
Copy Markdown
Collaborator

Thank you for the detailed explanation and contribution! This is looking very close to mergeable, I just had a couple stylistic suggestions

@kwilson62

Copy link
Copy Markdown

Looking forward to this getting fixed, currently experiencing the issue with "expression(list())"

@kwilson62

Copy link
Copy Markdown

@trekonom In addition to the bug you've fixed, there's also an issue where strip.position is not working in plotly.
Considering this issue has been around for awhile now, as seen in this issue:
#2103, and you seem to be the most recent / successful at fixing some of these facet issues, do you think you could take a crack at it?

trekonom and others added 2 commits September 6, 2024 08:58
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
@trekonom

trekonom commented Sep 6, 2024

Copy link
Copy Markdown
Contributor Author

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.

3 participants