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

Pass options to selectize to sort the filter levels #1083

Closed
DavidBlairs opened this issue Sep 4, 2023 · 11 comments · Fixed by #1120
Closed

Pass options to selectize to sort the filter levels #1083

DavidBlairs opened this issue Sep 4, 2023 · 11 comments · Fixed by #1120

Comments

@DavidBlairs
Copy link

Hello!

I have a local copy of DT that I've modified to sort the filter drop down levels in alphabetical order. I've added a line after line 507 in datatables.js:

hideSelected: true,
sortField: [{'field': 'text', 'direction': 'asc'}], # <- this line
onChange: function(value) {

Which seems to do the trick. I'd hopefully like to get this added in as an optional feature but as was mentioned in #1049, we want to avoid adding more options to the datatables function. I also want to avoid using the updateFilters function as it slows things down and complicates things more than I think should be necessary.

I'm curious what might be the best way to do it? I imagine passing custom parameters to the selectize input has probably come up before.

@stla
Copy link
Collaborator

stla commented Sep 6, 2023

But DT does not use selectize, does it? I can be wrong.

@DavidBlairs
Copy link
Author

I believe it is under:
image

Its used for the drop down filters in datatables.js line 502:
image

@stla
Copy link
Collaborator

stla commented Sep 6, 2023

Ok, didn't know that.

I observed there are several requests regarding passing options, e.g. this one for the dropdowns and another request for the sliders. Maybe it's time to consider these requests.

Do you think there are other selectize options that could be useful?

@DavidBlairs
Copy link
Author

@stla Most likely, none that spring to mind from my own use cases:

https://selectize.dev/docs/usage

Since these arn't built into datatables by default, they're certainly the least customisable aspects of DT. Maybe there's a way to add a catch all R list to pass any parameters you'd like to avoid lengthy parameters documentation?

I could definitely see an advantage to being able to control these drop downs more (passed options, callbacks etc). As was discussed in #587, being able to filter drop downs to update with what's actually in the table was a popular request and the solution was the updateFilters function. In practice, this function makes things quite slow and combining it with updateSearch breaks a suggested implementation by @mikmart when dealing with numeric columns. I'd think this could be better done with having more control on the javascript side to how selectize functions.

@mikmart
Copy link
Contributor

mikmart commented Jan 22, 2024

@DavidBlairs perhaps you're already aware, but in case you aren't: the order of the Selectize options is set by the factor levels in the data, so you can change the order by changing the factor levels.

That said, I think it would be fairly straightforward to allow passing a generic settings object to the filter initialization via the already-existing filter parameter in datatable(). I made a quick POC:

pak::pkg_install("mikmart/DT@b69df")

Usage looks like:

DT::datatable(
  data.frame(foo = factor(LETTERS, levels = sample(LETTERS))),
  filter = list(
    position = "top",
    settings = list(
      sortField = list(list(field = "text", direction = "desc"))
    )
  )
)

image

But probably a more robust implementation would require either separating the settings namespace to selectize and nouislider settings1, or requiring settings to be a list with an element for each column.

Footnotes

  1. Actually somehow it looks like noUiSlider and Selectize have no conflicting settings names, but of course that can't be relied upon.

@DavidBlairs
Copy link
Author

@mikmart Many thanks, I wasn't aware of that behaviour with selectize. I think your suggestion on implementation is ideal, would probably cover a lot of issues that arise in the future with the package. I agree that there may be conflicts in the future even if there isnt already but I also don't like the idea of having to group settings by the package they correspond to. Do you think it would be better to restrict the settings allowed? This would ensure that the settings allowed will perform as expected, they can be properly documented for the R package and it would avoid namespace issues.

@mikmart
Copy link
Contributor

mikmart commented Jan 23, 2024

I wouldn't wish the task of manual curation of options to pass to dependencies on anyone. And if the same setting name was preferred for both select and slider widgets for example, you'd have to arbitrarily rename one.

But you're right grouping settings by the specific library used to implement the filters doesn't seem right. I thought a bit and really what could make more sense is to group settings by the widget that they apply to. So for example:

datatable(
  filter = list(
    settings = list(
      select = list(sortField = ...),
      slider = list(...)
    )
  )
)

Or maybe even simplified one level to:

datatable(
  filter = list(
    select = list(sortField = ...),
    slider = list(...)
  )
)

@mikmart
Copy link
Contributor

mikmart commented Jan 23, 2024

This is a cleaner way to do it, and includes both select and slider settings, but also demonstrates that freedom to specify any settings gives you the tools to shoot yourself in the foot also, if you mess with something you shouldn't.

DT::datatable(
  data.frame(
    foo = factor(LETTERS, levels = sample(LETTERS)),
    bar = rnorm(seq_along(LETTERS)) |> round(3)
  ),
  filter = list(
    position = "top",
    settings = list(
      select = list(
        sortField = list(field = "text", direction = "asc")
      ),
      slider = list(
        orientation = "vertical"
      )
    )
  )
)

image

@mikmart
Copy link
Contributor

mikmart commented Jan 23, 2024

@yihui any thoughts on exposing the config for Selectize and noUiSlider filter widgets for users to modify directly like above?

I'm not 100% sure it's a good idea now. At least I think it would need to be documented as a "use at your own risk" feature. Then again if there's no supported way to do it presumably sufficiently motivated people will hack their way to solutions anyway.

@yihui
Copy link
Member

yihui commented Jan 24, 2024

@mikmart Thanks! I'm okay with your idea above. Usually I tend to give users more freedom even if it means they could shoot themselves in the foot. A "use at your own risk" note in the documentation is certainly helpful.

So for example:

datatable(
  filter = list(
    settings = list(
      select = list(sortField = ...),
      slider = list(...)
    )
  )
)

Or maybe even simplified one level to:

datatable(
  filter = list(
    select = list(sortField = ...),
    slider = list(...)
  )
)

I don't have a strong opinion on these two ways. Either way is fine to me.

On a slightly related note, I'm a little concerned by the fact that DT is still using a very old version of selectize.js: #1112 (comment) If we mention the selectize options in the documentation, I'm not sure if there are certain options in the new version of selectize.js that are not available in the old version or have changed since the old version. This could be confusing. Ideally, we should look into the possibility of upgrading selectize.js in DT. That's a different task, though.

@mikmart
Copy link
Contributor

mikmart commented Jan 24, 2024

@mikmart Thanks! I'm okay with your idea above. Usually I tend to give users more freedom even if it means they could shoot themselves in the foot. A "use at your own risk" note in the documentation is certainly helpful.

Okay thanks! In that case I'll see about adding documentation and putting a PR together for this.

On a slightly related note, I'm a little concerned by the fact that DT is still using a very old version of selectize.js: #1112 (comment) If we mention the selectize options in the documentation, I'm not sure if there are certain options in the new version of selectize.js that are not available in the old version or have changed since the old version. This could be confusing. Ideally, we should look into the possibility of upgrading selectize.js in DT. That's a different task, though.

Yes that's very true and also applies to noUiSlider (#1023). I tried searching for docs for older versions of both at some point, but haven't been able to find them easily available.

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 a pull request may close this issue.

4 participants