Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#8807 closed defect (fixed)

[PATCH][CCLA] Sort arrow issues on IE7 and Firefox

Reported by: Jared Jurkiewicz Owned by: Bryan Forbes
Priority: high Milestone: 1.3
Component: DojoX Grid Version: 1.2.3
Keywords: Cc:
Blocked By: Blocking:

Description

This problem is seen in IE7 using Dojo 1.2.3.

When a DataGrid? column is sorted, a sort arrow icon appears on the right side of the column. The problem I am seeing is that IE appears to be using the same behavior when the mouse is hovering over the sort arrow as it does when hovering over the left side of the column. See the attached test case grid_sort_arrow_problem.html.

If you sort the first column and hover over the sort arrow icon you see the expected behavior. That is, the mouse does not change. If you sort the second column and hover over the sort arrow icon the mouse changes to the not-allowed cursor icon since the left side of the column is not resizable. If you sort the third column and hover over the sort arrow icon the mouse changes to the resize cursor and allows you to resize the column, since the left side of the column is resizable.

Testcase coming.

Attachments (3)

grid_sort_arrow_problem.html (3.9 KB) - added by Jared Jurkiewicz 10 years ago.
Testcase for issue.
ie_arrow_oddity.patch (605 bytes) - added by Jared Jurkiewicz 10 years ago.
Simple CSS patch to get the right 'hover' icon on the arrow.
ie_resize_completefix.patch (2.8 KB) - added by Jared Jurkiewicz 10 years ago.
Patch for resize issue (complete, I hope!)

Download all attachments as: .zip

Change History (12)

Changed 10 years ago by Jared Jurkiewicz

Testcase for issue.

comment:1 Changed 10 years ago by Jared Jurkiewicz

Milestone: tbd1.3

comment:2 Changed 10 years ago by Jared Jurkiewicz

Partial patch. I figured out how to get rid of the 'NOT ALLOWED' and whatnot icons from the header on IE when you hover over the arrow or character. The patch is a simple hover check in the CSS. Patch will be attached.

It doesn't fix the odd 'being able to resize by pulling the arrow' problem, but at least you get the right icons on IE.

Changed 10 years ago by Jared Jurkiewicz

Attachment: ie_arrow_oddity.patch added

Simple CSS patch to get the right 'hover' icon on the arrow.

comment:3 Changed 10 years ago by Jared Jurkiewicz

The resize issue isn't IE specific. I can reproduce it on FF. The hover-over showing the wrong pointer was IE specific and I believe my CSS patch resolves it.

If you're okay with the patch, I can check it in.

comment:4 Changed 10 years ago by Jared Jurkiewicz

Summary: Sort arrow issues on IE7 and Firefox[PATCH][CCLA] Sort arrow issues on IE7 and Firefox

comment:5 Changed 10 years ago by Jared Jurkiewicz

Okay, I think I fixed this (and found another bug that was helping cause it)

The first bug was in _Builder.js:

Function:

canResize:

The return of that function was:

return !cell.noresize && !cell.canResize();

That looks wrong, as you're returning if the cell itself is resiable, then I whould think the logic would be:

return !cell.noresize && cell.canResize();

That lead me to discovering the second bug:

cells/_base.js

function:

canResize:

canResize: function(){

var uw = this.unitWidth; return uw && (uw == 'auto');

},

Again, that return looks wrong. Its inverted from what I believe it should be. 'auto' is an autosizing cell and by that definition uses all remaining space. So should not the check be:

canResize: function(){

var uw = this.unitWidth; return uw && (uw != 'auto');

},

?

If I combine the fix in _Builder with the fix in cells/_base.js, I get closer to working, but not right. There is still an IE issue.

After copious amounts of debugging I discovered that IE is doing somehting *amazingly* stupid here. The event passed to the domousemove and domousedown functions of grid have a 'cellX' value. This is the X position in the cell. Its used to determine if the position is currently in the moveable boundry on the cell edges. Well ... turns out that in IE, when you hover over the icon or text character, IE returns an X position on the LEFT even though the node is floated right ... so the check ends up looking at the wrong adjacent cells to determine movability. Doh.

So, I fixed this by adding in an IE check to the sizer boundry tests and such that if the event that came in was an over or down on the sort ICON nodes, then they are simply ignored, since their positions are not correct from an X calculation perspective. This doesn't seem to have any side effects other than making it so an icon on a far right of the node acts like the left resize block. Ugh.

Patch coming shortly (Also includes my CSS patch for hover-over icon values)

Changed 10 years ago by Jared Jurkiewicz

Attachment: ie_resize_completefix.patch added

Patch for resize issue (complete, I hope!)

comment:6 Changed 10 years ago by Jared Jurkiewicz

I did also grep for all uses of 'canResize' in the codebase and my fix of the inverted return value for it doesn't affect any of them. All usage of it call the canResize defined in _Builder, which in turn calls the cell canResize() function. so now that canResize of cell returns right and canResize of builder checks its return correctly, it should all still be good.

It would be great to get a review of this. A product at my employer is already running with the fix and has reported it's working great for them. If you're happy with it, I'll commit it.

comment:7 Changed 10 years ago by Adam Peller

Priority: normalhigh

comment:8 Changed 10 years ago by Jared Jurkiewicz

Resolution: fixed
Status: newclosed

Fixed in changest: 17007

Note: See TracTickets for help on using tickets.