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

fix(306,308,310): Sidebar and accordion updates #231

Merged
merged 25 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e8ec836
fix: Disable transitions in sidebar tests with screenshots
gadenbuie Oct 23, 2023
d8893ac
chore: Need at least a small amount of transition time
gadenbuie Oct 24, 2023
a76aec1
chore: Avoid warning about re-used inputId
gadenbuie Oct 24, 2023
abc1ef3
tests(310): Update snapshots
gadenbuie Oct 24, 2023
c1db883
tests(308): Update snaps
gadenbuie Oct 24, 2023
bd98187
tests(308): Screenshot after a delay
gadenbuie Oct 24, 2023
ebd1562
tests(308): Try longer delay
gadenbuie Oct 24, 2023
528680e
tests(308): Temporarily disable sidebar transition
gadenbuie Oct 24, 2023
fb14d40
tests(308): Update snaps for fixed test
gadenbuie Oct 24, 2023
9ce15c6
tests(310): Update snaps with sidebar padding-top on mobile fix
gadenbuie Oct 26, 2023
dc18512
chore(308): Rename test file
gadenbuie Oct 26, 2023
58c3b75
tests(308): New snaps
gadenbuie Oct 26, 2023
4081ca9
test(306): Rename test file and snapshots
gadenbuie Oct 26, 2023
46f7307
tests(306): Fix snapshot timing
gadenbuie Oct 26, 2023
b128f3b
tests(308): Use latest chromote features
gadenbuie Oct 27, 2023
b4f702c
tests(306): Update for latest chromote
gadenbuie Oct 27, 2023
d906e10
test(310): Use latest chromote
gadenbuie Oct 27, 2023
e8800db
Generate apps deps (GitHub Actions)
gadenbuie Oct 27, 2023
4b2b3f2
test(306): Update snaps for all (using mac 4.3 snaps)
gadenbuie Oct 30, 2023
4d94f30
tests(308): Accept new snaps
gadenbuie Oct 30, 2023
9d0a0ff
tests(310): new snaps
gadenbuie Oct 30, 2023
c0d6c65
Merged origin/main into sidebar/padding-transition
gadenbuie Oct 31, 2023
af84fd2
chore(deps): Use dev chromote
gadenbuie Oct 31, 2023
a60c49c
`devtools::build_readme()` (GitHub Actions)
gadenbuie Oct 31, 2023
9e2bcfe
tests: accept update snaps
gadenbuie Oct 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added .Rprofile
Empty file.
6 changes: 3 additions & 3 deletions R/data-apps-deps.R
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ apps_deps_map <- list(`001-hello` = "rsconnect", `012-datatables` = "ggplot2",
"sf", "withr"), `302-bootswatch-themes` = c("ggplot2", "progress",
"rversions", "sf", "withr"), `304-bslib-card` = c("rlang",
"rversions"), `305-bslib-value-box` = c("rlang", "rversions"
), `306-accordion-add-remove` = "magrittr", `308-sidebar-kitchen-sink` = c("lorem",
"rversions", "testthat"), `309-flexdashboard-tabs-navs` = "rmarkdown",
`310-bslib-sidebar-dynamic` = c("rversions", "testthat"),
), `306-accordion-add-remove` = "magrittr", `308-sidebar-kitchen-sink` = c("jsonlite",
"lorem", "testthat"), `309-flexdashboard-tabs-navs` = "rmarkdown",
`310-bslib-sidebar-dynamic` = c("jsonlite", "testthat"),
`311-bslib-sidebar-toggle-methods` = c("rversions", "testthat"
), `313-bslib-card-tab-focus` = c("rversions", "testthat",
"withr"), `314-bslib-tooltips` = "withr", `315-bslib-input-switch` = "withr",
Expand Down
1 change: 1 addition & 0 deletions R/data-shinyverse.R
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ shinyverse_remotes <- c(
"rstudio/shiny",
"rstudio/shinymeta",
"rstudio/shinytest",
"rstudio/chromote",
"rstudio/shinytest2",
"rstudio/shinythemes",
"rstudio/shinyvalidate",
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ These GitHub packages will be installed to make sure the latest package developm
- [rstudio/shiny](http://github.com/rstudio/shiny)
- [rstudio/shinymeta](http://github.com/rstudio/shinymeta)
- [rstudio/shinytest](http://github.com/rstudio/shinytest)
- [rstudio/chromote](http://github.com/rstudio/chromote)
- [rstudio/shinytest2](http://github.com/rstudio/shinytest2)
- [rstudio/shinythemes](http://github.com/rstudio/shinythemes)
- [rstudio/shinyvalidate](http://github.com/rstudio/shinyvalidate)
Expand Down
4 changes: 3 additions & 1 deletion inst/apps/306-accordion-add-remove/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ ui <- page_sidebar(
"accordion-icon-color" = "white",
"accordion-icon-active-color" = "white"
) %>%
bs_add_rules(".bslib-sidebar-layout .main { background-color: lightgray; }"),
bs_add_rules(
".bslib-sidebar-layout .main { background-color: lightgray; }"
),
sidebar = sidebar(
bg = "#1E1E1E",
accordion(
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Copy link
Member Author

Choose a reason for hiding this comment

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

@cpsievert I've fixed the sidebar position, but it looks like theres a bug here that could be 1) a selectize issue; 2) an accordion issue; or 3) a bug in the test app. I poked at it a bit but didn't find an easy answer. (Note I also tried with the latest selectize fixes in shiny PR 3923.)

Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ test_that("{shinytest2} recording: accordion-select", {
variant = platform_variant(), name = "accordion-select",
height = height, width = width,
view = interactive(),
options = list(bslib.precompiled = FALSE)
options = list(bslib.precompiled = FALSE),
screenshot_args = list(
delay = 0.5,
selector = "viewport",
options = list(captureBeyondViewport = FALSE)
)
)

# Make sure the set_input() calls complete in order
Expand Down
25 changes: 18 additions & 7 deletions inst/apps/308-sidebar-kitchen-sink/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@ plotly_bars <- plot_ly(x = LETTERS[1:3], y = 1:3) %>%
add_bars()

sidebar_long <- sidebar(lorem::ipsum(3, 3))
sidebar_short <- sidebar(
p("A simple sidebar"),
actionButton("foo", "This button does nothing")
)
sidebar_short <- local({
i <- 0
function() {
i <<- i + 1
sidebar(
p("A simple", code("sidebar")),
actionButton(sprintf("foo-%d", i), "This button does nothing"),
bg = "#1F77B4",
`data-bs-theme` = "dark"
)
}
})

ui <- page_navbar(
title = "Sidebar kitchen sink",
Expand All @@ -21,12 +29,15 @@ ui <- page_navbar(
position = "right",
id = "global_sidebar",
bg = "#1E1E1E",
`data-bs-theme` = "dark",
shiny::markdown(
"Learn more about `bslib::sidebar()` [here](https://rstudio.github.io/bslib/articles/sidebars.html)"
)
),
header = tagList(
tags$style(HTML(".plotly .modebar-container { display: none; }")),
tags$style(HTML("
.plotly .modebar-container { display: none; }
")),
span("header", class = "bg-dark"),
span("content", class = "bg-dark")
),
Expand All @@ -37,10 +48,10 @@ ui <- page_navbar(
nav_panel(
"Fill",
plotly_bars,
layout_sidebar(plotly_bars, sidebar = sidebar_short),
layout_sidebar(plotly_bars, sidebar = sidebar_short()),
card(
card_header("Depth"),
layout_sidebar(plotly_bars, sidebar = sidebar_short)
layout_sidebar(plotly_bars, sidebar = sidebar_short())
)
),
nav_panel(
Expand Down
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
library(shinytest2)

# Only take screenshots on mac + r-release to reduce diff noise
release <- rversions::r_release()$version
release <- jsonlite::fromJSON("https://api.r-hub.io/rversions/resolve/release")$version
release <- paste0(
strsplit(release, ".", fixed = TRUE)[[1]][1:2],
collapse = "."
Expand All @@ -10,7 +10,8 @@ release <- paste0(
is_testing_on_ci <- identical(Sys.getenv("CI"), "true") && testthat::is_testing()
is_mac_release <- identical(paste0("mac-", release), platform_variant())

DO_SCREENSHOT <- is_testing_on_ci && is_mac_release
DO_SCREENSHOT <- (is_testing_on_ci && is_mac_release) ||
identical(Sys.getenv("SHINYTEST2_SCREENSHOT"), "true")

test_that("{shinytest2} recording: 308-sidebar-kitchen-sink", {
height <- 1200
Expand All @@ -22,7 +23,13 @@ test_that("{shinytest2} recording: 308-sidebar-kitchen-sink", {
view = interactive(),
seed = 101,
height = height,
width = width
width = width,
# TODO: rstudio/shinytest2#367
screenshot_args = list(
selector = "viewport",
delay = 0.5,
options = list(captureBeyondViewport = FALSE)
)
)

expect_screenshot <- function() {
Expand All @@ -35,7 +42,7 @@ test_that("{shinytest2} recording: 308-sidebar-kitchen-sink", {
expect_screenshot()

# Contents should render to their natural height on mobile
app$set_window_size(width = 500, height = 1000)
app$set_window_size(width = 500, height = 2000)
expect_screenshot()
app$set_window_size(width = width, height = height)

Expand All @@ -48,6 +55,8 @@ test_that("{shinytest2} recording: 308-sidebar-kitchen-sink", {
expect_screenshot()

app$click("toggle_sidebar")
Sys.sleep(1) # Wait for transition to complete
app$wait_for_js("
document.querySelector('.bslib-sidebar-layout.transitioning') === null
")
expect_screenshot()
})
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ library(shinytest2)

# Only take screenshots on mac + r-release to reduce diff noise
expect_screenshot_mac_release <- local({
release <- rversions::r_release()$version
release <- jsonlite::fromJSON("https://api.r-hub.io/rversions/resolve/release")$version
release <- paste0(
strsplit(release, ".", fixed = TRUE)[[1]][1:2],
collapse = "."
Expand Down Expand Up @@ -69,7 +69,12 @@ test_that("310-bslib-sidebar-dynamic: dynamically added sidebars are fully funct
width = 1200,
view = interactive(),
options = list(bslib.precompiled = FALSE),
expect_values_screenshot_args = FALSE
expect_values_screenshot_args = FALSE,
screenshot_args = list(
selector = "viewport",
delay = 0.5,
options = list(captureBeyondViewport = FALSE)
)
)

expect_sidebar_hidden <- expect_sidebar_hidden_factory(app)
Expand Down
Loading