rstudio / connectwidgets

The {connectwidgets} package allows you to curate your content on RStudio Connect, helping to create organized groups of content within an RMarkdown document or Shiny app.
https://rstudio.github.io/connectwidgets/
Other
21 stars 7 forks source link

Grid view not working #72

Closed oude-gao closed 1 year ago

oude-gao commented 2 years ago

Hi I recently redployed a RMD page with the grid view, and it looks like each content is very large and takes up almost one page, which means the layout where each content is a square-like item does not exist.

Sessioninfo:

R version 4.1.0 (2021-05-18) Platform: x86_64-pc-linux-gnu (64-bit) Running under: Ubuntu 20.04.4 LTS

Matrix products: default BLAS: /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.9.0 LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.9.0

locale: [1] LC_CTYPE=C.UTF-8 LC_NUMERIC=C LC_TIME=C.UTF-8 LC_COLLATE=C.UTF-8
[5] LC_MONETARY=C.UTF-8 LC_MESSAGES=C.UTF-8 LC_PAPER=C.UTF-8 LC_NAME=C
[9] LC_ADDRESS=C LC_TELEPHONE=C LC_MEASUREMENT=C.UTF-8 LC_IDENTIFICATION=C

attached base packages: [1] stats graphics grDevices utils datasets methods base

other attached packages: [1] dplyr_1.0.9 connectwidgets_0.1.1

loaded via a namespace (and not attached): [1] knitr_1.39 magrittr_2.0.3
[3] tidyselect_1.1.2 R6_2.5.1
[5] rlang_1.0.2 fastmap_1.1.0
[7] fansi_1.0.3 tools_4.1.0
[9] xfun_0.31.2.0.0.0.1653056887 utf8_1.2.2
[11] cli_3.3.0 DBI_1.1.2
[13] htmltools_0.5.2.9000.0.0.0.1640018799 ellipsis_0.3.2
[15] yaml_2.3.5 assertthat_0.2.1
[17] digest_0.6.29 tibble_3.1.7
[19] lifecycle_1.0.1 crayon_1.5.1
[21] purrr_0.3.4 vctrs_0.4.1
[23] rsconnect_0.8.25 glue_1.6.2
[25] evaluate_0.15 rmarkdown_2.14.1.0.0.0.1650924090
[27] compiler_4.1.0 pillar_1.7.0
[29] generics_0.1.2 pkgconfig_2.0.3

veronicahorgan commented 2 years ago

@oude-gao have you found a solution to this problem by any chance? I am experiencing the same!

edavidaja commented 2 years ago

Could you post a reprex?

veronicahorgan commented 2 years ago

Hi! Sorry I don’t know how to do a reprex when the content is coming from rstudio connect and the URLs are required in the data frame? Please let me know if this is possible! Thanks

On 12 Aug 2022, at 13:08, E. David Aja @.***> wrote:



Could you post a reprexhttps://reprex.tidyverse.org/?

— Reply to this email directly, view it on GitHubhttps://github.com/rstudio/connectwidgets/issues/72#issuecomment-1213044042, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AO3VVN3G7MTI3IGOZ2UVZADVYY5EXANCNFSM5YGSKW6A. You are receiving this because you commented.Message ID: @.***>

edavidaja commented 2 years ago

In this case, you can create a test page using the public rstudio connect test server at https://beta.rstudioconnect.com:

---
title: connectwidgets test
---

```{r}

library(connectwidgets)
client <- connect(
  server = "https://beta.rstudioconnect.com",
  api_key = Sys.getenv("CONNECT_API_KEY")
  )

all_content <- content(client)
all_content |> by_tag("experimental") |> rsc_grid()

sessioninfo::session_info()
FMKerckhof commented 2 years ago

I was facing the same issue and made the reprex available as suggested above. It can be found at: https://beta.rstudioconnect.com/connect/#/apps/58759967-ba8a-4719-972d-d9a088aaff6d/access

As can be seen from the screenshots below, rather than displaying a clean "grid" the content tiles are taking up the entire content width:

image

I would say this is a clear bug, the solution suggested in #74 did not make a difference for me

Should it matter, we are using Rstudio connect v.2022.05.0 build v2022.05.0-0-g762d29c the github version of connectwidgets and R 4.2.1 on windows or linux. I have tested this with Firefox 103.0.2 as well as Chrome 104.0.5112.81 (Official Build)

FMKerckhof commented 2 years ago

Could this be related to the breaking changes introduced in sass release https://github.com/rstudio/sass/releases/tag/v0.4.0 ? https://github.com/rstudio/connectwidgets/blob/c3e51a9fac7ef355237eba2daabbe3cecaec4dda/R/theme.R#L103 specifically, the part related to the htmlDependency's could be relevant?

FMKerckhof commented 2 years ago

@edavidaja : let me know if you require any more testing from my side or if I can help to resolve the issue. We have gotten our resident CSS wizard to write a custom css that will reformat the rsc_cards in an rsc_col container to mimick the behavior of rsc_grid but it is a hacky solution at best so I really hope we can get the rsc_grid solution working again.

edavidaja commented 2 years ago

I was able to reproduce by updating my version of bslib to 0.4.0; the temporary fix here is to pin bslib to 0.3.1.

FMKerckhof commented 1 year ago

@edavidaja I checked the recently released bslib 0.4.2 which has a new card API (https://rstudio.github.io/bslib/news/index.html#bslib-042) . The issue still persists but I do get more informative error messages - since the connect beta doesn't allow for (re)publishing anymore, I have published it to shinyapps.io : https://fpkerckh.shinyapps.io/Reprex_broken_grid/

FMKerckhof commented 1 year ago

Furthermore, I tried the new "responsive column first layout" (https://rstudio.github.io/bslib/articles/layouts.html#responsive-column-first-layout) to no avail. Again, warnings are thrown: bindFillRole() only works on htmltools::tag() objects (e.g., div(), p(), etc.), not objects of type 'shiny.tag.list'.

boshek commented 1 year ago

Can confirm that bslib 0.3.1 (pinned with renv) is still a working solution and that upgrading to sass 0.4.5 (update on CRAN today) did not change this behaviour.

tommarshall2 commented 1 year ago

Hacky fix only, but I was able to get the grid to display reasonably by adding the following css to my document:

.rscgrid-item {
  width: 22.75%;
}

.rscgrid__grid {
  justify-content: space-between;
}
FMKerckhof commented 1 year ago

@edavidaja any update when a potential fix would be rolled out to connectwidgets?

al-obrien commented 1 year ago

Considering the usefulness of this project and how this function is a core component, hoping for a permanent fix as well soon. That said, the temporary fix of bslib works but isn't ideal of course...

jeremy-allen commented 1 year ago

@edavidaja I ran into the same thing today, then with renv::install("bslib@0.3.1") grids were back

msummersgill commented 1 year ago

I'm also running into this issue with the development branch of bslib - 0.5.0.9000.

RStudio Connect Version 2022.12.0

sessionInfo()
## R version 4.2.2 (2022-10-31)
## Platform: x86_64-pc-linux-gnu (64-bit)
## Running under: Ubuntu 20.04.5 LTS
## 
## Matrix products: default
## BLAS:   /usr/lib/x86_64-linux-gnu/atlas/libblas.so.3.10.3
## LAPACK: /usr/lib/x86_64-linux-gnu/atlas/liblapack.so.3.10.3
## 
## locale:
##  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
##  [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
##  [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
##  [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
##  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
## [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
## 
## attached base packages:
## [1] stats     graphics  grDevices utils     datasets  methods   base     
## 
## other attached packages:
## [1] connectwidgets_0.2.0 data.table_1.14.6   
## 
## loaded via a namespace (and not attached):
##  [1] knitr_1.43       magrittr_2.0.3   tidyselect_1.2.0 R6_2.5.1        
##  [5] rlang_1.1.1      fastmap_1.1.1    fansi_1.0.3      httr_1.4.4      
##  [9] dplyr_1.1.2      tools_4.2.2      xfun_0.39        utf8_1.2.2      
## [13] cli_3.6.1        jquerylib_0.1.4  htmltools_0.5.5  yaml_2.3.6      
## [17] digest_0.6.33    tibble_3.2.1     lifecycle_1.0.3  sass_0.4.6      
## [21] vctrs_0.6.2      curl_5.0.0       glue_1.6.2       cachem_1.0.8    
## [25] evaluate_0.18    rmarkdown_2.22   compiler_4.2.2   bslib_0.5.0.9000
## [29] pillar_1.9.0     generics_0.1.3   jsonlite_1.8.7   pkgconfig_2.0.3
gadenbuie commented 1 year ago

@edavidaja @jeremy-allen Is it possible reproduce this issue without deploying content to Connect, i.e. to use rsc_grid() in a way that can be previewed locally?

edavidaja commented 1 year ago

I would expect the problem to reproduce when the document is rendered locally?

gadenbuie commented 1 year ago

I would expect the problem to reproduce when the document is rendered locally?

That makes sense. Could someone with more experience with connectwidgets than me rewrite the reprex to avoid the Connect dependency?

msummersgill commented 1 year ago

I would expect the problem to reproduce when the document is rendered locally?

That makes sense. Could someone with more experience with connectwidgets than me rewrite the reprex to avoid the Connect dependency?

Spoofing a data frame makes it locally reproducible.

---
title: connectwidgets test
---

```{r warning = FALSE}
x <- letters[1:6]
data.frame(id = seq_along(x),
           guid = "", 
           name = x,
           title = paste0("Title for ",x), 
           description = paste0("Description for ",x),
           app_mode = "rmd-static",
           content_category = "", 
           url = "", 
           owner_guid = "",
           owner_username = "", 
           owner_first_name = "",
           owner_last_name = "",
           created_time = structure(1681955573,
                                    class = c("POSIXct", 
                                              "POSIXt"), tzone = "UTC"),
           updated_time = structure(1686101559,
                                    class = c("POSIXct", 
                                              "POSIXt"), tzone = "UTC"),
           tags = rep(list(structure(list(id = "10",
                                          name = "x",
                                          parent_id = "1",
                                          created_time = "2019-05-30T15:45:36Z",
                                          updated_time = "2019-05-30T15:45:36Z"), class = "data.frame", row.names = 1L)),6))|>  
  connectwidgets::rsc_grid()


![image](https://github.com/rstudio/connectwidgets/assets/19575728/bf91dcc3-c0f1-4dee-9ec9-8289c1b96c45)
gadenbuie commented 1 year ago

At its core, the issue lies in the usage of the @media-breakpoint-down mixin here (and in other similar places):

https://github.com/rstudio/connectwidgets/blob/a78fa2c7ddd27a3054f207f1da6f1055112d529a/inst/theming/rsc_grid.scss#L65-L75

My first recommendation is to avoid using this mixin. I'd recommend considering using display: grid with grid-template-columns set to repeat(autofit, minmax({min}, {max})) with appropriate min/max values.

The reason for avoiding the @media-breakpoint-down mixin is that it's not defined in BS3 and its behavior changed between BS4 and BS5. The fact that it's not defined in BS3 but seen in documents where BS3 is used (such as a plain R Markdown document) is a clue that something's awry.

The problem we're seeing here in this issue is that in bslib 0.4.0 the default Bootstrap version switched to 5. In BS5, @media-breakpoint-down(xs) just returns the styles without a @media query, so the rules for the smallest breakpoint are being applied even at the largest viewport sizes.

This points to the fact that connectwidgets calls bslib::bs_theme() directly as the default theme, which by default uses the latest version of Bootstrap, unless the user requests a specific version, e.g. version = 4. https://github.com/rstudio/connectwidgets/blob/a78fa2c7ddd27a3054f207f1da6f1055112d529a/R/theme.R#L20-L23 Looking at this section of how the grid widget dependencies are loaded, I think the bslib theme resolution and dependency creation could be refactored and simplified to use the latest best practices from bslib, in particular bs_dependency_defer() for dynamic theming as described in the theming article.

In summary, I'd recommend the following steps:

  1. Rewrite the grid component CSS to avoid calling Bootstrap mixins. (You could also consider vendoring mixins.)
  2. Revisit the component Sass to ensure there isn't an implicit dependency on Bootstrap 4 variables. Along these lines, I'd recommend leaning more heavily on internal Sass variables set with !default.
  3. Rewrite the connectwidgets dependency-providing functions to use bs_dependency_defer() for dynamic theming.

I'm happy to help with the second and third step, either to contribute a PR or to pair through one. It'd be most effective, however, if someone with more experience with connectwidget's CSS could tackle the first step of dealing with the @media-breakpoint-down() mixin usage.