-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Added dismissalert to remove arbitrary number of messages #30
Changes from all commits
b57a772
e8c393d
60534f6
66b430e
17b4faf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,4 @@ | |
.RData | ||
.Ruserdata | ||
inst/doc | ||
.DS_Store |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
# Generated by roxygen2: do not edit by hand | ||
|
||
export(closeAlert) | ||
export(runExample) | ||
export(shinyalert) | ||
export(useShinyalert) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,8 @@ | ||
var swalService = new SwalService({showPendingMessage: false}); | ||
shinyalert = {}; | ||
shinyalert.num = 0; // Used to make the timer work | ||
shinyalert.instances = []; // Used to make the timer work | ||
|
||
Shiny.addCustomMessageHandler('shinyalert.show', function(params) { | ||
shinyalert.num++; | ||
|
||
var callbackJS = function(value) {}; | ||
if (params['callbackJS'] != null) { | ||
|
@@ -13,15 +12,27 @@ Shiny.addCustomMessageHandler('shinyalert.show', function(params) { | |
} | ||
|
||
var callbackR = function(value) {}; | ||
if (params['cbid'] != null) { | ||
var cbid = params['cbid']; | ||
delete params['cbid']; | ||
// Always create cbid | ||
var cbid = params['cbid']; | ||
delete params['cbid']; | ||
if(typeof cbid === 'string'){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is cbid not alwasy going to be a string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It wasn't before, but my understanding is that your PR ensures that |
||
callbackR = function(value) { | ||
Shiny.onInputChange(cbid, value); | ||
} | ||
} | ||
|
||
var callback = function(value) { | ||
// Remove instance immediately | ||
var ii = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use a better variable name |
||
for(ii in shinyalert.instances){ | ||
swalService.close( shinyalert.instances[ii].swal_id ); | ||
if( shinyalert.instances[ii].cbid === cbid ){ | ||
shinyalert.instances.splice( ii, 1 ); | ||
} | ||
break; | ||
} | ||
|
||
|
||
if ('compareVersion' in Shiny && | ||
Shiny.compareVersion(Shiny.version, ">=", "1.1.0") ) { | ||
Shiny.setInputValue(params['inputId'], value, {priority: "event"}); | ||
|
@@ -33,15 +44,54 @@ Shiny.addCustomMessageHandler('shinyalert.show', function(params) { | |
delete params['inputId']; | ||
} | ||
|
||
if (params['timer'] != 0) { | ||
var timer = params['timer']; | ||
delete params['timer']; | ||
|
||
var swal_id = swalService.swal(params, callback); | ||
shinyalert.instances.push({ | ||
swal_id : swal_id, | ||
cbid : cbid | ||
}); | ||
|
||
// Changed timer != 0 to timer > 0 | ||
if (timer > 0) { | ||
setTimeout(function(x) { | ||
if (x == shinyalert.num) { | ||
swalService.close(); | ||
var ii = 0; | ||
for(ii in shinyalert.instances){ | ||
swalService.close( x ); | ||
if( shinyalert.instances[ii].swal_id === x ){ | ||
shinyalert.instances.splice( ii, 1 ); | ||
} | ||
break; | ||
} | ||
}, params['timer'], shinyalert.num); | ||
}, timer, swal_id); | ||
} | ||
delete params['timer']; | ||
|
||
Shiny.unbindAll($(".sweet-alert")); | ||
swalService.swal(params, callback); | ||
}); | ||
|
||
Shiny.addCustomMessageHandler('shinyalert.close', function(params) { | ||
var n = (params && params.count) || shinyalert.instances.length, | ||
cbid = params.cbid; | ||
|
||
var i, item; | ||
if( typeof cbid === 'string' ){ | ||
// close specific alert | ||
for( i = 0; i < shinyalert.instances.length; i++ ){ | ||
item = shinyalert.instances[i]; | ||
if( item.cbid === cbid ){ | ||
swalService.close( item.swal_id ); | ||
shinyalert.instances.splice( i, 1 ); | ||
break; | ||
} | ||
} | ||
} else { | ||
// dismiss n alerts | ||
item = shinyalert.instances.splice(0, n); | ||
for( i = 0; i < item.length; i++ ){ | ||
swalService.close( item[i].swal_id ); | ||
} | ||
} | ||
|
||
}); |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
shinyalert.R | ||
shinyalert.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the if statement I added a comment that the serialization isn't happening to prevent a performance issue #19
Did you notice that and test if this change is going to re-introduce that issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recommend
params <- as.list(environment())
at the very beginning. Chances are too small for two alerts sharing the same cbid withrunif
postfix. If you get rid of the enclosing environment,digest::digest
will be quite fast as everything else should be basic types and contain no environment. Alternatively you could useutils::capture.output({print(environment())})
to distinguish environments because the runtime environment of R functions will unlikely to be named, meaning its memory address will be printed, resulting in something like"<environment: 0x7f802d60a1b0>"
. Serializing string is much faster than serializing an environment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, if you do want to keep
params <- as.list(environment())
, you can create randomcbid
with no digests for alerts without callbacks. The returnedcbid
can be used to close that specific alert or writers can use thatcbid
to generate their own callbacks outside ofshinyalert
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The line
params <- as.list(environment())
is not for serialization or creating a unique ID, it was simply a way to get a list of all the function arguments values - default values for arguments that weren't provided and the user-supplied values for the arguments that were provided. Do you know of an alternative way to get all the arguments of the current function as a list?But anyway, you're right that the digest isn't actually required , it was only to add some randomness. Instead of using the params hash plus a random integer, it might be better to just use the
uuid
packageThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that way you are right, but your function arguments contains no function except
callbackR
, which will be removed before serialization. Therefore serialization should be fast. Also personally I think usingrunif
is good enough to create unique IDs unless people set seed before calling the function. In that case, you might want to considerutils::capture.output({print(environment())})
I proposed to ensure different IDs created.