Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#13181 closed defect (fixed)

[patch] Setting the DataGrid rowHeight attribute creates empty space between each page row.

Reported by: dmcintyre Owned by: Evan
Priority: high Milestone: 1.8.1
Component: DojoX Grid Version: 1.6.1
Keywords: Cc:
Blocked By: Blocking:

Description

Setting the rowHeight attribute in the DataGrid? creates a empty space between each page row in the grid. The space is caused because the top of the next page row is off by 1px time rowsPerPage. So if you have 25 rows per page you will have a 25px gap between pages. The measurePage method in _Scroller.js is adding the 1px per row. Why?

Browser: IE7 & 8 Win7 & Win2003

Steps to reproduce: Create a grid with the rowHeight and rowsPerPage settings specified. Have enough data so you will get multiple page rows in your grid.

Attachments (4)

_Scroller.js.patch (658 bytes) - added by Anton 7 years ago.
patch fix this problem
13181-evan.html (2.8 KB) - added by Evan 7 years ago.
Please run under dojox/grid/tests/
patch.patch (1.2 KB) - added by Anton 7 years ago.
second patch
13181-evan.patch (2.3 KB) - added by Evan 7 years ago.
A patch makes rowHeight works better, though there are still remained issues in IE7 and setting an arbitrary small rowHeight will break grid

Download all attachments as: .zip

Change History (18)

comment:1 Changed 8 years ago by Douglas Hays

Owner: set to evan

comment:2 Changed 8 years ago by Evan

Owner: changed from evan to Evan

Changed 7 years ago by Anton

Attachment: _Scroller.js.patch added

patch fix this problem

comment:3 Changed 7 years ago by Anton

I attach patch for fix problem with Grid, please check and commit.

comment:4 Changed 7 years ago by bill

Component: GeneralDojoX Grid
Summary: Setting the DataGrid rowHeight attribute creates empty space between each page row.[patch] Setting the DataGrid rowHeight attribute creates empty space between each page row.

comment:5 Changed 7 years ago by Sébastien Le Ray

I'm hitting the same bug. Could kzyp or evans take a look at this. I don't think the + 1 was added by mistake and can't believe noone uses DataGrid? with rowHeight.

Reproduced on firefox 3.5.19, 12, 14, Chromium 18

Using tundra theme & dojo 1.5.1 (a bit outdated but code is the same)

[Edit] Soria & Claro are not affected, because of the 1px border they add dojoxGridRow => The patch will fix defect for nihilo/tundra but introduce it for others. I guess some DOM querying cannot be avoided

Last edited 7 years ago by Sébastien Le Ray (previous) (diff)

comment:6 in reply to:  5 Changed 7 years ago by Evan

Replying to beuss:

I'm hitting the same bug. Could kzyp or evans take a look at this. I don't think the + 1 was added by mistake and can't believe noone uses DataGrid? with rowHeight.

Reproduced on firefox 3.5.19, 12, 14, Chromium 18

Using tundra theme & dojo 1.5.1 (a bit outdated but code is the same)

[Edit] Soria & Claro are not affected, because of the 1px border they add dojoxGridRow => The patch will fix defect for nihilo/tundra but introduce it for others. I guess some DOM querying cannot be avoided

Thanks beuss,

The rowHeight issue has been a known issue for dojox.grid for a long time, there hasn't been an ideal solution due to the machinery of the existing grid to absolutely position pages. The commonly met issue is page overlap - an explicit grid.scroller.rowHeightChanged() might help, but that brings significant rendering speed downgrade.

Just had a quick try, the _Scroller.js.patch seem not working for me("13181-evan.html") with obvious page overlaps

Any comments are welcomed.

Changed 7 years ago by Evan

Attachment: 13181-evan.html added

Please run under dojox/grid/tests/

Changed 7 years ago by Anton

Attachment: patch.patch added

second patch

comment:7 Changed 7 years ago by Anton

Hi All,

I checked this example and height rows more 30px because of the text is big. And I think, better add style for dojoxGridCell:

white-space: nowrap; overflow: hidden; text-overflow: ellipsis;

But all text in cell only one line if rowHeight is't null.

Please, check my last patch.

comment:8 Changed 7 years ago by Sébastien Le Ray

Yes you've to add this kind of style to nodes.

I've this in my style.css:

.dojoxGrid.fixedRowHeight .dojoxGridCell {
	overflow: hidden;
	white-space: nowrap;
	text-overflow: ellipsis;
}

The fixedRowHeight (or dojoxGridFixedRowHeight) might be automatically added to the grid if rowHeight is set.

Have a look at #15810 too for filtering issues.

comment:9 Changed 7 years ago by Sébastien Le Ray

Please note that anthony's patch may break under claro theme and any themes for which the +1 was intended.

Changed 7 years ago by Evan

Attachment: 13181-evan.patch added

A patch makes rowHeight works better, though there are still remained issues in IE7 and setting an arbitrary small rowHeight will break grid

comment:10 Changed 7 years ago by Evan

Thanks anthony/beuss,

Just post a patch that works better for me now(also with rowselector), the remained issues are:

  1. Not working in IE7
  2. Setting somehow small rowHeigth still results into page overlaps

Though not a perfect solution, the rowHeight is now working much better, we may get this in after 1.8 release freezing with a bit more tests, also add corresponding info in the grid doc page.

comment:11 Changed 7 years ago by Sébastien Le Ray

I don't think the "small rowHeight" issue is really one. I guess it appears when rowHeight is smaller than the line height which is a strange setting anyway.

Maybe you should create a meta-ticket and add #13181, #15810, #12296, #13983 and maybe others to track rowHeight issues in DataGrid?

comment:12 Changed 7 years ago by Evan

Resolution: fixed
Status: newclosed

In [29695]:

Fixes #13181 !strict, make dojox/grid work better with "grid.rowHeight" attr, still two remained limitation of grid.rowHeigth:

  • Not working in IE7
  • Setting somehow small rowHeigth still results into page overlaps

comment:13 Changed 7 years ago by Evan

In [29697]:

Refs #13181 !strict, also back port to 1.8.x branch

comment:14 Changed 7 years ago by Evan

Milestone: tbd1.8.1
Note: See TracTickets for help on using tickets.