glin / reactable

Interactive data tables for R
https://glin.github.io/reactable
Other
612 stars 79 forks source link

Custom JS function not work with recent htmlwidgets version (>=1.6.3) #348

Closed jhk0530 closed 6 months ago

jhk0530 commented 7 months ago

Hi, I don't think this is an issue with reactable, but I'm raising it just in case of further problems.

Since the version of htmlwidgets was updated from 1.6.2 to 1.6.3 on November 22, my custom Javascript function in onClick is not working.


library(reactable)

reactable(iris[1:5, ], 
    selection = "single", 
    onClick = JS("function(row, column, rowInfo) {
        console.log('asdf');
        row.toggleRowSelected();
      }")
)

This code selects a row when the table is clicked and outputs a log named asdf. (htmlwidgets 1.6.2)

However, if you are using the htmlwidgets 1.6.3 version, the click will not select the row and output a log.

I think user should not using 1.6.3 version until this problem solved.

Thanks.

glin commented 7 months ago

Hi, thanks for the report. Looking into it now, but I might not get back until after the weekend.

glin commented 7 months ago

Here's a quick workaround fix with limited testing if you install the latest dev version: https://github.com/glin/reactable/commit/1d60ea6d3eaf9a655cf6ebccb973c3b03ae4969d

When I get a chance, I'll comment in the htmlwidgets issue and possibly do a CRAN release. It seems like a complicated issue, and a better place for the fix might be either reactR or htmlwidgets.

jhk0530 commented 7 months ago

Thanks, glin.

So Here's what I understand

  1. Custom JS function didn't work properly for some reason. ( = I used it correctly )
  2. it seems problem with in reactR or htmlwidgets not reactable
  3. But with recent dev version of reactable (will be CRAN soon), you can use custom JS function with recent htmlwigets version

Am I right?

timelyportfolio commented 7 months ago

When I read through the htmlwidgets release notes, I figured the recursion only into list would surface somewhere, and here it is. It seems to me without a whole lot of deep thought that reactR will be the correct and least invasive place to fix. If we use @glin approach, we could apply here and I think most will be solved. I'll plan to test today. Let me know if anyone has thoughts.

Reference

https://github.com/ramnathv/htmlwidgets/blob/master/NEWS.md?plain=1#L5-L7

*htmlwidgets no longer recurses into list-like objects when searching for JavaScript strings wrapped in `JS()`, unless the object has the class `"list"` or `"data.frame"`. This stops htmlwidgets from (possibly infinitely) recursively searching objects that are not actually recursive. Widget authors who relied on the previous behavior should ensure that their widget's `JS()` calls are wrapped in objects that have the class `"list"` or `"data.frame"`. 
timelyportfolio commented 7 months ago

If anyone has a chance, please test reactable CRAN version (not patched version) with remotes::install_github("react-R/reactR@develop") and remotes::install_github("ramnathv/htmlwidgets").

jhk0530 commented 7 months ago

Nice work. It works. (at least for below code)

library(reactable)

reactable(iris[1:5, ], 
    selection = "single", 
    onClick = JS("function(row, column, rowInfo) {
        console.log('asdf');
        row.toggleRowSelected();
      }")
)

Versions

@timelyportfolio

mbjohnsonmn commented 7 months ago

Thank you very much for this post and the fix! I can confirm it fixed the issue for me, i.e., the reactable silently failing to render.

a-drumm commented 7 months ago

I couldn't run an example from the documentation and was very very confused. Using the versions suggested (reactable 0.4.4, reactR 0.5.1, and htmlwidgets 1.6.3.9000) it runs fine.

jhk0530 commented 6 months ago

Hello guys, update htmlwidgets into 1.6.4 (CRAN) will solve this problem.

So I'll close this issue. Thanks !