FixedColumns: scroller DOM setup hardcoded classnames
FixedColumns: scroller DOM setup hardcoded classnames
dbr
Posts: 7Questions: 0Answers: 0
The scroller DOM are hardcoded class names, they should be dynamic.
If for some reason the class names are modified in the DataTable (via oClasses) the fixedColumns fail to select the correct dom elements to work correctly.
@file FixedColumns.js @version 2.0.3 [line: 389]
[code]
/* Set up the DOM as we need it and cache nodes */
this.dom.grid.dt = $(this.s.dt.nTable).parents('div.dataTables_scroll')[0];
this.dom.scroller = $('div.dataTables_scrollBody', this.dom.grid.dt )[0];
[/code]
A fix I guess would be:
[code]
/* Set up the DOM as we need it and cache nodes */
this.dom.grid.dt = $(this.s.dt.nTable).parents('div.'+this.s.dt.oClasses.sScrollWrapper)[0];
this.dom.scroller = $('div.'+this.s.dt.oClasses.sScrollBody, this.dom.grid.dt )[0];
[/code]
If for some reason the class names are modified in the DataTable (via oClasses) the fixedColumns fail to select the correct dom elements to work correctly.
@file FixedColumns.js @version 2.0.3 [line: 389]
[code]
/* Set up the DOM as we need it and cache nodes */
this.dom.grid.dt = $(this.s.dt.nTable).parents('div.dataTables_scroll')[0];
this.dom.scroller = $('div.dataTables_scrollBody', this.dom.grid.dt )[0];
[/code]
A fix I guess would be:
[code]
/* Set up the DOM as we need it and cache nodes */
this.dom.grid.dt = $(this.s.dt.nTable).parents('div.'+this.s.dt.oClasses.sScrollWrapper)[0];
this.dom.scroller = $('div.'+this.s.dt.oClasses.sScrollBody, this.dom.grid.dt )[0];
[/code]
This discussion has been closed.
Replies
I've opened an issue on this here: https://github.com/DataTables/Scroller/issues/12 . I've got a bit of work to do on Scroller tomorrow actually, so I'll take a look at this then.
Allan
Also the cloning on the FixedColumns doesnt seem to be deep enough? I have a callback on fnCreatedRow, that adds some extra script functionality on some cells. This functionality is being lost when FixedColumns clones the table parts. I will post my findings as soon as I have something to show.
Ideally yes - however, it adds a reasonable amount of complexity which is why it currently does not. This is a feature that I would like to add in future when I have the time available to do so.
> Also the cloning on the FixedColumns doesnt seem to be deep enough?
It does an HTML clone with jQuery events and data (using $().clone). Is the functionality being added after the clone is done?
Allan
Just a quick look: it does seem to be about deep cloning (http://api.jquery.com/clone/ ), it seems you use .clone(true) for copying data and events but it should be .clone(true,true) to capture anything put inside the cells as well. I haven't tried it yet to see if this is the issue.. Lunch! ;-)
Throughout all the scripts I have looked at, I have notice dt.s.aoColumns is used to track the current state of the columns. So to me it seems important that if this variable is key for keeping track of the state of the rendered table, that it should be kept up-to-date.
I understand the complexity is a bit boggling (DataTables is quite amazing), but I am sure it can be tightly controlled. I need this to work, so I will have a peck at it.
Yes the bVisible variable of the column configuration object is used to track what the column's visibility state is and that is always correct (it is a read only semi-private variable). So yes, FixedColumns would need to be updated to take account of that variable. Thinking about it, with my recent changes in FixedColumns 2.5, it probably won't actually be too difficult to support.
Allan
But the general gist is that when you clone the columns you have take in account the visibility state. So if you set 4 right columns to be fixed but only 2 of them are visible, you just clone the ones that are visible.
So in the looping of "_fnCloneRight","_fnCloneLeft" you need to skip the invisible columns.
(FixedColumns 2.5)
[code]
for ( i=this.s.iTableColumns-this.s.iRightColumns ; i
When you are changing the visibility of columns and you have a scroller involved and the xScrollInner is not set, the width of the table needs to be removed.
Currently jquery.dataTables.js 1.10-dev (line:3537)
[code]
// When all else fails
tableStyle.width = _fnStringToCss( sanityWidth );
[/code]
There is an assumption made here "When all else fails" and "Not possible to take account of it" that the current width of the table is what it currently is. When column visibility is at play this may not be so because the width of the entire table may be changing due to columns being added or removed.
It may be safer to unset the width at this point and let the table size itself, rather than assume the current width was the intended width.
[code]
// When all else fails
tableStyle.width = "";
[/code]
Or maybe we need a case check of the visibilty state of the columns first and set the table width and accordingly switch between unsetting the width or use the sanityWidth. Or maybe we need a variable to track the change in column removal/addition.
I know that this maybe a bit complex of a situation to imagine, but I am looking quite deep at this point at this particular scenario.
BTW this is the case only when xScrollInner is not set!! When xScrollInner is set you are doing the right thing and observing the setting.
It is funny when I read this code and the names of the variables "iSanityWidth" and comments "When all else fails" it seems like you were trying to maintain your sanity to get through it. Actually it is great that you comment like this because it gives the reader the sense of what you were going through when writing it, and the possiblity to question the sanity! It is not easy to get your head around some of this stuff. lol :-)
BTW there is an inherent conceptual issue with FixedColumns in general that I have noticed because of this.
Fixed columns are applied after the datatable is drawn and not at the same time.
So on that first render if you are using a fnCreatedRow callback you have no idea whether or not fixed columns are intended to be applied or not because it hasn't happend yet. Ultimately the ideal situation is that FixedColumns should be rendered simultaneously with the DataTable and not generated as a patch after the DataTable has been drawn.
I also think doing so would also be *MUCH* more efficient, because you would be creating the fixed columns in the same iteration as creating the table. At the moment you are iterating over the dataset again; cloning rather than directly "re-using" elements at creation time and recalculating widths of known dimensions creating unnecessary extra effort for the browser.
To fix this however would be quite an undertaking as you would need some low level hooks into the datatable draw process, and a complete rewrite of FixedColumns... version 3? The nice thing about doing this is you wouldn't need to call "new FixedColumns(table,config)" after initialising the datatable, it would be nicely integrated together with the configuration of the FixedColumns applied with the configuration of the DataTable. However it would be bound to a particular version of DataTables that has the required hooks, so no backwards compatibility.
Just a quick note to say I haven't forgotten about this! I will be coming back to it - just a little overwhelmed with everything else to do at the moment :-)
Allan
I've just committed this change into FixedColumns to support hidden columns: https://github.com/DataTables/FixedColumns/commit/d29e19e . I've also added a new example showing that ability to update dynamically with ColVis. I'll be releasing this tomorrow as FixedColumns 2.5.0 :-).
Regarding the deep integration - I think you are right - there are performance enhancements to be had integrating deeply into DataTables. Certainly something to look at in the future!
Regards,
Allan