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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add wrench icon #121

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

[WIP] Add wrench icon #121

wants to merge 14 commits into from

Conversation

dmpe
Copy link
Contributor

@dmpe dmpe commented Jan 31, 2016

According to the requiest here: #92 The user would like to see a 馃敡 icon. I think - for me - this is pretty possible to accomplish.

Add icons (like the wrench) to the boxes header. Very useful for adding specific commands (like print, save) or help links.

However, i have yet to come up with the way of how to display the dropdown menu.
EDIT: maybe something like re-using dropdownMenu and extend it somewhat? See also old comments.

馃啓 This also deletes Rcpp which is no longer necessary.

add really big dashboard covering about 70-80% of all elements used or described.
update news file as I believe version update to AdminLTE 2.3.2 is fine and not going to break anything.
delete `interactive()` because tests are in Rignore
add comments
fix tabItems issue
comment repro issues
fix version, bump both package further ahead and make sure that rstudio#42 is not repro on my pc
delete Rcpp from travis
add 2 new repro cases for new issues
@dmpe dmpe changed the title [Do not merge yet] Add 馃敡 icon [Do not merge yet] Add wrench icon Jan 31, 2016
@@ -250,7 +251,7 @@ infoBox <- function(title, value = NULL, subtitle = NULL,
#' @export
box <- function(..., title = NULL, footer = NULL, status = NULL,
solidHeader = FALSE, background = NULL, width = 6,
height = NULL, collapsible = FALSE, collapsed = FALSE) {
height = NULL, collapsible = FALSE, collapsed = FALSE, wrench = FALSE) {

boxClass <- "box"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After deep thinking:
Not so much sure if wrench= should be boolean. Rather something like tags$li =....
EDIT:see below

@dmpe dmpe changed the title [Do not merge yet] Add wrench icon [WIP but check it out] Add wrench icon Feb 5, 2016
@dmpe dmpe changed the title [WIP but check it out] Add wrench icon Add wrench icon [Ready to merge] Feb 5, 2016
@dmpe dmpe changed the title Add wrench icon [Ready to merge] [MRG] Add wrench icon Feb 9, 2016
@dmpe dmpe changed the title [MRG] Add wrench icon [MeRGe] Add wrench icon Feb 21, 2016
@dmpe
Copy link
Contributor Author

dmpe commented Mar 3, 2016

Hi @wch ,
Please, could you look on this change? I know you are very busy but I dont know if I should open more PR (e.g. I have just extended a behaviour so that sidebar shows icons only when it is collapsed #133) or wait for this one being merged.

I know that this project is not 100% priority nowadays - but I want to get a new version (with some of my changes) to CRAN so that many users could benefit from it.

@dmpe dmpe force-pushed the master-2 branch 3 times, most recently from 137576c to 13ff71f Compare March 3, 2016 09:52
@dmpe
Copy link
Contributor Author

dmpe commented Mar 9, 2016

So I guess it will be easier for you just to follow this:http://stackoverflow.com/a/5309051

Basically, you would (not tested)

git remote add originDMPE https://github.com/dmpe/shinydashboard.git
git fetch originDMPE 
git checkout master 
git merge --squash master-2

which would merge all my commits (from here) into one commit

OR basically https://github.com/blog/2141-squash-your-commits

@wch
Copy link
Contributor

wch commented Mar 22, 2016

It would be more consistent to implement this with something similar to the existing menus (message, notification, task). Instead of allowing users to pass in any content, it should be something like:

boxMenu(icon = shiny::icon("wrench"),
  item(text = "text")
)

This is only a rough idea though -- since the HTML is quite a bit different from the other dropdownMenus, it probably deserves a separate function. Ideally it should also be done in a way that's generalizable to other types of menus, though. For example, I see that one of the boxes in the AdminLTE demo has a Contacts menu, and it would be good if this were implemented in a way that could work for both.

Then the box function could take this boxMenu (or whatever it's called) as an option, maybe named menu.

Also, the menu should be able to be made dynamic, similar to various menus documented in the shinydashboard website.

An approach like this will be good for maintainability in the future.

@dmpe
Copy link
Contributor Author

dmpe commented Mar 23, 2016

 # Dropdown menu for tasks, with progress bar
  dropdownMenu(type = "user", badgeStatus = NULL,
               headerUserPanel(name = "John Malc",
                               desc = "Shiny Developer",
                              image = "https://almsaeedstudio.com/themes/AdminLTE/dist/img/user2-160x160.jpg")
  )
dropdownMenu <- function(...,
  type = c("messages", "notifications", "tasks", "user"),
  badgeStatus = "primary", icon = NULL, .list = NULL)
{
  type <- match.arg(type)
  if (!is.null(badgeStatus)) validateStatus(badgeStatus)
  items <- c(list(...), .list)

  # Make sure the items are li tags
  lapply(items, tagAssert, type = "li")

  dropdownClass <- paste0("dropdown ", type, "-menu")

  if (is.null(icon)) {
    icon <- switch(type,
      messages = shiny::icon("envelope"),
      notifications = shiny::icon("warning"),
      tasks = shiny::icon("tasks"),
      user = shiny::icon("user")
    )
  }

  numItems <- length(items)
  if (is.null(badgeStatus)) {
    badge <- NULL
  } else {
    badge <- span(class = paste0("label label-", badgeStatus), numItems)
  }

  tags$li(class = dropdownClass,
    a(href = "#", class = "dropdown-toggle", `data-toggle` = "dropdown",
      icon,
      badge
    ),
    tags$ul(class = "dropdown-menu",
      if (type != "user") tags$li(class = "header", paste("You have", numItems, type)),
      if (type != "user") {
        tags$li(
          tags$ul(class = "menu",
            items
          )
        )
      } else {
        tags$li(items)
      }
      # TODO: This would need to be added to the outer ul
      # tags$li(class = "footer", a(href="#", "View all"))
    )
  )

}

#' A panel displaying user information in a header
#'
#' @param name Name of the user.
#' @param desc Text or HTML to be shown next the name.
#' @param image A filename or URL to use for an image of the person. If it is a
#'   local file, the image should be contained under the www/ subdirectory of
#'   the application.
#'
#' @family sidebar items
#'
#' @seealso \code{\link{dashboardSidebar}} for example usage.
#'
#' @export
headerUserPanel <- function(name, desc = NULL, image = NULL) {
  if (!is.null(image)) {
    tags$li(
      class = "user-header",
      img(src = image, class = "img-circle", alt = "User Image"),
      p(name),
      tags$small(desc)
    )
  }
}

@dmpe dmpe changed the title [MeRGe] Add wrench icon [WIP] Add wrench icon Mar 23, 2016
@wch
Copy link
Contributor

wch commented Mar 28, 2016

The interface for boxItem is very different from other menus in shinydashboard, and it should be made more consistent.

For example, there's a dropdownMenu to create the menu item, and then notificationItem to create and individual item.

headerTag <- NULL
if (!is.null(titleTag) || !is.null(collapseTag)) {
if (!is.null(titleTag) || !is.null(boxTools)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't boxTools always be non-NULL? I think the boxTools div should be created only if it's needed.

@dmpe
Copy link
Contributor Author

dmpe commented Mar 28, 2016

The interface for boxItem is very different from other menus in shinydashboard, and it should be made more consistent.

Yes, it is different. The reason is also that it can contain anything, from a download button to print-this-image links etc. Giving users only text and href parameters seems too little for me. boxItem/boxMenu will not be re-usable on other components.

@dmpe
Copy link
Contributor Author

dmpe commented Mar 29, 2016

Something like this ?

#' @inheritParams box
#' @export
boxItem <- function(...) {
  listOfValues <- list(...)
  # include each arg into <li> </li> tags
  listOfLi <- lapply(listOfValues, tags$li)
}

#' @param icon Default icon (if boxMenu is used) is wrench
#' @export
dropDownMenuBox <- function(boxItem = NULL, icon = shiny::icon("wrench")){
  tags$div(class = "btn-group",
           tags$button(class = "btn btn-box-tool dropdown-toggle",
                       `type` = "button",
                       `data-toggle` = "dropdown",
                       icon),
           tags$ul(class = "dropdown-menu",
                   `role` = "menu",
                   boxItem)
  )
}

I find such a split quite unnecessary, don't you too ?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

None yet

3 participants