Skip to content

Commit

Permalink
Fix #2349, #2329, #1817: bugs triggered by networkD3 sankey plot
Browse files Browse the repository at this point in the history
* All of these were caused by the presence of multiple body tags on the
page, which happened because networkD3's sankey plot generates SVGs
containing body tags via SVG's foreignObject tag
* In various places, the 'body' jQuery selector string is used under the
assumption there is only one 'body' tag on the page. The presence of
multiple 'body' tags breaks reliant code in strange ways.
* The fix was to use document.body or 'body:first' instead of 'body'.
  • Loading branch information
alandipert committed Mar 22, 2019
1 parent 722b1d0 commit 3d11780
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 13 deletions.
3 changes: 2 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ shiny 1.2.0.9001

* Fixed [#2308](https://github.com/rstudio/shiny/issues/2308): When restoring a bookmarked application, inputs with a leading `.` would not be restored. ([#2311](https://github.com/rstudio/shiny/pull/2311))

* Fixed [#2349](https://github.com/rstudio/shiny/issues/2349), [#2329](https://github.com/rstudio/shiny/issues/2329), [#1817](https://github.com/rstudio/shiny/issues/1817): These were various bugs triggered by the presence of the [networkD3](https://christophergandrud.github.io/networkD3/) package's Sankey plot in an app. Impacted features included `dateRangeInput`, `withProgressBar`, and bookmarking ([#2359](https://github.com/rstudio/shiny/pull/2359))

### Documentation Updates

* Fixed [#2247](https://github.com/rstudio/shiny/issues/2247): `renderCachedPlot` now supports using promises for either `expr` or `cacheKeyExpr`. (Shiny v1.2.0 supported async `expr`, but only if `cacheKeyExpr` was async as well; now you can use any combination of sync/async for `expr` and `cacheKeyExpr`.) [#2261](https://github.com/rstudio/shiny/pull/2261)


shiny 1.2.0
===========

Expand Down
6 changes: 3 additions & 3 deletions inst/www/shared/datepicker/js/bootstrap-datepicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@
visualPadding = 10,
container = $(this.o.container),
windowWidth = container.width(),
scrollTop = this.o.container === 'body' ? $(document).scrollTop() : container.scrollTop(),
scrollTop = this.o.container === 'body:first' ? $(document).scrollTop() : container.scrollTop(),
appendOffset = container.offset();

var parentsZindex = [];
Expand All @@ -686,7 +686,7 @@
var left = offset.left - appendOffset.left,
top = offset.top - appendOffset.top;

if (this.o.container !== 'body') {
if (this.o.container !== 'body:first') {
top += scrollTop;
}

Expand Down Expand Up @@ -1766,7 +1766,7 @@
enableOnReadonly: true,
showOnFocus: true,
zIndexOffset: 10,
container: 'body',
container: 'body:first',
immediateUpdates: false,
title: '',
templates: {
Expand Down
2 changes: 1 addition & 1 deletion inst/www/shared/datepicker/js/bootstrap-datepicker.min.js

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions srcjs/init_shiny.js
Original file line number Diff line number Diff line change
Expand Up @@ -413,17 +413,17 @@ function initShiny() {
// Need to register callbacks for each Bootstrap 3 class.
var bs3classes = ['modal', 'dropdown', 'tab', 'tooltip', 'popover', 'collapse'];
$.each(bs3classes, function(idx, classname) {
$('body').on('shown.bs.' + classname + '.sendImageSize', '*',
$(document.body).on('shown.bs.' + classname + '.sendImageSize', '*',
filterEventsByNamespace('bs', sendImageSize));
$('body').on('shown.bs.' + classname + '.sendOutputHiddenState ' +
$(document.body).on('shown.bs.' + classname + '.sendOutputHiddenState ' +
'hidden.bs.' + classname + '.sendOutputHiddenState',
'*', filterEventsByNamespace('bs', sendOutputHiddenState));
});

// This is needed for Bootstrap 2 compatibility and for non-Bootstrap
// related shown/hidden events (like conditionalPanel)
$('body').on('shown.sendImageSize', '*', sendImageSize);
$('body').on('shown.sendOutputHiddenState hidden.sendOutputHiddenState', '*',
$(document.body).on('shown.sendImageSize', '*', sendImageSize);
$(document.body).on('shown.sendOutputHiddenState hidden.sendOutputHiddenState', '*',
sendOutputHiddenState);

// Send initial pixel ratio, and update it if it changes
Expand Down
2 changes: 1 addition & 1 deletion srcjs/input_binding_fileinput.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var IE8FileUploader = function(shinyapp, id, fileEl) {
this.iframe.id = iframeId;
this.iframe.name = iframeId;
this.iframe.setAttribute('style', 'position: fixed; top: 0; left: 0; width: 0; height: 0; border: none');
$('body').append(this.iframe);
$(document.body).append(this.iframe);
var iframeDestroy = function() {
// Forces Shiny to flushReact, flush outputs, etc. Without this we get
// invalidated reactives, but observers don't actually execute.
Expand Down
2 changes: 1 addition & 1 deletion srcjs/modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ exports.modal = {
let $modal = $('#shiny-modal-wrapper');
if ($modal.length === 0) {
$modal = $('<div id="shiny-modal-wrapper"></div>');
$('body').append($modal);
$(document.body).append($modal);

// If the wrapper's content is a Bootstrap modal, then when the inner
// modal is hidden, remove the entire thing, including wrapper.
Expand Down
2 changes: 1 addition & 1 deletion srcjs/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ exports.notifications = (function() {
if ($panel.length > 0)
return $panel;

$('body').append('<div id="shiny-notification-panel">');
$(document.body).append('<div id="shiny-notification-panel">');

return $panel;
}
Expand Down
2 changes: 1 addition & 1 deletion srcjs/shinyapp.js
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ var ShinyApp = function() {
var $container = $('.shiny-progress-container');
if ($container.length === 0) {
$container = $('<div class="shiny-progress-container"></div>');
$('body').append($container);
$(document.body).append($container);
}

// Add div for just this progress ID
Expand Down

0 comments on commit 3d11780

Please sign in to comment.