Skip to content

fix: scale exploration plot height dynamically with facet count#1284

Draft
Gero1999 wants to merge 2 commits into
mainfrom
1283-enhancement/dynamic-facet-plot-height
Draft

fix: scale exploration plot height dynamically with facet count#1284
Gero1999 wants to merge 2 commits into
mainfrom
1283-enhancement/dynamic-facet-plot-height

Conversation

@Gero1999

Copy link
Copy Markdown
Collaborator

Issue

Closes #1283

Description

When faceting exploration plots by a variable with many levels (e.g. USUBJID), the subplots in the middle rows shrink and become unreadable. This happened because the plot container used a fixed viewport height (height = \"90vh\", fillable = TRUE) and plotlyOutput(height = \"100%\") — plotly tried to fit all panels within that fixed space.

Changes:

  • Added .facet_plot_height() helper that computes a dynamic pixel height based on the number of facet panels (300px per row, minimum 500px). Returns NULL for plots with ≤4 panels so small plots are unaffected.
  • Applied the helper to both individual and mean plot renderPlotly calls.
  • Switched layout_sidebar containers from fillable = TRUE to fillable = FALSE and removed height = \"100%\" from plotlyOutput, so large faceted plots extend beyond the viewport and scroll naturally.

Definition of Done

  • Exploration plots (individual and mean) scale their height dynamically based on the number of facet panels
  • Middle-row subplots render at the same size as top/bottom rows
  • Large faceted plots are scrollable within the exploration tab
  • Existing plots with few facets (≤4) are not affected

How to test

  1. Upload a dataset with many subjects (20+)
  2. Go to Exploration > Individual Plots
  3. Set Facet By to USUBJID
  4. Verify all subplots render at equal size and the plot is scrollable
  5. Switch to a facet variable with few levels (e.g. PCSPEC) — verify the plot looks the same as before

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • App or package changes are reflected in NEWS
  • Package version is incremented
  • R script works with the new implementation (if applicable)
  • Settings upload works with the new implementation (if applicable)
  • If any .scss change was done, run data-raw/compile_css.R

Notes to reviewer

  • The .facet_plot_height() helper uses p$facet and p$data to count panels without re-querying the data. It only activates for FacetWrap with >4 panels.
  • The QC plot module (qc_plot.R) already uses a similar dynamic height pattern but computes it differently (based on subjects × groups). This helper is simpler since it only needs the facet panel count.
  • Switching fillable = FALSE changes the layout behavior — the plot area now scrolls independently. Worth testing that sidebar interactions (zoom, hover) still work correctly with the scrollable container.

Gero1999 and others added 2 commits April 30, 2026 09:57
Plots with many facet panels (e.g. facet by USUBJID) had middle rows
shrunk because the container used a fixed viewport height. Now the
plotly height scales with the number of facet rows (300px per row,
min 500px). Containers switched to fillable=FALSE so large plots
scroll instead of compressing.

Co-authored-by: Ona <no-reply@ona.com>
Resolve NEWS.md conflict: keep both bug fix entries (branch's #1283
dynamic facet height and main's #1273 ratio aggregation fix).

Co-authored-by: Ona <no-reply@ona.com>
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.

Enhancement: exploration plots shrink when faceting by many levels (e.g. USUBJID)

1 participant