JLynch7 / SlickGrid

A lightning fast JavaScript grid/spreadsheet
https://github.com/jlynch7/SlickGrid/wiki
MIT License
89 stars 76 forks source link

cleanupRows never removes rows from cache if hasFrozenRows is false #54

Open ldmberman opened 11 years ago

ldmberman commented 11 years ago

this leads to some bugs: rows removed after vertical scrolling appear only after horizontal scrolling because we have additional cleaning in render -

if (lastRenderedScrollLeft != scrollLeft) {
  cleanUpAndRenderCells(rendered);
}

the fix is probably to replace && with || in cleanupRows:

if ( ( ( i = parseInt(i, 10)) !== activeRow )
    && ( i < rangeToKeep.top || i > rangeToKeep.bottom )
    || ( removeFrozenRow )
    ) {
   removeRowFromCache(i);
}
marbetschar commented 9 years ago

Any news on that one? As I'm currently experiencing scrolling issues too and need that to be fixed. Unfortunately I do not have any clue where to begin - if so I would fix it by myself :(

scrolling-error

marbetschar commented 9 years ago

Seems like https://github.com/JLynch7/SlickGrid/pull/77 has partly fixed it. But it does not work in 100% of all cases. Any more ideas?

marbetschar commented 9 years ago

I think I got the fix for the scrolling errors - although I'm not sure if there are better ways to implement:

As posted before #77 fixes the frozenColumns scrolling missbehaviour on the left hand side of my screenshot. Unfortunately it doesn't fix the horizontal scrolling missbehaviour of the frozenRows at the bottom.

Seems like the error occurs due to the fact, that the visible cells of these rows are rendered and cached at the beginning. On horizontal scrolling the cells which are newly entering the viewport on the right hand side are then not rendered, since SlickGrid assumes they are already complete due to the fact that the rows are in the rowCache.

To force SlickGrid to rerender the frozenRows at the bottom every time a scrolling occured I've changed the cleanupRows function as follows:

BEFORE (not working)

function cleanupRows(rangeToKeep) {
        for (var i in rowsCache) {
            var removeFrozenRow = true;

            if (hasFrozenRows
                && ( ( options.frozenBottom && i >= actualFrozenRow ) // Frozen bottom rows
                || ( !options.frozenBottom && i <= actualFrozenRow ) // Frozen top rows
                )
                ) {
                removeFrozenRow = false;
            }

            if (( ( i = parseInt(i, 10)) !== activeRow )
                && ( i < rangeToKeep.top || i > rangeToKeep.bottom )
                && ( removeFrozenRow )
                ) {
                removeRowFromCache(i);
            }
        }
    }

AFTER (working)

function cleanupRows(rangeToKeep) {
        for (var i in rowsCache) {
            var removeFrozenRow = true;

            if( hasFrozenRows && !options.frozenBottom && i <= actualFrozenRow ){ // Frozen top rows (force re-rendering of frozen bottom rows)
                removeFrozenRow = false;
            }

            if (( ( i = parseInt(i, 10)) !== activeRow )
                && ( i < rangeToKeep.top || i > rangeToKeep.bottom )
                && ( removeFrozenRow )
                ) {
                removeRowFromCache(i);
            }
        }
    }

As this has a measurable impact on the grid performance I'm wondering if there is a better way?