DataTables / DataTablesSrc

DataTables source repository
https://datatables.net
MIT License
627 stars 423 forks source link

Column width behaviour on 2 #283

Open niphlod opened 3 months ago

niphlod commented 3 months ago

It seems that width calculation on version 2 doesn't take into account cells that change content (and may wildly change the width).

Documentation on [columns.adjust()](https://datatables.net/reference/api/columns.adjust()) seems to permit this

Call it when the table becomes visible if hidden when initialised (for example in a tab) or when the data changes significantly.

I'm using something that could be definitely defined as a "trick" (but falls on the "when the data changes significally, IMHO) : rather than playing with orthogonal data, if I have a cell containing a link, I'd like "Copy" and "Export" buttons to have as the content the href of a link rather than the text representation of the link itself.

Till 1.13.x this worked:

At this point, the user sees <a href="somethingverylong" data-name="shortrepr">shortrepr</a>, the cell is "short" (on 1.13.x), but on "copy" and "export" datatables exports https://somethingverylong (because it strips html tags but fetches the original content of the cell)

On 2.x, this still works, but the cell is "long": it seems to gather the length from the original content rather than the actual cell content.

I managed to reproduce a minimal case here (click on the "name" column to trigger a "draw") Repro with 2.x https://live.datatables.net/laliruxi/1 Repro with 1.13.x https://live.datatables.net/yiroxoyu/1

AllanJard commented 3 months ago

Interesting one - thank you for posting this. I'm actually okay with the v2 behaviour there (and mildly surprised by the v1 behaviour, which I think is bug!).

The key thing here is that when the DataTable is initialised there is a long string in it, and it correctly adjusts the column widths to show that. But then the DOM is updated without telling DataTables (important point - it doesn't listen for changes in the DOM), so it can't adjust. This is mentioned in the FAQs - although not directly about column widths.

That is works in 1.x is testament to the fact that I've improved the column width handling in v2 funnily enough. If you inspect the header cell in v1 (without resizing the window, so you'd need the inspector already open) you'll see that there is a large assigned width, but that is mostly being ignored! I'm fairly sure there is a setTimeout for column widths in v1 somewhere as well which might be effecting it, although possibly that was for Ajax - can't recall!

Either way, I'm comfortable that the v2 behaviour is actually okay.

niphlod commented 3 months ago

I'm inclined towards thinking that this "trick" needs to be properly "deprecated" on my side, since now export has orthogonal options and I can define directly in the renderer "display" vs "export" differently.

At this point though I don't see the "or when the data changes significantly" bit in the documentation of columns.adjust()

Call it when the table becomes visible if hidden when initialised (for example in a tab) or when the data changes significantly.

Given that one may not have control on the renderer and something changes the DOM without telling Datatables, isn't "columns.adjust()" supposed to bridge "the gap", telling Datatables "hey, I'm sure something has changed, please (re)adjust based on the newly changed data " ?

AllanJard commented 3 months ago

Ah yes, I think I see why v1 is resizing columns now. v2 caches the strings that it uses for the sizing table and needs a data invalidation (e.g. calling invalidate() or a row/cell, or updating a row/cell) in order to trigger the recalculation. It was added as a performance optimization - but this is one of those cases where an an optimization causes unexpected effects!

niphlod commented 3 months ago

I'm perfectly fine with the performance optimization, but I guess the docs should reflect it, or for all intent and purposes "when the data changes significantly" columns.adjust() won't adjust ... anything !? For 2.x right now is more or less nearer to a columns.adjustAfterVisible() (i.e. it's useful just after the table is shown, e.g. in a tab).

columns.adjust() missing the "hey datatables, your content changed, please adjust accordingly" and relying on "call invalidate() on cell/row" moves the "your content changed" to invalidate, so it remains a columns.adjust() => please adjust... but it DOESN'T work as advertised, or, at least, it's counter-intuitive.

On top of that, if invalidate() should be called on each changed row/cell, before calling columns.adjust(), as long as it's clearly documented it could be a way to achieve "hey datatables, your content changed, please adjust accordingly". I do think that, at this point, either a new API should need to be surfaced or something like columns.adjust(invalidate : true) or columns.adjust(changed: true) or columns.adjust(newcontent: true) should be provided. Once "the data changed significantly" the majority of the usecases would need to invalidate() a large portion of the table, so I don't see users happily looping on each row calling invalidate() + calling columns.adjust() at the end, for the purpose of letting the table resize according to the "refreshed content".

AllanJard commented 3 months ago

The expectation, as noted in the FAQs is that any change to the data should be through the DataTables API, and not through the DOM. Column widths aren't the only thing that would have problems with the data being changed without using the API.

niphlod commented 3 months ago

okay, then maybe just highlighting the FAQ in the context of adjust() may avoid users falling into the same culprit I fell into regarding this, 'cause bug or not, it's a definitive change in behaviour.

AllanJard commented 3 months ago

Agreed! I'll add a note in. Thanks for flagging this up.

niphlod commented 3 months ago

np, have a nice day