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

Responsive f7Grid #270

Merged
merged 4 commits into from
Dec 4, 2024
Merged

Responsive f7Grid #270

merged 4 commits into from
Dec 4, 2024

Conversation

ThomasChln
Copy link
Contributor

Enable responsive behavior in f7Grid as described here https://framework7.io/docs/grid

@DivadNojnarg
Copy link
Member

DivadNojnarg commented Nov 22, 2024

Thanks @ThomasChln. Could we rename the parameter using camelCase? To match with the rest of the API. Besides, I see there are other breakpoints such as for small, large, xl. Do you want to extend the parameter usage to cover these cases: for instance, I'd see it like:

#' Framework7 grid container
#'
#' Grid container for elements
#'
#' @param ... Row content.
#' @param cols Columns number. Numeric between 1 and 20.
#' @param gap Whether to display gap between columns. TRUE by default.
#' @param responsiveCl Class for responsive behavior. 
#' The format must be `<SIZE-NCOLS>` such as `<medium-4>`, `<large-3>`.
#' SIZE must be one of \code{c("xsmall", "small", "medium", "large", "xlarge")}.
#' NCOLS has to be between 1 and 20.
#'
#' @author David Granjon, \email{dgranjon@@ymail.com}
#' @export
f7Grid <- function(..., cols, gap = TRUE, responsiveCl = NULL) {
  cl <- sprintf("grid grid-cols-%s", cols)

  if (!is.null(responsiveCl)) {
  #Format must be: <SIZE-NCOLS> like, <medium-4>, <large-3>
   tmp <- strsplit(responsiveCl, "-")[[1]]
   breakpoint <- tmp[1]
   ncols <- as.numeric(tmp[2])
   stopifnot(
     ncols >= 1 && ncols <= 20, 
     breakpoint %in% c("xsmall", "small", "medium", "large", "xlarge")
    )                                          
    cl <- paste(cl, sprintf("%s-grid-cols-%s", breakpoint, ncols))                
  }                                                                             

  if (gap) cl <- paste(cl, "grid-gap")
  shiny::tags$div(class = cl, ...)
}

What do you think?

@ThomasChln
Copy link
Contributor Author

Sure, looks good

@ThomasChln
Copy link
Contributor Author

Just pushed your new version. I guess it could be improved to be able to specify multiple breakpoints, e.g. on medium screens do 4 columns and on large 6. But I think it's good enough for now.

Copy link
Member

@DivadNojnarg DivadNojnarg left a comment

Choose a reason for hiding this comment

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

LGTM thanks. @ThomasChln You can also change the NEWS.md to add your contribution to the list of changes. The next version would be 2.0.2.9000. Otherwise I can do it later anyway. Let me know.

@DivadNojnarg DivadNojnarg merged commit 578dcb0 into RinteRface:master Dec 4, 2024
5 checks passed
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.

2 participants