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

Add 312-bslib-sidebar-resize #166

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Conversation

gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented May 8, 2023

Adds a manual testing app for plot and output resizing in sidebars (without cards).

Pairs with rstudio/bslib#566 where this behavior is improved for sidebars.

Expected behavior

In the app below, both the "static" ggplot2 output and the htmlwidget plotly plot output will stretch during the sidebar transition. The static plot will re-render when the transition is complete, so there's a small "snap" when the new plot is rendered. (No snap is seen in the case of an htmlwidget like plotly.)

All plots stretch during resize when the shared sidebar is toggled; only the sidebar in the local sidebar layout stretches when the local sidebar is toggled.

Kapture.2023-05-09.at.15.56.42.mp4

Failing behavior

When the behavior isn't present, neither the plot nor the htmlwidget's will update its width until after receiving a re-render event from the server.

Kapture.2023-05-09.at.15.55.44.mp4

@gadenbuie gadenbuie force-pushed the 312-bslib-sidebar-resize branch from 71f34d4 to daf5438 Compare May 9, 2023 16:00
@gadenbuie gadenbuie marked this pull request as ready for review May 9, 2023 16:02
@gadenbuie gadenbuie changed the title Add 312-bslib-sidebar-resize (manual app) Add 312-bslib-sidebar-resize May 9, 2023
@gadenbuie gadenbuie requested review from cpsievert and schloerke May 9, 2023 20:36
Copy link
Contributor

@schloerke schloerke left a comment

Choose a reason for hiding this comment

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

I'm happy that it is automated. I'll let you and @cpsievert work out the details

@gadenbuie gadenbuie requested a review from cpsievert May 15, 2023 17:45
@cpsievert
Copy link
Contributor

cpsievert commented May 18, 2023

@gadenbuie do you know why these tests fail pretty consistently on Windows (and sometimes on Mac)?

@gadenbuie
Copy link
Member Author

gadenbuie commented May 18, 2023

The Mac 3.6.3 errors are due to ggiraph

  Error: Error: package or namespace load failed for ‘ggiraph’:
   object 'guide_gengrob' not found whilst loading namespace 'ggiraph'
  Error: Error: loading failed
  Execution halted
  ERROR: loading failed

I was just starting to try to figure out how to look into the Windows failures (any ideas?)

gadenbuie added 4 commits May 18, 2023 15:33
This reverts commit b78e103.

Revert "debug: try again"

This reverts commit 2e443c3.

Revert "add debugging for windows and limit to just windows"

This reverts commit 50767b2.
@gadenbuie
Copy link
Member Author

There's some general flakiness in this PR that I want to resolve before merging. Carson and I talked through some options in person, one of which would be to do all of the sidebar size polling in the browser, rather than communicating back and forth via shinytest2.

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