DataTablesSrc icon indicating copy to clipboard operation
DataTablesSrc copied to clipboard

advanced rendering

Open milux opened this issue 11 years ago • 11 comments

This patch tries to introduce "advanced rendering" into DataTables. "Advanced" is meant to express that the (display) render function is not limited to just returning the new content of a table cell, but rather change (render) the table cell directly. This includes classes, css style and - of course - the content of the cell.

This is realized by using the call() function to execute the render function in the context of the table cell, e.g. "this" refers to the rendered cell.

Example:

Old code to color cells with different css classes depending on their data:

        var fnCreatedPointsCell = function (nTd, sData) {
            //eTd: jQuery element, iPoints: determined level relevant for CSS (limited to 5 via Math.min() => points5plus class)
            var eTd = $(nTd), iPoints = Math.min(parseInt(sData), 5);
            //iterate over classes and add/remove them accordingly
            $.each(["points0", "points1", "points2", "points3", "points4", "points5plus"], function(i, v) {
                if(i === iPoints) {
                    eTd.addClass(v);
                } else if(eTd.hasClass(v)) {
                    eTd.removeClass(v);
                }
            });
        };

used by the respective column with

                render: {
                    display: function(data) {
                        return data;
                    }
                },
                createdCell: fnCreatedPointsCell

Findings:

  • Complicated, rendering can't change the color of the cell directly (needs to be re-created).
  • When modifying cell content with cell.data(), the class/color is therefore not changed. Another dirty fix (not explicitly mentioned here) is required to fix this without recreating the whole table's DOM.

Realizing this functionality with the new render algorithm is very easy and doesn't require any dirty fixes:

        var cellRender = function (data) {
            if(data === undefined) {
                return "-";
            }
            //td: jQuery element, points: determined level relevant for CSS (limited to 5 via Math.min() => points5plus class)
            var td = $(this).text(data), points = Math.min(parseInt(data), 5);
            //iterate over classes and add/remove them accordingly
            $.each(["points0", "points1", "points2", "points3", "points4", "points5plus"], function(i, v) {
                if(i === points) {
                    td.addClass(v);
                } else if(td.hasClass(v)) {
                    td.removeClass(v);
                }
            });
        };

used in the column (note the absence of the "createdCell" callback) with

                render: {
                    display: cellRender
                }

I tried to design this patch as much backwards compatible as possible, using

  • a new table-wide parameter "newCellRender" which has to be set "true" to enable the new rendering
  • code which makes the rending to fill the cell with whatever is returned by the display render function when this is not "undefined"

Additional things: I had to introduce a new (fifth) render type (besides "display", "sort", "filter" and "type"), called "adjust". This renderer is used for the automatic width calculation. Since the original implementation just removes markup tags from the return value of the display render function and uses the pure text for width approximation

s = _fnGetCellData( settings, i, colIdx, 'display' )+'';
s = s.replace( __re_html_remove, '' )

it should not be necessary to define the "adjust" renderer in most of the cases. Compatibility: If "newCellRender" is not "true", "adjust" is not used and the implementation falls back to the old behaviour, using "display" as its source.

milux avatar Oct 19 '14 12:10 milux

Hello devs, anybody who wants to review this? I would really love to see this (or a similar) feature implemented in DataTables! Regards, milux

milux avatar Mar 07 '15 15:03 milux

Sorry - I thought I had responded to this at the time - apparently not! I'm a little bogged down at the moment with a number of other things, so I can't review at the moment, but I will try to as soon as I can.

Allan

DataTables avatar Mar 07 '15 21:03 DataTables

Hello Allan,

are you still bogged down? The problem with change sets like that one is that it gets harder and harder to integrate as the code is changed continuously. It was already a lot of effort to nicely integrate my changes into the various files of DataTablesSrc. It makes me a bit sad to see that this request is now open for 1.5 years, because meanwhile you've changed a lot of things that make it hard to redo this changes properly. That said, I have no resources anymore to do such a integration in the recent source files again.

Kind regards

milux avatar Apr 21 '16 09:04 milux

Hi - yes I'm really sorry I haven't been able to pull this in yet. However, I've not forgotten about it - when I get the time to move on to the next major version of DataTables I hope to introduce this feature then and I'll be using ideas from here.

DataTables avatar Apr 21 '16 09:04 DataTables

And do you have any concrete plans when this is going to happen? ;)

milux avatar Apr 21 '16 09:04 milux

Nope. I truly wish I did - but the amount of time that support takes up is just phenomenal. It makes planning anything very difficult and thus I'm not giving out any more dates.

DataTables avatar Apr 21 '16 09:04 DataTables

Understood. ;) I will stick to my customized version and hope that it won't take another year until an official version arrives with support for this... One last question: Why do you want to wait for so long? I mean... I took extra efforts to make this virtually-100-percent-compatible, hoping for a quick merge in one of the minor versions. What made this stuff so special that you decided to wait until the arrival of a major release?

milux avatar Apr 21 '16 09:04 milux

The lack of unit tests in DataTables, which I am working to resolve now. I hope it won't be a year as well - I'll be very disappointed if it is.

DataTables avatar Apr 21 '16 09:04 DataTables

That point is true indeed... However, the unit tests were already broken and outdated when a posted this one and a half years ago. (I know because I tried to use them for testing my implementation again. ;)) Since then, you introduced a bunch of changes to DataTables without the safety net of unit tests, why so careful in this particular case? ;)

milux avatar Apr 21 '16 10:04 milux

Because I felt it had potentially larger impact.

I realise you are disappointed. So am I. I truly wish I had more time and I very much appreciate your taking the time to send the PR. I am working as much as is possible on all of this! It is proving to be difficult to balance everything.

I can say that I hope to include something along these lines in future. I can't say it will be 100% compatible, but more advanced rendering will be in the next major version.

DataTables avatar Apr 21 '16 10:04 DataTables

Well, regarding the impact you're potentially right :laughing: I wouldn't be so annoying if it wouldn't matter to me. Anyway, thanks for your patience, I don't want to steal more of your valuable time as you need it to fix your messed-up test suite, right? :wink: Feel free to contact me if you need any explanation or inspiration with the advanced rendering stuff when day X arrives.

milux avatar Apr 21 '16 10:04 milux