Skip to content

theme gains a new element panel.merge to control whether to merge anel area into single gTree #6398

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

Closed
wants to merge 1 commit into from

Conversation

Yunuuuu
Copy link
Contributor

@Yunuuuu Yunuuuu commented Apr 5, 2025

Try to fix #6396

@Yunuuuu
Copy link
Contributor Author

Yunuuuu commented Apr 7, 2025

I'm not sure whether the current implementation will be accepted. I've reviewed the tests—while some changes are present, there don’t appear to be any visual differences. The current error is due to the vignette, which I plan to tweak once the implementation itself is accepted.

@teunbrand
Copy link
Collaborator

Hi Yunuuuu,

Thanks for putting together the PR to fix the issue, however we feel that the added complexity of the code does not result in sufficient utility.
Moreover, several reverse dependencies (like patchwork) may also rely on the current panel structure. While I understand why you'd want a panel spanning dendrogram, I do not think viewport-based tricks will be the cleanest path to achieve what you want. Instead, I'll suggest that you implement a Facet subclass where you have control over placing panels and annotations. Moreover, because facets have access to scales, you should be able to derive the break locations of discrete scales, so you don't have to rely on these viewports.
Because we feel a superior alternative is available and we're reluctant to add this level of complexity for a niche feature, we're closing this PR.

@teunbrand teunbrand closed this Apr 8, 2025
@Yunuuuu
Copy link
Contributor Author

Yunuuuu commented Apr 9, 2025

After testing, I found it difficult to handle this using a single Facet object alone, since the panels input to Facet$draw_panels() is unpredictable. It’s challenging to break down panels into individual layer grobs.

Several issues contribute to this unpredictability:

  • The Layer$draw_geom method doesn't always return a grob if the developer doesn’t follow the expected conventions. No error or warning is issued in such cases, and gList(NULL) or gList(gList()) simply works silently.
  • Coord$draw_panel will sometimes wrap all layer grobs into single gTree in CoordRadial, sometimes it won't

Solutions:

  • Ensure that Layer$draw_geom() always returns a single grob or a gTree.
  • always wrap the all layer grobs into a single gTree before passing to ``Coord$draw_panel`

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.

Feature request: drawing ordering from layer to layer instead of panel to panel
2 participants