1.10.17 - Bootstrap container-fluid removed

cersos

Love it that the little arrows in the first row are gone, but I think there are some issues.

I now get horizontal scroll bars where I didn't on 16, and the scroller plugin does not seem to work.

Bootstrap 4.1

If you'd like some test cases let me know.

Steve

allan

Hi,

The regular Bootstrap 4 and Scroller Bootstrap 4 examples appear to be okay. Could you give me links to test cases showing the issues?

One point about 1.10.17 - you might see the examples are using 1.10.18 today. .17 and .18 are functionally identical - there was a packaging error for the .17 release on npm which required a new tag to resolve.

Allan

cersos

The only difference between these two fiddles is the version of DataTables. The JavaScript code is identical.

Maybe I'm doing something wrong, but the in the 18 version I get a horizontal scroll bar and scroller isn't working. I might could live without scroller, but having to go back to all my code and deal with the horizontal scroll bar will be a pain.

DataTables 1.10.16:
https://jsfiddle.net/43hzLe5m/3/

DataTables 1.10.18:
https://jsfiddle.net/5cwdg27s/

allan

The good news is that it isn't a software issue . I duffed up the version that the download builder is told to use - I typoed 1.5.0 (the new version) as 5.04.4 - not even a simple typo...

Correcting that allows it to work.

Thanks for the test case and letting me know about that!

Allan

cersos

Thanks for the awesome turnaround on this issue.
That fixed scroller issue, but the horizontal scroll bar is still there.

Latest fiddle: https://jsfiddle.net/ues5o9rm/

Steve

cersos

I am fairly certain I found the culprit, but I'd rather not fix it on my end.

In 1.10.16 the "table_wrapper" div element has a class of container-fluid.

In 1.10.18 it does not.

The child div's of table_wrapper have a class of row. The bootstrap row class has a -15 left and right margin, it is designed to be a child of an element with a class of container or container-fluid.

I hope that helps.

Steve

allan

Hi Steve,

That's as a result of this bug.

To add it back in if you want it, you can add:

$.extend( $.fn.DataTable.ext.classes, {
  sWrapper: "dataTables_wrapper container-fluid dt-bootstrap4",
} );

into your code to set the default.

Allan

cersos

Honestly, I consider how it is now a bug. It is incorrectly using the row class.

The bootstrap row class has a negative 15 left and right margin. It is specifically designed to only be used within one of the container classes.

Negative margins without something to counteract that will cause headaches as it will be outside the bounds of the parent object. This will be difficult for others to troubleshoot.

I wish I had seen that bug report, I would have argued against the "fix". You either need one of the container classes or don't use the row class.

I hate to correct this on the back-end because if it gets switched back then I'll have an issue.

Steve

cersos

Bootstrap documents the reason for the negative margins.

https://getbootstrap.com/docs/4.1/layout/grid/

cersos

@allan Do you consider this topic closed? That would be unfortunate, it was actually correct in 16. The comment in the bug report you referenced, "Any Bootstrap containers should be the responsibility of the developer" really isn't correct. You are already applying bootstrap classes within DataTables.

The work-around to put the container-fluid class back works.

I'll just have to keep an eye on it. If/when you move to bootstrap 5, the dt-bootstrap4 class will be invalid.

Steve

allan

No. Let me dig into this and if needed I'll roll back the change. I'll post back here with what I find.

Allan

cersos

Thanks for taking the time with this.

Your efforts are appreciated.

Steve

allan

I wasn't going to sleep without looking into it... .

The thinking with the change was that although DataTables itself does use Bootstrap grids for the layout, it doesn't need to use a container - that is something that I consider to be at a slightly higher level - usually a wrapper at the top level of the document.

This is DataTables with Bootstrap 4 and a fluid container: http://live.datatables.net/cifapuhi/1/edit

This is it without: http://live.datatables.net/wovifotu/1/edit

The fluid-container is at the top level. This lets the developer using DataTables decide if they want the 15px margin for their container or not. The negative margin is still used by the row/col-* combination.

Was my decision the right one? Honestly I don't know. It seems correct to me since the developer using DataTables now has the choice to use a container or not - whereas it was forced before. But the input of a Bootstrap export would be very welcome.

Allan

tangerine

But the input of a Bootstrap export would be very welcome.

...Bootstrap expert... ?

I just can't help it....

Joyrex

@cersos - Honestly, when BS5 comes out Allan will need to make an entirely new theme (like he did with BS3), so I don't think that's a concern (for right now).

Add to how long it was between BS3 and BS4, we'll probably be on DataTables 2.x by then!

I got bit by this "bug" when upgrading from 1.10.16 to 1.10.18 and my tables suddenly shifted to the left!

cersos

I'm certainly no Bootstrap expert, but my feeling is you're kind of 1/2 way using it now.

Since you are using the row class on the divs contained by the dataTable_wrapper element they are greater than 100% of the width, 30px wider to be exact. That's why you get the horizontal scroll bar.

if you put the container-fluid class on the datatable_wrapper element that is negated. If the user already has a container around it, then they will end up with a nested container and lose 30px.

It's not that the developer really has a choice, the DataTable has to be wrapped in a container[-fluid] or you'll get the scroll bar.

If I'm using Bootstrap, and I don't know to wrap my DataTable in a container[-fluid] then I'll just have to figure out why I have a horizontal scroll bar.

I guess it's just something to be aware of. Now I know, maybe it will help others too.

Steve

cersos

Another option would be to add the class m-0 to each direct child div of the datatable_wrapper element.

https://jsfiddle.net/nd3c8fek/

It just troubles me that by default, unless you do something about it, the DataTable has an undesirable horizontal scroll bar.

It's kind of a catch-22. If the DataTable is already wrapped by a container, the user loses 30px. If there isn't one, they get a scroll bar and possible confusion.

Hard to know which way to go.

Steve

allan

Yes, this isn't an easy one unfortunately. Are there many Bootstrap sites which don't use a Bootstrap container (I guess your own is an example!)?

I'm working on a new debugger for DataTables which will run automated tests, and this sounds like an ideal use case for that - to check there there is a container, and if not then show a warning.

Not ideal and as you say the out of the box without a contianer isn't great. But I guess that's true in other Bootstrap sites which use a grid but no container.

Allan

Joyrex

@allan - My two cents (as someone who uses Bootstrap extensively):

One of the things I had to do when moving to BS4 was to remove the <div class="col"> around all my DataTables that were in a <div class="row">, since the .container-fluid you were injecting into the DataTables wrapper DIV was causing extra padding.

In Bootstrap 4.1's docs, they specifically say "In a grid layout, content must be placed within columns and only columns may be immediate children of rows" (https://getbootstrap.com/docs/4.1/layout/grid/)

So, I think you were right in removing the .container-fluid from the DataTables wrapper DIV, which allows developers to position it as they like - whether that is wrapping it in a .container .container-fluid DIV, or in a .col-* DIV. Much easier to add something than to remove it (especially if it is generated by another script, in this case the DataTables JS).

allan

Thanks for the feedback - much appreciated.

If anyone else has any - it would be very welcome as well.

Allan

p.s. I've updated the title of the thread to reflect the issue at hand.