stla / jsTreeR

A wrapper of the jQuery plugin `jsTree`.
Other
46 stars 4 forks source link

Error in browser console when using the grid parameter #23

Closed ismirsehregal closed 8 months ago

ismirsehregal commented 11 months ago

Running the below app using the latest jsTreeR dev version and the latest shiny cran version gives me:

image

library(jsTreeR)
library(shiny)

nodes <- list(
  list(
    text = "Fruits",
    type = "fruit",
    icon = "supertinyicon-transparent-raspberry_pi",
    a_attr = list(class = "helvetica"),
    children = list(
      list(
        text = "Apple",
        type = "fruit",
        data = list(
          price = 0.1,
          quantity = 20,
          cssclass = "lightorange"
        )
      ),
      list(
        text = "Banana",
        type = "fruit",
        data = list(
          price = 0.2,
          quantity = 31,
          cssclass = "lightorange"
        )
      ),
      list(
        text = "Grapes",
        type = "fruit",
        data = list(
          price = 1.99,
          quantity = 34,
          cssclass = "lightorange"
        )
      ),
      list(
        text = "Mango",
        type = "fruit",
        data = list(
          price = 0.5,
          quantity = 8,
          cssclass = "lightorange"
        )
      ),
      list(
        text = "Melon",
        type = "fruit",
        data = list(
          price = 0.8,
          quantity = 4,
          cssclass = "lightorange"
        )
      ),
      list(
        text = "Pear",
        type = "fruit",
        data = list(
          price = 0.1,
          quantity = 30,
          cssclass = "lightorange"
        )
      ),
      list(
        text = "Strawberry",
        type = "fruit",
        data = list(
          price = 0.15,
          quantity = 32,
          cssclass = "lightorange"
        )
      )
    ),
    state = list(
      opened = TRUE
    )
  ),
  list(
    text = "Vegetables",
    type = "vegetable",
    icon = "supertinyicon-transparent-vegetarian",
    a_attr = list(class = "helvetica"),
    children = list(
      list(
        text = "Aubergine",
        type = "vegetable",
        data = list(
          price = 0.5,
          quantity = 8,
          cssclass = "lightgreen"
        )
      ),
      list(
        text = "Broccoli",
        type = "vegetable",
        data = list(
          price = 0.4,
          quantity = 22,
          cssclass = "lightgreen"
        )
      ),
      list(
        text = "Carrot",
        type = "vegetable",
        data = list(
          price = 0.1,
          quantity = 32,
          cssclass = "lightgreen"
        )
      ),
      list(
        text = "Cauliflower",
        type = "vegetable",
        data = list(
          price = 0.45,
          quantity = 18,
          cssclass = "lightgreen"
        )
      ),
      list(
        text = "Potato",
        type = "vegetable",
        data = list(
          price = 0.2,
          quantity = 38,
          cssclass = "lightgreen"
        )
      )
    )
  )
)

grid <- list(
  columns = list(
    list(
      width = 200,
      header = "Product",
      headerClass = "bolditalic yellow centered",
      wideValueClass = "cssclass"
    ),
    list(
      width = 150,
      value = "price",
      header = "Price",
      wideValueClass = "cssclass",
      headerClass = "bolditalic yellow centered",
      wideCellClass = "centered"
    ),
    list(
      width = 150,
      value = "quantity",
      header = "Quantity",
      wideValueClass = "cssclass",
      headerClass = "bolditalic yellow centered",
      wideCellClass = "centered"
    )
  ),
  width = 600
)

types <- list(
  fruit = list(
    a_attr = list(
      class = "lightorange"
    ),
    icon = "supertinyicon-transparent-symantec"
  ),
  vegetable = list(
    a_attr = list(
      class = "lightgreen"
    ),
    icon = "supertinyicon-transparent-symantec"
  )
)

ui <-   fluidPage(
  tags$head(
    tags$style(
      HTML(c(
        ".lightorange {background-color: #fed8b1;}",
        ".lightgreen {background-color: #98ff98;}",
        ".bolditalic {font-weight: bold; font-style: italic; font-size: large;}",
        ".yellow {background-color: yellow !important;}",
        ".centered {text-align: center; font-family: cursive;}",
        ".helvetica {font-weight: 700; font-family: Helvetica; font-size: larger;}"
      ))
    )
  ),
  titlePanel("jsTree grid"),
  jstreeOutput("jstree")
)

server <-   function(input, output){
  output[["jstree"]] <-
    renderJstree(jstree(nodes, search = TRUE, grid = grid, types = types))
}

shinyApp(ui, server)
ismirsehregal commented 11 months ago

PS: it seems this error appears again every time the browser window is resized.

ismirsehregal commented 11 months ago

Chrome gives:

init.ts:245 Uncaught TypeError: Cannot read properties of undefined (reading 'onResize') at HTMLDivElement. (init.ts:245:15) at Function.each (jquery.min.js:2:3003) at S.fn.init.each (jquery.min.js:2:1481) at C (init.ts:236:30) at e.value (debounce.ts:79:19) at debounce.ts:47:15 (anonymous) @ init.ts:245 each @ jquery.min.js:2 each @ jquery.min.js:2 C @ init.ts:236 value @ debounce.ts:79 (anonymous) @ debounce.ts:47 setTimeout (async) value @ debounce.ts:42 regular @ sendImageSize.ts:33 (anonymous) @ debounce.ts:116 setTimeout (async) n @ debounce.ts:110 dispatch @ jquery.min.js:2 v.handle @ jquery.min.js:2 init.ts:245 Uncaught TypeError: Cannot read properties of undefined (reading 'onResize') at HTMLDivElement. (init.ts:245:15) at Function.each (jquery.min.js:2:3003) at S.fn.init.each (jquery.min.js:2:1481) at C (init.ts:236:30) at e.value (debounce.ts:79:19) at debounce.ts:47:15 (anonymous) @ init.ts:245 each @ jquery.min.js:2 each @ jquery.min.js:2 C @ init.ts:236 value @ debounce.ts:79 (anonymous) @ debounce.ts:47 setTimeout (async) value @ debounce.ts:42 regular @ sendImageSize.ts:33 (anonymous) @ debounce.ts:116 setTimeout (async) n @ debounce.ts:110 dispatch @ jquery.min.js:2 v.handle @ jquery.min.js:2

ismirsehregal commented 11 months ago

Maybe this is related to the latest changes in shiny described here?

ismirsehregal commented 11 months ago

@gadenbuie do you think the above could be caused by shiny's new asynchronous rendering?

stla commented 11 months ago

Yeah, I saw this error with Chrome. The file init.ts is from Shiny I think, so I believe these errors are not related to 'jsTreeR'.

ismirsehregal commented 11 months ago

The errors remain after installing shiny's latest dev version.

gadenbuie commented 11 months ago

Hi all! I tested the example app in this thread with an older version of Shiny (1.7.4) that predates the async changes and the error is still present. This means it's unlikely to be due to a recent shiny-specific change.

I poked around a little bit but didn't see anything immediately obvious as to the source of the error. I can say that when the browser window is resized, htmlwidgets that are shiny outputs will attempt to call the .resize() method of the output binding so that the htmlwidget can update it's size. This is mostly used by plots, e.g. resizing the plot to fit the new container constraints.

In the case of jstreer, where there is no resize logic or desire to resize the component, these warnings are harmless and can probably be ignored.

ismirsehregal commented 11 months ago

@gadenbuie thanks for looking into this and sharing your insights! Good to know that you consider the error uncritical.

@stla I just realized that these errors seem to be related to the grid table

Once we use jstree(nodes, search = TRUE, types = types) instead of jstree(nodes, search = TRUE, grid = grid, types = types) in the above example the errors are gone. This is probably also the reason why the error was overlooked in the past.

ismirsehregal commented 10 months ago

@stla I just saw another error, which fires only once on startup (not after every window resize):

image

The browser points at:

image

stla commented 10 months ago

Thanks. That should be fixed now. Strange that we didn't see this problem earlier.

ismirsehregal commented 10 months ago

The resize method in jstreer.js currently is empty.

stewerner commented 8 months ago

I'm having the same issue and to me it seems under certain circumstances this can impact other widgets. In my affected app when using a jstree with a grid, there are plotly charts, which no longer get resized once the browser window size changes (or e.g. the right sidebar from {shinydashboardPlus} is opened). Once I comment out the grid parameter everything works as expected (the plots are resized again).

Unfortunately, so far I wasn't able to condense this into a reprex (only happens with my bigger app).

@stla or @gadenbuie is there any minimal .resize() method {jsTreeR} could use to prevent the above mentioned errors?

Thanks in advance!

ismirsehregal commented 8 months ago

Perhaps it would be possible for shiny to check if the method exists before calling it, to prevent the error:

image

using something like this:

if (typeof binding.onResize === "function") { 
  binding.onResize();
}

However, I still don't understand why this is happening only when using the grid plugin. Maybe something is overwritten?

stla commented 8 months ago

@ismirsehregal Are you sure? The error is:

Cannot read properties of undefined (reading 'onResize')

So that means that binding is undefined. No?

ismirsehregal commented 8 months ago

Hi Stéphane, sure about what? The error happening only when the grid is active (and the window is resized)?

Here I read:

Note - if a me.onChange property exists but is not a function, a TypeError will be thrown just like when you call any non-function as a function in JavaScript.

..and thought maybe that is the case. However, my JS knowledge is quite limited.. so I could very well be wrong.

So maybe shiny needs to check for binding to be defined.

stla commented 8 months ago

Yes, the grid has the class shiny-bound-output, among others. Indeed, one gets it if in the console we do:

$(".shiny-bound-output")

It is the element

<div class="jstree-grid-wrapper jstreer html-widget html-widget-output shiny-report-size html-fill-item shiny-bound-output" style="width: 600px;">......</div>

But this element has no attached data. So binding is undefined.

Maybe we should remove the shiny-bound-output class of this element in the JS code:

$(".jstree-grid-wrapper").removeClass("shiny-bound-output")

What do you think, @gadenbuie ?

stewerner commented 8 months ago

@stla thanks for looking into this! I can confirm, that once I remove the class "shiny-bound-output" from the "jstree-grid-wrapper" div (via the browser) the plotly charts in my app are again resized. It would be great if this could somehow be incorporated into {jsTreeR}.

ismirsehregal commented 8 months ago

I think the question is: what is a clean way to do so?

Is simply removing the class "shiny-bound-output" appropriate? I have no experience in building custom outputs but I guess the issue could be, that the grid is a parent div of the tree but the tree was defined as the output? What is shiny expecting from the data attribute?

stla commented 8 months ago

@stewerner

I've done a change: I included the class removal in the ready.jstree event listener. It works but I'm not sure there's no undesirable effects. Could you try it please?

remotes::install_github("stla/jsTreeR@unbind_grid")
stewerner commented 8 months ago

@stla this version works fine - thank you!

So far I haven't encountered any unwanted side-effects.

ismirsehregal commented 8 months ago

I raised another issue here hoping for clarification.

ismirsehregal commented 8 months ago

Stéphane's explanation on the root cause can be found here. Thanks for this!

ismirsehregal commented 8 months ago

Fixed via https://github.com/stla/jsTreeR/commit/d199614f5a202d519146d07a071c61931b7841a4 Thanks Stéphane!