Allow providing callback prop to execute the command

[iddan]

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 no this need to be used.

Additional context An example would be: Without executeCommand:

function go() {
    history.go(this.link);
}

const props = {
  options: [
    {
      name: "Go to example",
      link: "http://example.com",
      command: go
    },
  ],
};

With executeCommand:

const props = {
  options: [
    {
      name: "Go to example",
      link: "http://example.com",
    },
  ],
  executeCommand: (command) => {
    history.go(command.link)
  }
};

[asabaylus]

@iddan can you describe what you are testing? The use of this should be optional within the command functions

[iddan]

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.

[asabaylus]

Have you considered passing a noop function as commands then using onSelect to execute your function?

[iddan]

No, I haven't. Seem like a little more complicated than the solution I proposed but it should do. If I understood it correctly, It will require creating custom code to the onSelect and updating the loading state manually.

[asabaylus]

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.

[iddan]

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.

[asabaylus]

@iddan I've got a few other enhancements to get to first but I'm open to a pull request

[iddan]

I'll try to get around it.