ColVis menu positioning does not seem body offset aware?

Ashbjorn

Site that exhibits the behavior as described below: http://192.241.236.31/themes/preview/smartadmin/1.5/ajaxversion/#ajax/datatables.html

Steps to reproduce the behavior:

1. Navigate to the URL above
2. Click on the cogwheel on the far right of the side
3. On the sliding menu that appears select the option: "inside .container"
4. With the body content centered scroll down to the 3rd grid that has the ColVis enabled
5. Click on the "Show/hide columns" button
6. Verify that the collection menu is offset to right

I did not really notice this particular behavior until the site was set to 'container' mode, this applies the rather simplistic 'margin: 0 auto' style to the body element to achieve centered/fluent positioning of the content/body.

However when this is the case, and thus the body gains a positive offset value, clicking the ColVis button causes the collection menu to be offset to the right for the amount equal to the body offset value. Notice that the offset value is a separate value then when referring to the "left" CSS style value, which actually has the correct value (or so it seems) and is used throughout the function.

After taking a look at the source inside the '_fnCollectionShow' function I saw that the offset of the body is seemingly never taken into account. Of course I am quite oblivious to the code shown in that function, so my "fix" is most very likely not the way to "patch" this function properly, hopefully someone else has better insights to this. But my small adjustment did however provide me with the necessary correction to keep the menu displayed correctly in this situation without breaking the "default" behavior.

My initial patch was adding these lines near the end of the function definition:

                var bodyOffset = $(document.body).position().left;
                if (bodyOffset > 0) {
                    var currentOffset = $(nHidden).offset().left;

                    $(nHidden).offset({ left: currentOffset - bodyOffset });
                }

This repositions the collection menu to where I would expect it to show. In addition I also noticed that the variables iDivX and iLeft contain the same value as what is calculated above, so perhaps it would be better to use those values instead, but I am not sure if they should serve that purpose.

I hope my assumptions here are correct and that the "fix" as described is an actual fix or at least recognized as such when dealing with a body that is placed inside a container by using the commonly used centering tricks for CSS.

Note: The DataTables .net site does not seem to posses this "behavior/bug" since the centering of the content is achieved using a different technique/approach. Perhaps this could have played a part in determining whether the positioning had any quirks during the development of this plugin. But that is of course also a crude assumption and not meant to be disrespectful.

As an added bit of information, the ColReorder plugin demonstrates the same behavior when you start dragging a column and the "indicator" appears to where the column can be repositioned in between the column markers. So my guess would be that a similar patch could correct that as well, but I have been unable to locate the exact spot where this might be, so I have not bothered with it.

Regardless of what is occurring I do wish to thank everyone who has made DataTables and these Extensions possible, since they are a joy to use and always leaves people impressed when demonstrating their prowess.

allan

Hi,

Thanks for posting this. Setting the document's body width to anything other than 100% is not something that I've come across before and thus isn't something that my code is accounting for. There will likely need to be a number of updates across various packages to do so - thanks for pointing this out.

I'm not going to commit an immediate fix (due to time constraints), but have added this to my to do list.

Regards,
Allan

Ashbjorn

I'm not sure whether to post this, but while waiting for the eventual fix, I was able to find out where to make changes in the ColReorder script to "fix" this issue for my situation; meaning that it fixed it for me and that the code changes are limited to my knowledge and do not reflect the actual changes made by the author.

I made the following changes:

"_fnMouseMove": function (e) {
    var that = this;
    e.pageX = e.pageX - $(document.body).offset().left;

Here I "correct" the offset of the header drag element so it moves at my mouse cursor again.

Then further down when determining the position of the pointer element:

            "_fnRegions": function () {
                var aoColumns = this.s.dt.aoColumns;
                var bodyOffset = $(document.body).offset();

Here I start by capturing the extra value required to offset the behavior. Then I add this value as a subtraction on the following lines:

this.s.aoTargets.push({
    "x": $(this.s.dt.nTable).offset().left - bodyOffset.left,
    "to": 0
});

and here

if (aoColumns[i].bVisible) {
    this.s.aoTargets.push({
        "x": ($(aoColumns[i].nTh).offset().left - bodyOffset.left) + $(aoColumns[i].nTh).outerWidth(),
        "to": iToPoint
    });
}

With the result that the plugin works great with both an expected full width body, but is also capable of dealing with a centered fluent positioned body.

Sorry if this caused any trouble =]

allan

Hi,

That's fantastic - thanks for posting your findings. I've got a massive support backlog at the moment, so I don't anticipate being able to do this myself in the near future. Hopefully it won't be too far away, but not this week certainly! :-)

Allan

Ashbjorn

@allan I have updated my work to use the newest DataTables and the Buttons extension, it was a satisfying Sunday to get everything working again, however, unfortunately the behavior described was not addressed. Plannings and priorities aside is this "fix" going to be part of the package at some point, or should I just take note to update the script after each update going forward?

It's perfectly understandable that other issues are taking priority, just wanted to get an update on the matter, I will be making the changes in the newest libraries to prevent the client from getting upset about an old fixed bug creeping back in; because those things never happen right ;-)

Thanks for the wonderful effort with this new update and the much needed Buttons extension library, it's working very well so far!

Ashbjorn

@allan, I made the following changes, please let me know if they are the proper way to "fix" this behavior and something you would've written:

dataTables.buttons.js

on line 1067 change

            var hostOffset = host.offset();

to

            var hostOffset = host.offset();
            var bodyOffset = $(document.body).offset();

on line 1079 change

                    left: hostOffset.left

to

                    left: hostOffset.left - bodyOffset.left

This adjustment causes the 'colvis' button to be placed correctly when this perhaps deprecated way of centering the page content is used.

The adjustments for dataTables.colReorder.js are still the same as described above; I have yet to figure out a better way to do this, or to use some internal/plugin variable that might grant access to the body in a more natural way, so I would still consider these changes to be a hack rather than a patch.

Hope this helps,

allan

Yes, they look like the correct changes to make. I haven't tried it yet, but I don't immediately see anything wrong with that.

Why a hack? If the body is offset (which I'm not sure why that would be done rather than just using a container element?) then that needs to be taken account of.

Allan

Ashbjorn

I would call it a hack as long as it's only in my local code, a patch when it would be the proper changes to update the existing packages going forward ;-)

But of course that lies with you and you might come up with a more suitable way to address this, I could submit a pull-request if it helps with your planning.

Even so I can understand considering the perhaps awkward solution to use the body instead of a container that this situation is simply not common enough to be part of the code base (although the template in question is quite popular).

In any case, thanks for the reply, for the time being I will keep patching the new releases :-)