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

Improve documentation explaining passing multiple args in event handlers #238

Merged
merged 1 commit into from
Jan 2, 2020

Conversation

NHDaly
Copy link
Collaborator

@NHDaly NHDaly commented Dec 31, 2019

Explain how to pass an array, and/or use argument destructuring, in order to pass multiple arguments to a handler.

Follow up to #237; fixes #222.

Explain how to pass an array, and/or use argument destructuring, in order to pass multiple arguments to a handler.
@NHDaly NHDaly added the docs label Dec 31, 2019
@NHDaly NHDaly requested a review from twavv December 31, 2019 16:02
@NHDaly NHDaly mentioned this pull request Dec 31, 2019
@twavv
Copy link
Member

twavv commented Jan 1, 2020

Looks good to me.

Honestly, I think we should structure things a little differently so that we can just "naturally" support multiple arguments. We do this in WebIO with the RPC protocol. We should be able to do that without breaking anything since passing arrays should continue to work.

Alternatively, one thing that I've been liking recently is defining handlers using Val{...} types. I'm not sure it works as well in conjunction with what I described above because it's harder to do with variable numbers of args (without method ambiguity errors). I think it works well for WebIO to define new message types though: https://github.com/JuliaGizmos/WebIO.jl/pull/372/files#diff-f5a345e16e62d96204ee9e29d0386ad4R38

function Blink.handle_msg(::Val{:press}, window::Window, data)
    @show data["foo"]
end
Blink.msg("press", {
    foo: "foo",
});

It's a little bit less dynamic but that might not always be a bad thing (and we could keep the existing "dynamic" method anyway).

@NHDaly
Copy link
Collaborator Author

NHDaly commented Jan 2, 2020

Yeah that does seem really nice as well! I don't have much preference, but defining functions that are more "normal," definitely seems like an improvement.

For now, I'm going to merge this docs change, since this is the current way to do things, but i'm supportive of moving towards something more like that!

@NHDaly NHDaly merged commit 70215d6 into master Jan 2, 2020
@NHDaly NHDaly deleted the NHDaly-docs-patch-1 branch January 2, 2020 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in documentation
2 participants