JLynch7 / SlickGrid

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

Changes made to handleClick() in slick.grid.js break row selection #76

Open jusbuc2k opened 10 years ago

jusbuc2k commented 10 years ago

The last block of handleClick() breaks row selection, because it only calls setActiveCellInternal if there are frozen rows.

I fixed this by changing this:

handleClick() {
...
  if ((activeCell != cell.cell || activeRow != cell.row) && canCellBeActive(cell.row, cell.cell)) {
                if (!getEditorLock().isActive() || getEditorLock().commitCurrentEdit()) {
                    if (hasFrozenRows) {
                        if (( !( options.frozenBottom ) && ( cell.row >= actualFrozenRow ) )
                            || ( options.frozenBottom && ( cell.row < actualFrozenRow ) )
                            ) {
                            scrollRowIntoView(cell.row, false);
                        }

                        setActiveCellInternal(getCellNode(cell.row, cell.cell));
                    }
                }
            }
}

to this:

handleClick() {
...
  if ((activeCell != cell.cell || activeRow != cell.row) && canCellBeActive(cell.row, cell.cell)) {
                if (!getEditorLock().isActive() || getEditorLock().commitCurrentEdit()) {
                    if (hasFrozenRows) {
                        if (( !( options.frozenBottom ) && ( cell.row >= actualFrozenRow ) )
                            || ( options.frozenBottom && ( cell.row < actualFrozenRow ) )
                            ) {
                            // what was the point here?
                        }
                    }
                    scrollRowIntoView(cell.row, false);
                    setActiveCellInternal(getCellNode(cell.row, cell.cell));
                }
            }
}

This eliminates any difference of handling on hasFrozenRows or not, However, I have to believe there was a reason for the conditional, but I'm not sure what it was.

Edit: Fixed codeblocks md syntax.

gigermocas commented 10 years ago

I had to make a similar fix on my side, as this was breaking my app regarding editor activation on click.

Rybadour commented 10 years ago

@jusbuc2k Assuming you want to keep his intentions you should change it to the following instead:

handleClick() {
...
  if ((activeCell != cell.cell || activeRow != cell.row) && canCellBeActive(cell.row, cell.cell)) {
                if (!getEditorLock().isActive() || getEditorLock().commitCurrentEdit()) {
                    if (hasFrozenRows) {
                        if (( !( options.frozenBottom ) && ( cell.row >= actualFrozenRow ) )
                            || ( options.frozenBottom && ( cell.row < actualFrozenRow ) )
                            ) {
                            scrollRowIntoView(cell.row, false);
                        }
                    } else {
                        scrollRowIntoView(cell.row, false);
                    }

                    setActiveCellInternal(getCellNode(cell.row, cell.cell));
                }
            }
}