Shift+left click selection breaks after scrolling a long way

[rhpijnacker]

Use case:

• Create an OnDemand+Selection grid
• Add a lot of rows ( > 1000)
• Select one of the first rows
• Scroll down (I used a mouse wheel scroll to scroll at least five pages)
• Shift+click to select the range of rows

The following exception happens at Selection.js:455 (dgrid v0.3.9)
Uncaught TypeError: Cannot read property 'element' of undefined

Some debugging shows that getting the row at line 453 fails because this.row(toRow) returns undefined. This seems to be caused by List._rowIdToObject no longer containing the requested row.
A workaround seems to be to set farOffRemoval to Infinity.

[kfranqueiro]

This is due to the fact that OnDemandList removes rows from the DOM after they are scrolled out of view, so by the time you shift+click, the originally-selected row (and any number of rows in between) is no longer rendered. See also #444.

The distance before which rows are removed is controlled by the farOffRemoval property exposed by OnDemandList. You could potentially raise this (or set it to Infinity) to get around this limitation, but keep in mind then that the DOM in the grid could get rather heavy depending on how many total rows you have (which is why the feature exists in the first place, to avoid that).

Unfortunately, trying to fix this would be pretty onerous, since Selection would need to do something to keep a contiguous list of IDs of rendered rows, just in case this happens (and there are likely more cases it won't than it will, so it seems like a lot of expense for an edge case). At the very least we should probably resolve the error, but there would still likely be unexpected behavior since the start of the range would be lost.

[rhpijnacker]

Hi Kenneth,

Thanks for your response.

I played around with farOffRemoval a bit, but this has some bad side effects when working with a list with a lot of rows.

Without knowing too much about how Selection is designed, it seems to me that lazy row rendering should be orthogonal to whether a row is selected which would make it possible to do large range selections combined with lazy row rendering.

As you said, it would be nice to at least make sure that no errors occur when this happens. E.g. selecting a row too far off would then just behave as if it were the first selection. Now all kind of funny stuff happens.

Thanks, Ronald

[rhpijnacker]

Hi Kenneth,

To work around getting an exeption over and over, I added an extra check to see whether 'toRow' is defined after the call to 'toRow = this.row(toRow)'. This would be around line 503 in the current master.

 if(toRow){
   if(!toRow.element){
     toRow = this.row(toRow);
   }
 }
 // Catch cases where toRow has been recycled
 if(toRow){

Could you include this extra check until a more permanent solution is available? Thanks!

Ronald

[rhpijnacker]

Hi Ken,

Thanks for integrating and improving the workaround.

Ronald