Opened 11 years ago

Closed 6 years ago

#7273 closed defect (patchwelcome)

After sort, scroller reset to (0,0)

Reported by: Joseph Scheuhammer Owned by: Bryan Forbes
Priority: high Milestone: 2.0
Component: DojoX Grid Version: 1.6.0
Keywords: Cc: Becky Gibson, davidb
Blocked By: Blocking:

Description

Invoking sort on the grid by, say, clicking a column header causes parts of the DOM to be replaced. A side effect is that the current position of the scroller is lost, and it is reset to (0,0).

To reproduce:

  1. load http://archive.dojotoolkit.org/nightly/dojotoolkit/dojox/grid/tests/test_keyboard.html
  2. move the grid's horizontal scroll bar such that column 15 is in view.
  3. click column 15 header to sort the grid.
  4. after the sort is finished, the horizontal position of the grid is reset back to column 1.

The grid scroll position should not change.

Attachments (3)

grid.html (1018 bytes) - added by Jared Jurkiewicz 10 years ago.
HTML file showing header offset problem on IE
states.json (3.7 KB) - added by Jared Jurkiewicz 10 years ago.
data file for grid.html
grid.patch (558 bytes) - added by Jared Jurkiewicz 10 years ago.
Patch to fix the IE header align issue. (Doesn't fix the snap left, but it does fix IE so they stay aligned like FF does)

Download all attachments as: .zip

Change History (24)

comment:1 Changed 11 years ago by Bryan Forbes

Milestone: tbd1.2

comment:2 Changed 11 years ago by Nathan Toone

Owner: changed from Bryan Forbes to Nathan Toone
Status: newassigned

just a clarification - horizontal scroll should not be reset...what about vertical scroll? Since you are sorting, there is no concept of "where you were" - that is,

comment:3 Changed 11 years ago by Nathan Toone

Resolution: fixed
Status: assignedclosed

(In [14868]) Fixes #7273 Fixes #7186 - restore scroll position when sorting or resizing columns !strict

comment:4 Changed 10 years ago by Jared Jurkiewicz

Resolution: fixed
Status: closedreopened

This problem still occurs on 1.3

comment:5 Changed 10 years ago by Jared Jurkiewicz

I looked at the changeset. It only restores vertical position, not both vertical and horizontal...

comment:6 Changed 10 years ago by Jared Jurkiewicz

Also, there is another problem specific to IE. When it snaps back to the left, the header doesn't follow and alignment is off. I'll attach two files that readily show this. I tracked down (Along with Ben Schell) how to at least fix the alignment issue on IE. I'll attach a patch as well.

In reality, it ought to be keeping horizontal position, but at min it needs to at least keep the headers aligned if it snaps back.

Changed 10 years ago by Jared Jurkiewicz

Attachment: grid.html added

HTML file showing header offset problem on IE

Changed 10 years ago by Jared Jurkiewicz

Attachment: states.json added

data file for grid.html

comment:7 Changed 10 years ago by Jared Jurkiewicz

In IE, load grid.html, then scroll all the way right (Do not change top/bottom scroll). Click sort. The table body snaps back to all the way left ... but the header does not. It won't correct until you move a scroller.

I should also note that if you have already scrolled down and then right, it doesn't occur. It only occirs when scrollTop = 0;

We tracked this down to on IE the scroll event doesn't seem to fire for scrollTop = 0. We found a simple workaround for it. I'll attach a patch next.

Changed 10 years ago by Jared Jurkiewicz

Attachment: grid.patch added

Patch to fix the IE header align issue. (Doesn't fix the snap left, but it does fix IE so they stay aligned like FF does)

comment:8 Changed 10 years ago by Bryan Forbes

Milestone: 1.21.3
Owner: changed from Nathan Toone to Bryan Forbes
Priority: normalhigh
Status: reopenednew
Summary: Grid: after sort, scroller reset to (0,0)[patch][ccla] Grid: after sort, scroller reset to (0,0)

comment:9 Changed 10 years ago by Bryan Forbes

Resolution: fixed
Status: newclosed

(In [16562]) * Applying patch from Jared (fixes #7273 !strict).

comment:10 Changed 10 years ago by Bryan Forbes

Milestone: 1.3future
Resolution: fixed
Status: closedreopened
Summary: [patch][ccla] Grid: after sort, scroller reset to (0,0)After sort, scroller reset to (0,0)

I checked in the patch to fix the header align issue in IE. I'm moving this to future because this is going to be an invasive fix.

comment:11 Changed 8 years ago by evan

Resolution: worksforme
Status: reopenedclosed

Had a thorough testing on all browsers by using the mentioned test case, not seeing this issue anymore in 1.6, so closing this as worksforme.

comment:12 Changed 8 years ago by Bryan Forbes

Milestone: future1.6.1
Resolution: worksforme
Status: closedreopened
Version: 1.1.11.6.0

I just tested this again in Firefox 3.6, IE6, IE7, IE8, Chrome 10.0.648.127, Chrome 9.0.597.107, and Safari 5.0.3 (6533.19.4). Safari and Chrome 9 are the only ones that returns to the top. This needs further investigation as to why Safari and Chrome 9 don't return to the previous scroll position.

comment:13 Changed 7 years ago by bill

Milestone: 1.6.21.8

Moving apparently forgotten ticket to 1.8.

comment:14 Changed 7 years ago by Colin Snover

Priority: highblocker

Bulk update of open ticket priorities.

comment:15 Changed 7 years ago by Bryan Forbes

According to #12224, this behavior happens because of the first line of dojox.grid._View#render. I need to investigate this further.

comment:16 Changed 7 years ago by Bryan Forbes

#12224 is a duplicate of this ticket.

comment:17 Changed 7 years ago by cjolif

this ticket is a 1.8 blocker because it was moved automatically from high to blocker at some point. See:

"Bulk update of open ticket priorities."

that's why I think we can downgrade it to not block the release.

comment:18 Changed 7 years ago by cjolif

Priority: blockerhigh

comment:19 Changed 7 years ago by Colin Snover

Milestone: 1.82.0

1.8 has been tagged; moving all outstanding tickets to next major release milestone.

comment:20 Changed 6 years ago by bill

DojoX Grid and EnhancedGrid are deprecated in favor of dgrid and gridx.

You should upgrade your code to use one of those two grids.

We will consider patches to the old DojoX Grid code though.

comment:21 Changed 6 years ago by bill

Resolution: patchwelcome
Status: reopenedclosed
Note: See TracTickets for help on using tickets.