Open ksassnowski opened 3 years ago
Thanks @ksassnowski - Really appreciate the time you took to make improvements and write the tests🙌 I'll take a look this weekend or Monday and get back to you! Have a great weekend!
Sorry for the wait @ksassnowski, it's still on my list to review, thanks for your patience 🙏
No worries 🤘
Updates?
This PR adds a basic test suite for the existing functionality. Since I'm not too familiar with Livewire, I didn't add any tests using
Livewire::test()
. I also tried to mostly document the existing behavior without changing too much of the implementation.There were a few issues I noticed while writing the tests:
execute
method on theSpotlight
class does not check ifgetCommandById
actually found a command. If it returnsnull
themethod_exists
check will fail with aTypeError
.placeholder
onSpotlightCommandDependency
doesn't have a default value but also isn't required in the constructor. This effectively makes it mandatory to callsetPlaceholder
since you'd run into an error when callingtoArray
because the$placeholder
property has not been initialized yet.setType
method on theSpotlightCommandDependency
class accepts any string, even though it looks like the package only knows how to deal withsearch
andinput
. Maybe replace it with two separate methods to set the$type
tosearch
orinput
, respectively?Anyways, let me know what you think!