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

extend GridOption with editCommandHandler option #15

Merged
merged 2 commits into from
Feb 26, 2018
Merged

extend GridOption with editCommandHandler option #15

merged 2 commits into from
Feb 26, 2018

Conversation

asdman384
Copy link
Contributor

@asdman384 asdman384 commented Feb 25, 2018

editCommandHandler works in base slickgrid (6pac/SlickGrid) but was lost in Angular-Slickgrid. Very usefull option I think.

@ghiscoding
Copy link
Owner

ghiscoding commented Feb 25, 2018

Sorry I have't used that option yet and we aren't actually using inline editor on current project. Correct me if I'm wrong but it looks like the only missing piece is to make it available in the gridOption interface is that correct?

From this undo demo, it looks like I also missed another option enableAddRow. I tried to cover all of the possible SlickGrid options, but it's quite possible that I missed a few since there are so many.

EDIT
Oops I didn't realized at first that this was a PR lol

@@ -49,6 +49,9 @@ export interface GridOption {
/** Defaults to false, when enabled will give the possibility to edit cell values with inline editors. */
editable?: boolean;

/** option to intercept edit commands and implement undo support. Call command.execute() to finish edit operation*/
editCommandHandler?: (row: any, column: any, command: any) => void;
Copy link
Owner

@ghiscoding ghiscoding Feb 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a mistake on the first argument, from the 6pac undo demo - queueAndExecuteCommand the function signature is the following

function queueAndExecuteCommand(item, column, editCommand) { }

I tried it locally and it does return the item data.

Also please use the types (TypeScript) when they are known.
So at the end I think it should be

editCommandHandler?: (item: any, column: Column, command: any) => void;

We could also create a EditCommand type in the future for the 3rd argument, since it's probably also a known list of command arguments.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually found the official wiki - editCommandHandler which does say the 1st argument is item. From that doc, we can also see that the editCommand (last argument) is also a known type, so we should create a new type for that, to create a new type, simply create a new interface and put it in the angular-slickgrid/models/ folder. I can do that part if you are confused about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right 'row' is confusing name (initially I meant rowData but named it as 'row').
Good idea! I'll create EditCommand type and will rename 'row' to 'item'. Thanks for the clarification.

@asdman384
Copy link
Contributor Author

Hello, Ghiscoding
Please review new commits

@ghiscoding ghiscoding merged commit da67007 into ghiscoding:master Feb 26, 2018
@ghiscoding
Copy link
Owner

Thanks that visually looks all good. Expect a new version by the end of the week. I'm working on another feature and that needs to go out first.

Thanks for the contribution :)

@ghiscoding
Copy link
Owner

It's now in the new just released version 0.12.0. Thanks again :)

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

Successfully merging this pull request may close these issues.

2 participants