-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Allow providing callback prop to execute the command #520
Comments
@iddan can you describe what you are testing? The use of this should be optional within the command functions |
I'm testing the creation of the options provided to the command palette. If they could be data only I would be able to create them with a pure function which is much easier to unit test. |
Have you considered passing a noop function as commands then using onSelect to execute your function? |
No, I haven't. Seem like a little more complicated than the solution I proposed but it should do. |
Yes, you'd be using onSelect much like the browser native drop down list box and matching on the returned object so that your function could be executed. For context, I've periodically considered eliminating the inclusion of functions passed to the commands prop. It's really not required and breaks with native browser control conventions. However, I wanted a batteries included component the was easy to use. |
I agree it's a simpler approach. That is why I'm suggesting a more React-ive solution for the problem that separates data from function. |
@iddan I've got a few other enhancements to get to first but I'm open to a pull request |
I'll try to get around it. |
Is your feature request related to a problem? Please describe.
My problem is the command property must be a function. Because of that, I must use
this
in it for the object to be testable.Describe the solution you'd like
I suggest introducing a new prop
executeCommand: (command: Command) => void
which will take the command object and execute it. That way the command object can contain only data and nothis
need to be used.Additional context
An example would be:
Without
executeCommand
:With
executeCommand
:The text was updated successfully, but these errors were encountered: