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

[WIP] progress bar in infoBox #135

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

[WIP] progress bar in infoBox #135

wants to merge 6 commits into from

Conversation

dmpe
Copy link
Contributor

@dmpe dmpe commented Mar 19, 2016

@dmpe
Copy link
Contributor Author

dmpe commented Mar 20, 2016

Problem

The problem I will need to solve with this PR is following:

In order to display the progress bar width, one needs to have e.g.

<div class="progress">
  <div class="progress-bar" style="width: 60%"></div>
</div>

On this, a CSS code below will apply:

.info-box .progress .progress-bar {
    background: #fff; //white
}

Which, however, needs to have height property (additionally) as this gets lost (somehow), because we make the progress-bar reactive and that adds a new shiny class to it.
<div id="progressBarValue" class="shiny-html-output shiny-bound-output">

Solutions

1.

We could solve it by doing this (with default CSS from AdminLTE):

<div>
      <div id="progressBarValue" class="shiny-html-output shiny-bound-output"> // IMPORTANT
            <div class="progress">
                   <div class="progress-bar" style="width: 60%"></div>
           </div>
      </div>
</div>

But this requires to have

// in server.R
server <- function(input, output) {
  output$progressBarValue <- renderUI({
    numberPercent <- paste0("width: ", input$progress, "%")
    div(class = "progress", div(class = "progress-bar", style = numberPercent))
  })
}

// then in UI.R
infoBox(.....progressBarValue = uiOutput("progressBarValue"))

// in boxes.R #L90
if (progressBar) div(class = "progress", progressBarValue),

which I dont like.

2.

Or we need to append height: 2px; to the above CSS code. So that we would have

<div class="progress">
        <div id="progressBarValue" class="shiny-html-output shiny-bound-output">
                <div class="progress-bar" style="width: 60%; height: 2px;"></div>
        </div>
</div>
// in server.R
server <- function(input, output) {
  output$progressBarValue <- renderUI({
    div(class = "progress-bar", style = paste0("width: ", input$progress, "%; height: 2px;"))
  })
}

// then in UI.R
infoBox(.....progressBarValue = uiOutput("progressBarValue"))

// in boxes.R #L90
if (progressBar) div(class = "progress", progressBarValue),

which is not much better in terms what we would have in boxes.R and in server.R but anyway better than nothing.

3.

It would be best, if there would be a function like findTagInHTML_And_AppendPropertyToIt(toFind, add) where I could specify div(class="progress-bar"), "style='width: 60%'" and it would work reactively. 😄

https://jsfiddle.net/uns7e8e8/1/

This uses solution number 2
# paste0("width: ", progressBarValue, "%; height: 2px;"))),

if (progressBar) div(class = "progress", progressBarValue),
if (!is.null(subtitle)) span(class = "progress-description", subtitle)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

span(class = "progress-description", subtitle) now uses CSS from AdminLTE which makes sure it behave correctly due to margins.

@dmpe dmpe mentioned this pull request Mar 20, 2016
@@ -83,7 +87,12 @@ infoBox <- function(title, value = NULL, subtitle = NULL,
div(class = "info-box-content",
span(class = "info-box-text", title),
if (!is.null(value)) span(class = "info-box-number", value),
if (!is.null(subtitle)) p(subtitle)
# if (progressBar) div(class = "progress", div(class = "progress-bar", style = progressBarValue)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Extraneous comments?

@dmpe
Copy link
Contributor Author

dmpe commented Mar 22, 2016

Hi @wch,

Thanks again. The reason for comments was that I was not sure about the solution and thus they were there for the ease of testing.
Does this mean that you agree that users will be required to use ... somewhere in there code.
Maybe you could take a look on issue #119 where there was an attempt to it.

  output$progressBarValue <- renderUI({
    div(class = "progress-bar", style = paste0("width: ", input$progress, "%; height: 2px;"))
  })

@@ -58,6 +58,7 @@ valueBox <- function(value, subtitle, icon = NULL, color = "aqua", width = 4,
#' content; the icon will use the same color with a slightly darkened
#' background.
#' @param href An optional URL to link to.
#' @param progressValue Must be between 0 and 100.
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have a more detailed description of what this does.

@dmpe
Copy link
Contributor Author

dmpe commented Mar 28, 2016

For this particular example, there's probably no need to make the progressValue dynamic, since no one will actually use it this way.

Well at least Jack would, #119 :)
So, just to make it static, right ?

@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