DT icon indicating copy to clipboard operation
DT copied to clipboard

Hitting 'escape' after starting an edit with `editable = "cell"` throws an error

Open stla opened this issue 5 years ago • 1 comments

Hello

Try to run a Shiny app rendering a datatable with the option editable = "cell", and double-click on a cell. If you hit 'escape' this cancels the edit without any issue. But if you type something in the text input and then you hit 'escape', a JavaScript error is thrown, and the edit is not cancelled.

Such a code is executed when editable = "cell":

js0 <- "
$(document).ready(function(){
  var changed = false;
  var $input = $('#myinput');
  $input.on('change', function() {
    console.log('change');
    // changed = true;
    // var valueNew = $input.val();
    $input.remove();
  }).on('blur', function() {
    console.log('blur');
    // if (!changed) $input.trigger('change');
  }).on('keyup', function(e) {
    console.log('keyup');
    // hit Escape to cancel editing
    if (e.keyCode === 27) $input.trigger('blur'); 
  });
});"

Try it with this simple app, this reproduces the error:

library(shiny)
ui <- fluidPage(
  tags$head(tags$script(HTML(js0))),
  tags$input(id = "myinput", type = "text")
)
server <- function(input, output, session){}
shinyApp(ui, server)

I'm not sure to exactly understand what happens, but if you comment $input.remove() you will see (in the browser console) that when you start an edit and you hit 'escape', this triggers the change event before triggering the blur event. So when $input.remove() is not commented, and when you start an edit and hit 'escape', this firstly triggers the change event, which removes $input, and then the error is thrown by the subsequent blur event. Or something like that (again, I don't exactly understand what happens).

I think that replacing if (e.keyCode === 27) $input.trigger('blur'); with if (e.keyCode === 27) $input.remove(); could fix this issue. I will fork the repo to try.

stla avatar Jul 05 '20 06:07 stla

Using $input.trigger('blur') instead of $input.remove() sounds good to me.

yihui avatar Aug 04 '20 21:08 yihui