Add preventDefault option to event bindings#148
Conversation
brianmhunt
left a comment
There was a problem hiding this comment.
Thanks for this, looks like there's a few minor things and this would be ok to merge.
Also add test for static value of preventDefault.
brianmhunt
left a comment
There was a problem hiding this comment.
Looks good to me.
Sorry the CI tests don't run automatically on commit, but I trust the tests pass as expected.
Feel free to merge.
On a release plan, I'm working on a Github action for releases. Note that it'll be with esbuild, which is not as well tested as the rollup version, so I'll be starting with 4.1.0... release tags.
| }) | ||
|
|
||
| it('Should allow default action by setting preventDefault:false', function () { | ||
| testNode.innerHTML = "<div data-bind='event: {click: test}'><a href='#' data-bind='event: {click: {preventDefault: false}}'>hey</a></div>" |
There was a problem hiding this comment.
We could also test preventDefault: true here, though the code path for that is identical to the unwrap() tested below so I'm not concerned.
Am I able to merge this into the upstream myself? I assumed you'd need to initiate that merge. (Sorry, I'm not too familiar with the workflow in github.) |
I'm not sure. 😄 I merged it in any case, now. |
Here's my attempt at #147 to add a
preventDefaultoption to event bindings, as an alternative to having the handler function return a boolean.