You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hello, hope you're doing fine. Thanks a lot for all your work in this wonderful package.
General explanation
This issue deals with problems I've encountered recently when trying to update the disabled dates in an airDatepickerInput using the updateAirDateInput function. The errors occur on some edge cases, particularly when trying to update the disabled dates in the selector to have either zero disabled dates (that is, no disabled dates), or just one.
Demo
I've created a little demo that shows the tricky cases, it just creates a datepicker object with some disabled dates and updates them:
library(shiny)
library(shinyWidgets)
library(magrittr)
# Generate all dates of march 2021march_days<- seq(as.Date("2021-03-01"), as.Date("2021-03-31"), by="days")
march_days<- sapply(march_days, function(x){toString(x)})
# The disabled ones will be all of those whose day is not multiple of 5disabled_days<- setdiff(march_days, c("2021-03-05", "2021-03-10", "2021-03-15",
"2021-03-20", "2021-03-25", "2021-03-30"))
ui<- fluidPage(
titlePanel("Updating an airDatepicker"),
sidebarLayout(
sidebarPanel(
# Setting the initially disabled
airDatepickerInput("date_picker", "Date",
minDate="2021-03-01",
maxDate="2021-04-1",
disabledDates=disabled_days,
update_on="close",
range=TRUE),
actionButton("button", label="Update picker")
),
mainPanel(
textOutput("text")
)
)
)
server<-function(input, output, session) {
# Show the datesoutput$text<- renderText({
input$date_pickerinput$date_picker_button
toString(isolate(input$date_picker))
})
# Update the disabled dates
observeEvent(input$button, {
print("clicking button")
# The tricky cases
updateAirDateInput(session, "date_picker", options=list(
# disabledDates = c()# disabledDates = NULL# disabledDates = character(0)# disabledDates = "2021-03-24"
))
})
}
shinyApp(ui=ui, server=server)
You can uncomment each of the possibilities and run the app to analyze the behaviour.
In order to explain the errors more easily, I will show the code that deals with the update of the disabledDates on the JS side, it is inside the inst/assets/air-datepicker2/datepicker-binding.js file:
At the moment, for the different cases the app behaves as follows:
c() and NULL: both are NULL values and therefore behave equally. The browser will receive the value and set the highlightedDates variable to Javascript's null, which will cause an error when filtering the dates. I don't know what happens when nothing is returned from the onRenderCell function, but the end result is that all elements are enabled.
character(0): The browser receives this value as an empty javascript vector and therefore all values are enabled.
"2021-03-24": The browser will receive the value and set the highlightedDates variable to string variable. Just as with null this crashes the code and leaves the app with all values enabled.
Expected behaviour
I think there is room for discussion on how the NULL values should be interpreted, but in any case the code should not crash and therefore I consider the current current code to be incomplete (it has a bug). A possibility for how the values could behave is the following one:
c() and NULL: I think are are two valid interpretations of NULL values for the update method. Stock Shiny update functions interpret NULL values by ignoring them, which leaves the widget as it is right now, without updating them. Making the code work that way would be somewhat complicated as the Javascript code stands right now. Another possible way to interpret NULL values is just to treat them just like the c(0), that could be done in either the R side (by converting NULL option to c(0) before sending it to the browser) or on the JS side (look at the code below).
character(0): The way it is handled currently is correct.
"2021-03-24": It should set the date specified as the only disabled date.
Possible solution
Although somewhat of a hotfix, a solution to both of the crashes would be to set the specific safeguards on the JS side for the possibilities that crash the code:
vardisabledDates=[];if(data.options.hasOwnProperty("disabledDates")){varoptDates=data.options.disabledDates;// R's NULL will be interpreted as enabling all dates if(optDates===null){disabledDates=[];}// Correct handling of the single stringelseif(typeof(optDates)==="string"){disabledDates=[optDates];}else{disabledDates=opDates;}}
As I mentioned before, the interpretation of the values and how to deal with them could be done in several different ways. A solution to the errors could be implemented on the R side too and the semantics of NULL could be different.
Thanks in advance :).
The text was updated successfully, but these errors were encountered:
Thanks a lot for the quick response.
I'll have a look at the code and will check that everything is correct.
I'll close the issue if I find no errors, thanks again!
Hello, hope you're doing fine. Thanks a lot for all your work in this wonderful package.
General explanation
This issue deals with problems I've encountered recently when trying to update the disabled dates in an
airDatepickerInput
using theupdateAirDateInput
function. The errors occur on some edge cases, particularly when trying to update the disabled dates in the selector to have either zero disabled dates (that is, no disabled dates), or just one.Demo
I've created a little demo that shows the tricky cases, it just creates a datepicker object with some disabled dates and updates them:
You can uncomment each of the possibilities and run the app to analyze the behaviour.
In order to explain the errors more easily, I will show the code that deals with the update of the disabledDates on the JS side, it is inside the
inst/assets/air-datepicker2/datepicker-binding.js
file:Current behaviour
At the moment, for the different cases the app behaves as follows:
c()
andNULL
: both areNULL
values and therefore behave equally. The browser will receive the value and set thehighlightedDates
variable to Javascript'snull
, which will cause an error when filtering the dates. I don't know what happens when nothing is returned from theonRenderCell
function, but the end result is that all elements are enabled.character(0)
: The browser receives this value as an empty javascript vector and therefore all values are enabled."2021-03-24"
: The browser will receive the value and set thehighlightedDates
variable to string variable. Just as withnull
this crashes the code and leaves the app with all values enabled.Expected behaviour
I think there is room for discussion on how the
NULL
values should be interpreted, but in any case the code should not crash and therefore I consider the current current code to be incomplete (it has a bug). A possibility for how the values could behave is the following one:c()
andNULL
: I think are are two valid interpretations ofNULL
values for the update method. Stock Shiny update functions interpretNULL
values by ignoring them, which leaves the widget as it is right now, without updating them. Making the code work that way would be somewhat complicated as the Javascript code stands right now. Another possible way to interpretNULL
values is just to treat them just like thec(0)
, that could be done in either the R side (by convertingNULL
option toc(0)
before sending it to the browser) or on the JS side (look at the code below).character(0)
: The way it is handled currently is correct."2021-03-24"
: It should set the date specified as the only disabled date.Possible solution
Although somewhat of a hotfix, a solution to both of the crashes would be to set the specific safeguards on the JS side for the possibilities that crash the code:
As I mentioned before, the interpretation of the values and how to deal with them could be done in several different ways. A solution to the errors could be implemented on the R side too and the semantics of
NULL
could be different.Thanks in advance :).
The text was updated successfully, but these errors were encountered: