SlickGrid setSelectedRows method fails to set CSS "selected" class attribute if called when there is an edit operation in place in another SlickGrid

b-giavotto commented 9 months ago

Describe the bug

The bug is related to the getEditorLock method which returns the editorLock. When an editor is displayed there is an active editorLock Object. the method setSelectedRows internally check if there is a editorLock active and does not perform any refresh operation, missing the CSS update operation. The problem is not related to the grid that was in edit but instead on every other related slickgrid that whose setSelectedRow method gets invoked while an inplace edit is in progress. Is there a way to allow the update without "skipping" the editorCheck ? Ideally the update operation should work on every other grid that is not the edititing one.

see file : slick.grid.ts, line 6854: setSelectedRows(rows: number[], caller?: string) { if (!this.selectionModel) { throw new Error('SlickGrid Selection model is not set'); } if (this && this.getEditorLock && !this.getEditorLock()?.isActive()) { <-- Here lies the issue . this.selectionModel.setSelectedRanges(this.rowsToRanges(rows), caller || 'SlickGrid.setSelectedRows'); } }

Reproduction

The description of the problem make useless a repro. However, if needed, i can provide it, however it should be relatively long and verbose...

Which Framework are you using?

Vanilla / Plain JS

Environment Info

| Executable          | Version |
| ------------------- | ------- |
| (framework used)    | does not matter |
| SlickGrid | VERSION | latest
| TypeScript          | does not matter |
| Browser(s)          | does not matter |
| System OS           | does not matter |

Validations

[X] Follow our Code of Conduct
[X] Read the Wikis.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
[X] The provided reproduction is a minimal reproducible example of the bug.

ghiscoding commented 9 months ago

Ideally the update operation should work on every other grid that is not the edititing one.

Are you really saying "grid" or did you mean "cell"? Because multiple grids in a page shouldn't matter.

There's multiple editor lock checks in the code, you'll have to investigate yourself and contribute any potential fix if you don't want to provide a reproduction. This is an Open Source project that is maintained in our spare time

b-giavotto commented 9 months ago

Ideally the update operation should work on every other grid that is not the edititing one.

Are you really saying "grid" or did you mean "cell"? Because multiple grids in a page shouldn't matter.

There's multiple editor lock checks in the code, you'll have to investigate yourself and contribute any potential fix if you don't want to provide a reproduction. This is an Open Source project that is maintained in our spare time

The problem arised in this scenario. I've created a cell editor which is basically a SlickGrid " nested " in a cell. During the interaction with the editor, i realized that the call setSelectedRows works as expected, except for the issue above (it does not apply the correct style). Doing some debugging and digging in the source code i've realized that the reason was the check i've already highlighted. This obviously related to some advanced scenario, while creating new SlickGrids on the fly while editing a cell. Hope is clear. Maybe i could provide a repro, but i do not want to take the responsability of changing the repo with the risk of breaking someother thing. As a quick fix, and to prove i was right in my diagnosis i've overrode the SlickGrid's setSelectedRows, omitting the check on gridEditorLock and worked fine.

ghiscoding commented 9 months ago

why don't you simply commit the editor before selecting the row? Something like

if (grid.getEditorLock()?.commitCurrentEdit()) {
  // ready to select
}

// or cancel the editor, if you prefer
if (grid.getEditorLock()?.cancelCurrentEdit()) {
}

this should be done by the user before allowing row selection

6pac commented 9 months ago

The editor lock is set by default to GlobalEditorLock in options:

asyncPostRenderCleanupDelay: 40,
auto: false,
editorLock: GlobalEditorLock,
showColumnHeader: true,
showHeaderRow: false,

GlobalEditorLock is shared amongst all grids in a context (eg. a page), for simplicity. To set a different lock per grid, simply pass a new editor lock into the options:

let options = {
  editorLock: new SlickEditorLock()
};
...
grid = new SlickGrid("#myGrid", data, columns, options);

I do this by default.

b-giavotto commented 9 months ago

The editor lock is set by default to GlobalEditorLock in options:
asyncPostRenderCleanupDelay: 40,
auto: false,
editorLock: GlobalEditorLock,
showColumnHeader: true,
showHeaderRow: false,
GlobalEditorLock is shared amongst all grids in a context (eg. a page), for simplicity. To set a different lock per grid, simply pass a new editor lock into the options:
let options = {
  editorLock: new SlickEditorLock()
};
...
grid = new SlickGrid("#myGrid", data, columns, options);
I do this by default.

thx, for the suggestion.

6pac / SlickGrid