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

Passing additional options to addEventListener #824

Closed
monstasat opened this issue Jul 1, 2019 · 3 comments
Closed

Passing additional options to addEventListener #824

monstasat opened this issue Jul 1, 2019 · 3 comments

Comments

@monstasat
Copy link
Contributor

monstasat commented Jul 1, 2019

For now there is no way to pass additional options to addEventListener method of eventTarget class. Js_of_ocaml only provides old-fashioned signature for addEventListener, where only useCapture flag can be passed. Here are the docs for options that can be passed to addEventListener. IMO, at least passive option should be also added to jsoo bindings (Dom, Dom_html, and Lwt_js_events modules).

Are there any reasons why this is still not a part of jsoo? Would you accept a PR if I implement this feature?

@monstasat
Copy link
Contributor Author

monstasat commented Jul 1, 2019

I've noticed that a corresponding PR (#807) already exists.
However, it lacks updates for Lwt_js_events modules.
As @FacelessPanda mentions in his PR, options can be passed to addEventListener as a single object or as optional arguments. I think passing optional arguments is a better option, as we can check browser support for additional options like passive and once and ignore them if not supported.
As for the Lwt_js_events module, the once option seems redundant because event listener is added once again after every event occurrence, so we may need to add only passive and use_capture optional arguments to existing functions.

@hhugo
Copy link
Member

hhugo commented Jul 14, 2019

#807 has been merged. PR welcome to add the corresponding feature in Lwt_js_events.

@monstasat
Copy link
Contributor Author

PR #866 implements the passive feature for Lwt_js_events.

@hhugo hhugo closed this as completed Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants