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

Event in Typescript definition for onCommit should be optional #153

Closed
shawnmitchell opened this issue Apr 30, 2019 · 2 comments
Closed

Comments

@shawnmitchell
Copy link
Contributor

Intellisense hint states "if you pass a KeyboardEvent as the second argument..."
And the source code checks for a non-null value of e

But the type definition has the event as required.

If a custom component wants some other trigger to commit a cell value, it should be possible to omit the event.

/** function (newValue, [event]) {} A callback to indicate that editing is over, here is the final value. If you pass a KeyboardEvent as the second argument, React-DataSheet will perform default navigation for you (for example, going down to the next row if you hit the enter key). You actually don't need to use onCommit if the default keyboard handling is good enough for you. */
        onCommit: (newValue: V, event: React.KeyboardEvent<HTMLElement>) => void;
       

handleCommit (value, e) {
    const {onChange, onNavigate} = this.props
    if (value !== initialData(this.props)) {
      this.setState({ value, committing: true })
      onChange(this.props.row, this.props.col, value)
    } else {
      this.handleRevert()
    }
    if (e) {
      e.preventDefault()
      onNavigate(e, true)
    }
  }

@shawnmitchell
Copy link
Contributor Author

I have not contributed to OSS before but I'd be happy to create a pull request for this if the owner agrees it's an issue. I can probably also tackle #139 while I'm in there as it seems like a one-liner.

@nadbm
Copy link
Owner

nadbm commented May 1, 2019

Thank you, the type file hasnt gotten much attention. Will merge soon

nadbm added a commit that referenced this issue May 6, 2019
…tSelectedCells_fixes

#153 & #139 - make event optional in onCommit and return array from getSelectedCells
@nadbm nadbm closed this as completed May 6, 2019
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

No branches or pull requests

2 participants