jQuery Grid Plugin - jqGridForum

Topic RSS

grid base code simplification

28/12/2009
18:45

Les

Member

Members

Forum Posts: 44

Member Since:
11/12/2009

Offline

I wanted to point out that the following code fragment can be simplified:

// current

$(ts).bind('mouseover',function(e) {

ptr = $(e.target).closest("tr.jqgrow");

if($(ptr).attr("class") !== "subgrid") {

$(ptr).addClass("ui-state-hover");

}

return false;

}).bind('mouseout',function(e) {

ptr = $(e.target).closest("tr.jqgrow");

$(ptr).removeClass("ui-state-hover");

return false;

});

// new

$(ts).bind('mouseover',function(e) {

ptr = $(e.target).closest("tr.jqgrow");

if(ptr.attr("class") !== "subgrid") {

ptr.addClass("ui-state-hover");

}

return false;

}).bind('mouseout',function(e) {

ptr = $(e.target).closest("tr.jqgrow");

ptr.removeClass("ui-state-hover");

return false;

});

There is no need to again do $(ptr)

28/12/2009
19:15

Les

Member

Members

Forum Posts: 44

Member Since:
11/12/2009

Offline

I noticed that row mouseover can be slow. I think this code can be further simplified:

$(ts).bind('mouseover',function(e) {

ptr = $(e.target).closest("tr.jqgrow");

if(!ptr.hasClass("subgrid")) {

ptr.addClass("ui-state-hover");

}

return false;

}).bind('mouseout',function(e) {

ptr.removeClass("ui-state-hover");

return false;

});

28/12/2009
19:31

tony

Sofia, Bulgaria

Moderator

Members

Moderators

Forum Posts: 7721

Member Since:
30/10/2007

Offline

Hello,

Thanks. Fixed.

Best Regards

Tony

For professional UI suites for Java Script and PHP visit us at our commercial products site - guriddo.net - by the very same guys that created jqGrid.

28/12/2009
21:08

markw65

Member

Members

Forum Posts: 179

Member Since:
30/07/2009

Offline

This last change doesnt look safe - ptr is set by code other than mouseover, so may be pointing at something other than the hover row. That could be fixed by using a different variable (eg hover_row). (Perhaps you did this - I dont see the change in github yet).

But, I think there is a bigger issue.

Every time the mouse moves from one sub-element of a row to another sub-element of a row, we get both a mouseout and a mouseover, and so the above code (even after the changes) removes and re-adds the class.

This will happen eg when moving from one td to another in the same row. But for grids whose cells have complex contents could be happening *a lot* within a single td.

I think the code as it stands is "correct" (Im assuming the mouseout is guaranteed to come before the mouseover, but havent checked that that is the case), but is certainly going to fire far too often.

It seems like you could check whether relatedTarget and target are contained in the same row, and only add/remove the class when the row actually changes.

That would somewhat negate the above change - because now you do two "closest" calls per mouseover/mouseout. But I suspect that most of the time is spent in adding/removing the class (because it causes the document to reflow).

Mark

28/12/2009
23:17

tony

Sofia, Bulgaria

Moderator

Members

Moderators

Forum Posts: 7721

Member Since:
30/10/2007

Offline

Mark,

Thank you. To be a honest I look at other side which IMHO is not the right one. You explain it very good, but the problem persisst and it can be seen very well in IE6. Also the change is rolled back until we found a better solution.

Thanks again

Tony

For professional UI suites for Java Script and PHP visit us at our commercial products site - guriddo.net - by the very same guys that created jqGrid.

28/12/2009
23:59

markw65

Member

Members

Forum Posts: 179

Member Since:
30/07/2009

Offline

The ideal solution would be to use the css pseudo class :hover in all browsers that support it (ie everything except ie6), and not have the event handlers at all for most browsers.

Unfortunately, you cant do that and stay compatible with themeroller, as far as I can tell (this seems to be a flaw with jquery-ui/themeroller).

Mark

19/01/2010
00:43

markw65

Member

Members

Forum Posts: 179

Member Since:
30/07/2009

Offline

While debugging another issue, I was using Fiddler to track network events for my page.

I noticed that in ie6 and ie7, if you wave the mouse in front of a grid, /lots/ of network events fire. And they're all fetching the same background image.

I googled around a bit, and found that ie6 has a known bug where setting the background image of an element will cause "flicker", because it fetches the image from the server (regardless of caching setting). The bug is supposedly fixed in ie7.

Well, the flicker may be fixed, but fetching the background image certainly isnt. Also, the well documented fix for ie6 'document.execCommand("BackgroundImageCache",false,true)' doesnt seem to stop the traffic with either ie6 or ie7.

Not sure what can be done, except to minimize the changes - so rather than changing the css on every mouseover and mouseout, we should verify that the row has actually changed. That would certainly cut down on the traffic...

Mark