Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unable to prevent cell editing in event onBeforeEditCell #992

Closed
5 tasks done
b-giavotto opened this issue Feb 16, 2024 · 3 comments
Closed
5 tasks done

unable to prevent cell editing in event onBeforeEditCell #992

b-giavotto opened this issue Feb 16, 2024 · 3 comments

Comments

@b-giavotto
Copy link

Describe the bug

there is no way to prevent cellediting (returning false on the event subscriber) in beforeEditCell handler of the grid.

Reproduction

define and subscrive an event on grid onBeforeEditCell, in the body simply return false;
the cause is because of the protected method called in makeActiveCellEditable does a call to another method, named trigger:
if (this.trigger(this.onBeforeEditCell, { row: this.activeRow, cell: this.activeCell, item, column: columnDef, target: 'grid' }).getReturnValue() === false) {

But the test is wrong!: the return value IS not a boolean type but instead a Promise
I think makeActiveCellEditable is called on other sites, and the fix should affect even others parts.

Which Framework are you using?

Vanilla / Plain JS

Environment Info

| Executable          | Version |
| ------------------- | ------- |
| (framework used)    | DNA|
| SlickGrid | VERSION |DNA
| TypeScript          | DNA|
| Browser(s)          | DNA|
| System OS           | DNA|

Validations

@6pac
Copy link
Owner

6pac commented Feb 19, 2024

I'll leave this one to @ghiscoding , I think it's probably a typescript issue.

@ghiscoding
Copy link
Collaborator

I don't have any problems with onBeforeEditCell and it works as intended on my side, please provide code to replicate the issue or contribute a Pull Request

ghiscoding pushed a commit that referenced this issue Feb 22, 2024
- related to comments in issue #992
ghiscoding added a commit that referenced this issue Feb 22, 2024
@ghiscoding
Copy link
Collaborator

ghiscoding commented Feb 22, 2024

The comments seems invalid, also since we did not have any examples with onBeforeEditCell returning false, I modified one of the example in PR #993 with proper E2E tests validating the effect of returning false within the subscriber. Another alternative might be to call e.preventDefault() to cancel event bubbling

But the test is wrong!: the return value IS not a boolean type but instead a Promise

this is wrong, it doesn't return a Promise but rather a SlickEventData which is synchronous and the way to get the returned value (however this is usually meant to be used by internal code only) is to call .getReturnValue() like we do internally in SlickGrid

SlickGrid/src/slick.grid.ts

Lines 5926 to 5929 in 61e73db

if (this.trigger(this.onBeforeEditCell, { row: this.activeRow, cell: this.activeCell, item, column: columnDef, target: 'grid' }).getReturnValue() === false) {
this.setFocus();
return;
}

So I'm closing since the details seems incorrect and there was also no feedback provided when asking for a repro

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants