Skip to content
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

DataTables 1.10.2 #558

Merged
merged 11 commits into from Aug 22, 2014
Merged

DataTables 1.10.2 #558

merged 11 commits into from Aug 22, 2014

Conversation

yihui
Copy link
Member

@yihui yihui commented Aug 1, 2014

I just integrated the new version of the DataTables library (1.10.2) into the development version of shiny. There were tremendous changes in parameter names from DataTables 1.9.x to 1.10.x, and a guide for upgrading parameter names from DT 1.9.x to 1.10.x is here: https://datatables.net/upgrade/1.10-convert I have tried to automatically correct some of the old parameter names in shiny (e.g. iDisplayLength -> pageLength), but this automatic correction certainly won't work for all use cases, especially if you have deeply customized your DataTables using complicated JavaScript options. You can see rstudio/shiny-examples@6f2a5618 for examples of converting DT 1.9 names to 1.10 names.

@yihui yihui added this to the 0.10.2 milestone Aug 1, 2014
@yihui
Copy link
Member Author

yihui commented Aug 1, 2014

Okay, I rewrote parseQueryString(nested = TRUE) and I feel more comfortable with it now.

> str(parseQueryString('a[i1][j1]=x&b[i1][j1]=y&b[i2][j1]=z', nested = TRUE))
List of 2
 $ a:List of 1
  ..$ i1:List of 1
  .. ..$ j1: chr "x"
 $ b:List of 2
  ..$ i1:List of 1
  .. ..$ j1: chr "y"
  ..$ i2:List of 1
  .. ..$ j1: chr "z"

I do not think I have anything else to add to this PR now.

@xydrolase
Copy link
Contributor

I like this update! Upgrading dataTables to 1.10 truly enhanced the extensibility by allowing users to specify new callback functions added in 1.10 as arguments of renderDataTable.

@HermanSontrop
Copy link

Hi Yihui,

I tested the version of the datatable you added. The basic functions work well. However, with the new version I guess the old way of specifying certain options doesn't work anymore, right? For instance, specifying iDisplayLength = 10 does nothing now.

One thing that is pretty annoying is that if there is an Inf or NaN value somewhere in the table, the rendering of the table fails completely. In that case you get an Invalid JSON response message from the browser, see http://datatables.net/tn/1

Also, if in a Shiny app a table is not yet rendered, but you select a tabpanel which displays the table, you get an Ajax error, see http://datatables.net/tn/7. In older versions I didn't get such a message I believe (I'm not completely sure though).

If I need to test more just let me know....

best Herman

My session info:

R version 3.1.1 (2014-07-10)
Platform: x86_64-w64-mingw32/x64 (64-bit)

locale:
[1] LC_COLLATE=Dutch_Netherlands.1252  LC_CTYPE=Dutch_Netherlands.1252    LC_MONETARY=Dutch_Netherlands.1252
[4] LC_NUMERIC=C                       LC_TIME=Dutch_Netherlands.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] ggvis_0.3.0.9001  shiny_0.10.1.9001

loaded via a namespace (and not attached):
 [1] assertthat_0.1  digest_0.6.4    dplyr_0.2       htmltools_0.2.4 httpuv_1.3.0    magrittr_1.0.1  parallel_3.1.1 
 [8] plyr_1.8.1      Rcpp_0.11.2     RJSONIO_1.2-0.2 tools_3.1.1     xtable_1.7-3   

1. sorted columns have different colors;
2. correct position of the processing info;
3. override the width of text input (search fields), otherwise they will be too wide (206px defined in bootstrap.min.css);
…taTables 1.10 passes parameters as arrays

e.g. columns[0][search][value]=foo&columns[1][search][value]=bar

we need list(columns = list(`0` = list(search = list(value = 'foo')), `1` = ...)
j <- gsub('[.].*', '', DT10[, 1]) %in% nms
# I cannot help you upgrade automatically in these cases, so I have to stop
if (any(grepl('[.]', DT10[j, 1])) || any(grepl('[.]', DT10[j, 2]))) stop(msg)
warning(msg)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jcheng5, @wch was wondering if you prefer this to be warning() or message(). Neither of us has a strong preference here, but we want to ask you just in case.

@yihui
Copy link
Member Author

yihui commented Aug 21, 2014

@HermanSontrop I just added an internal function to make it slightly easier to transit from DT 1.9 to 1.10, e.g. iDisplayLength will be converted to pageLength automatically, but this function will not cover all the cases, and you are strongly recommended to read this page and change your parameter names accordingly: https://datatables.net/upgrade/1.10-convert

I will take a look at the other two issues you mentioned. Thanks for the feedback!

wch added a commit that referenced this pull request Aug 22, 2014
@wch wch merged commit 0fb4ab2 into rstudio:master Aug 22, 2014
@yihui yihui mentioned this pull request Aug 23, 2014
@yihui
Copy link
Member Author

yihui commented Aug 23, 2014

@HermanSontrop I have confirmed the first issue you reported (#574). For the second one, could you make a minimal reproducible example, and file a new issue? Thanks!

@yihui yihui deleted the feature/datatables1.10 branch August 23, 2014 05:26
@HermanSontrop
Copy link

Hi Yihui,

I'll try to come up with a toy example (not sure if in my case everything
was caused by the Inf value instead of NaN). By the way, Shiny version
0.10.1.9002 breaks some code. For instance, the SuperZip example by Joe. It
also happens in the RStudio Gallery page. The apps starts normally, but
after the user does anything the app breaks. This does not happen in Shiny
version 0.10.1

best Herman

On Sat, Aug 23, 2014 at 7:26 AM, Yihui Xie notifications@github.com wrote:

@HermanSontrop https://github.com/HermanSontrop I have confirmed the
first issue you reported (#574
#574). For the second one, could
you make a minimal reproducible example, and file a new issue? Thanks!


Reply to this email directly or view it on GitHub
#558 (comment).

@yihui
Copy link
Member Author

yihui commented Aug 25, 2014

@HermanSontrop I just tried the SuperZip example and I did not see anything was broken (both the gallery example and my locally launched app worked well). Could you tell me your OS and web browser version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants