omines / datatables-bundle

DataTables bundle for Symfony
https://omines.github.io/datatables-bundle/
MIT License
255 stars 114 forks source link

Javascript improvements #145

Closed danut007ro closed 4 years ago

danut007ro commented 4 years ago

Hello,

This pull request contains the following improvements:

I use the urlBuilder option for filtering with LexikFormFilterBundle. Based on selected filters I'm building the url dynamically.

How does this sound? Thanks.

curry684 commented 4 years ago

For the scrollX - we don't intend to mirror every option available in DataTables as there's tons, plugins affect which ones, and we don't want to be forced to update the bundle when DataTables adds or removes an option. The ones we manage serverside are the ones we 'have' to because of facilities we support.

For your use case, why not this?

$(function() {
    $('#presidents').initDataTables({{ datatable_settings(datatable) }}, {
        scrollX: true,
        dom: null,
        urlBuilder: url => { doSomething(); },
    });
});
danut007ro commented 4 years ago

You are right about the scrollX parameter, I can do it without modifying the DataTable.

If I'm setting the dom parameter to null I get anUncaught TypeError: Cannot read property 'split' of null on jquery.dataTables.js:3580 so it looks like it's used if I pass it.

I'm not sure I understand about urlBuilder. How will urlBuilder be called when builing url for ajax request?

Thank you very much.

curry684 commented 4 years ago

I'm sorry I didn't test my code, was a pseudo example. In the case of the dom parameter I'm not sure I see the problem. You can define it server side, and/or override it clientside with the method shown, and a plugin can override it. My assumption that null would work for that was wrong but still if the plugin overrules it it should still do it no matter what you put in there.

As for the urlBuilder I assumed it was a DT feature, but I see in the code you propose it as an extension to the DT config. I think the use case is valid in itself, but I don't see the merits in introducing another config element for it, especially not if it is only used for followup requests.

I'd prefer just allowing url to be a function, and invoking it as $.ajax(typeof config.url === 'function') ? config.url(dt) : config.url, that would make it consistent and predictable. Would that work for you?

danut007ro commented 4 years ago

Hello and thanks for the feedback. I added the modification you suggested for config.url.

I also added the same for config.options. As the dtOpts variable is built using $.extend there is no way to remove an option from it. With this modification, the config.options can be a function which will be called after building dtOpts and you can update it (in my case, remove dom from it).

Thank you.

curry684 commented 4 years ago

Looks fine now, thanks!

curry684 commented 4 years ago

Released in https://github.com/omines/datatables-bundle/releases/tag/0.4.2