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 renderText bug for h3 elements #250

Merged
merged 3 commits into from Jan 8, 2020
Merged

Fix renderText bug for h3 elements #250

merged 3 commits into from Jan 8, 2020

Conversation

@schloerke
Copy link
Collaborator

@schloerke schloerke commented Dec 18, 2019

Fixes #245

There was a timing issue with how shiny initialized. Because titles were shuffled around, shiny said it looked like a shiny-bound-output class object, but it didn't have the proper information.

$(document).ready() is not needed as the JS is sourced at end of body and the Flexdashboard code is sourced at the beginning of the body. So the setTimeout can be dropped from acf013d


Have tried on Shiny 1.4+ and Shiny 1.3.2. (The issue was created when jQuery was bumped to v3. See #245 (comment))

@schloerke schloerke requested review from jjallaire and wch Dec 18, 2019
R/flex_dashboard.R Outdated Show resolved Hide resolved
…etach()

Sidebar, h1 titles, h2 -> h3, h3 -> li, chart title, headings, blockquotes, valueBox, captions.

Use `append(x)` instead of  `html(x)`. `html(x)` does not respect existing element information.
Use `contents()` over `html()`. `contents()` will return full elements with data objects intact, `html()` returns a string.
@schloerke schloerke force-pushed the bugfix/renderTextFailure branch from 378cd2c to befee8c Dec 23, 2019
@schloerke
Copy link
Collaborator Author

@schloerke schloerke commented Dec 23, 2019

Changed approach to use and parent.append(el.contents()) vs parent.html(el.html).

jquery links:

By not serializing to strings, the shiny data objects can be kept intact. The order of execution had changed once shiny moved up to jquery v3 in shiny v1.4.0.

These changes are only necessary to maintain the existing shiny data objects hidden in the DOM elements. Any occurrence of .html(x.html()) (if appropriate), was replaced with some form of .append(el.contents()).

Changes:

  • page headings (bug fix)
  • h2 -> h3 titles (bug fix)
  • tab titles (bug fix)
  • captions (bug fix)
  • global sidebar
  • h3 -> li
  • chart titles
  • valueBox
Jster testing document for github.com/rstudio/shinycoreci-apps testing...
---
title: "Shiny Location Kitchen Sink"
output:
  flexdashboard::flex_dashboard:
    orientation: rows
runtime: shiny
---

```{r}
library(flexdashboard)
library(shiny)
library(dygraphs)
```

# Inputs: `r renderText({getmonth()})` {.sidebar}


```{r}
selectInput(
  "month",
  label = "Pick a Month",
  choices = month.abb,
  selected = month.abb[2]
)
getmonth <- reactive({
  input$month
})
```

<div id="sidebarContent">

Sidebar Content: `r renderText(getmonth())`

</div>

```{r}
shinyjster::shinyjster_server(input, output, session)
shinyjster::shinyjster_js("
  var jst = jster();
  jst.add(Jster.shiny.waitUntilStable);
  jst.add(function() {
    // make sure on first page
    $('a[href=\"#section-p1\"]').click();
  });
  jst.add(Jster.shiny.waitUntilStable);
  jst.add(Jster.shiny.waitUntilStable);
  jst.add(Jster.shiny.waitUntilStable);
  jst.add(Jster.shiny.waitUntilStable);
  jst.add(Jster.shiny.waitUntilStable);
  jst.add(Jster.shiny.waitUntilStable);

  function assert_equal(id, txt) {
    assert_equal_qs('#section-' + id, txt);
  }
  function assert_equal_qs(qs, txt) {
    jst.add(function() {
      var el = $(qs);
      Jster.assert.isEqual(
        el.text().trim(),
        txt,
        {query_string: qs, element: el}
      );
    })
  }

  assert_equal_qs('a[href=\"#section-p1\"]', 'Page 1: Feb');
  assert_equal_qs('a[href=\"#section-p2\"]', 'Page 2: Feb');
  assert_equal('sidebarContent', 'Sidebar Content: Feb');

  assert_equal_qs('#section-hidden1 .value-output', 'VB1: Feb');
  assert_equal_qs('#section-hidden2 .value-output', 'VB2: Feb');
  assert_equal_qs('#section-hidden3 .value-output', 'VB3: Feb');

  assert_equal_qs('#section-row2 .chart-title', 'Row 2 Title: Feb');
  assert_equal_qs('#section-row2 p', 'Row 2 Content: Feb');
  assert_equal_qs('#section-row2 .chart-notes', 'Caption: Feb');

  assert_equal_qs('#section-row3 .chart-title', 'Row 3 Title: Feb');
  assert_equal_qs('#section-row3 p', 'Row 3 Content: Feb');

  assert_equal_qs('a[href=\"#section-tab1\"]', 'Tab 1 Title: Feb');
  assert_equal_qs('#section-tab1 p', 'Tab 1 Content: Feb');

  jst.add(function() {
    $('a[href=\"#section-tab2\"]').click();
  });
  jst.add(Jster.shiny.waitUntilStable);

  assert_equal_qs('a[href=\"#section-tab2\"]', 'Tab 2 Title: Feb');
  assert_equal_qs('#section-tab2 p', 'Tab 2 Content: Feb');

  // test second page
  jst.add(function() {
    $('a[href=\"#section-p2\"]').click();
  });
  jst.add(Jster.shiny.waitUntilStable);

  assert_equal_qs('#section-p2r1 .chart-title', 'P2R1 Title: Feb');
  assert_equal_qs('#section-p2r1 p', 'P2R1 Content: Feb');

  assert_equal_qs('#section-p2r2 .chart-title', 'P2R2 Title: Feb');
  assert_equal_qs('#section-p2r2 p', 'P2R2 Content: Feb');

  assert_equal_qs('a[href=\"#section-p2t1\"]', 'P2T1 Title: Feb');
  assert_equal_qs('#section-p2t1 p', 'P2T1 Content: Feb');

  jst.add(function() {
    $('a[href=\"#section-p2t2\"]').click();
  });
  jst.add(Jster.shiny.waitUntilStable);
  assert_equal_qs('a[href=\"#section-p2t2\"]', 'P2T2 Title: Feb');
  assert_equal_qs('#section-p2t2 p', 'P2T2 Content: Feb');



  jst.test();
  console.log('done!')
")
```


# Page 1: `r renderText({getmonth()})` {id=p1}


Row
-------------------------------------

### Hidden title 1 {id=hidden1}

```{r}
renderValueBox({
  valueBox({paste0("VB1: ", getmonth())}, "Value Box 1")
})
```

### Hidden title 2 {id=hidden2}

```{r}
renderValueBox({
  valueBox({paste0("VB2: ", getmonth())}, "Value Box 2")
})
```

### Hidden title 3 {id=hidden3}

```{r}
renderValueBox({
  valueBox({paste0("VB3: ", getmonth())}, "Value Box 3")
})
```

Row
-------------------------------------

### Row 2 Title:  `r renderText(getmonth())` {id=row2}

Row 2 Content: `r renderText(getmonth())`

```{r}
dygraph(ldeaths)
```

> Caption: `r renderText(getmonth())`


Row
-------------------------------------

### Row 3 Title:  `r renderText(getmonth())` {id=row3}

Row 3 Content: `r renderText(getmonth())`

```{r, fig.width=10, fig.height=7}
plot(cars)
```


Row {.tabset}
-------------------------------------

### Tab 1 Title:  `r renderText(getmonth())` {id=tab1}

Tab 1 Content: `r renderText(getmonth())`

```{r}
knitr::kable(mtcars)
```

### Tab 2 Title:  `r renderText(getmonth())` {id=tab2}

Tab 2 Content: `r renderText(getmonth())`

```{r}
renderTable({
  head(mtcars, n = 12)
})
```


# Page 2: `r renderText({getmonth()})` {id=p2}


Row
-------------------------------------

### P2R1 Title:  `r renderText(getmonth())` {id=p2r1}

P2R1 Content: `r renderText(getmonth())`

Row
-------------------------------------

### P2R2 Title:  `r renderText(getmonth())` {id=p2r2}

P2R2 Content: `r renderText(getmonth())`


Row {.tabset}
-------------------------------------

### P2T1 Title:  `r renderText(getmonth())` {id=p2t1}

P2T1 Content: `r renderText(getmonth())`

### P2T2 Title:  `r renderText(getmonth())` {id=p2t2}

P2T2 Content: `r renderText(getmonth())`
@wch
wch approved these changes Dec 30, 2019
* fix comment
* remove, do not detach old elements
* chartValue is text and should be added as text
@schloerke schloerke changed the title Fix renderText bug for h3 elements [WIP] Fix renderText bug for h3 elements Jan 2, 2020
@schloerke schloerke changed the title [WIP] Fix renderText bug for h3 elements Fix renderText bug for h3 elements Jan 8, 2020
@schloerke schloerke requested review from jjallaire and removed request for jjallaire Jan 8, 2020
@schloerke schloerke merged commit 404cdac into master Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.