Skip to content

Allow taking Fn(EVENT) for callbacks too #1989

Merged
siku2 merged 4 commits intoyewstack:masterfrom
ranile:into-event-callback
Aug 9, 2021
Merged

Allow taking Fn(EVENT) for callbacks too #1989
siku2 merged 4 commits intoyewstack:masterfrom
ranile:into-event-callback

Conversation

@ranile
Copy link
Member

@ranile ranile commented Aug 4, 2021

Description

This PR introduces a new trait, IntoEventCallback, to allow Fn(EVENT) to be passed for listeners, in addition to Callback.
This is workaround for specialization (or lack thereof) and should be removed once specialization hits.

Fixes #1657 (the main problem described)

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code
  • I have added tests

@ranile
Copy link
Member Author

ranile commented Aug 4, 2021

I'm not sure where the tests for this should go. This isn't macro related so yew-macro sounds like the wrong place but there's no where else that runs compile tests.

@mc1098
Copy link
Contributor

mc1098 commented Aug 4, 2021

Nice workaround :)

How about something like this in that mod.rs file?

#[test]
fn supported_into_event_callback_types() {
    let f = |_: usize| ();
    let cb = Callback::from(f);

    // Callbacks
    let _: Option<Callback<usize>> = cb.clone().into_event_callback();
    let _: Option<Callback<usize>> = (&cb).into_event_callback();
    let _: Option<Callback<usize>> = Some(cb).into_event_callback();

    // Fns
    let _: Option<Callback<usize>> = f.into_event_callback();
    let _: Option<Callback<usize>> = Some(f).into_event_callback();
}

Proves the supported types compile and the test passes :)
I think there are other tests, like the one you fixed, that cover the user/html! side of checking for incorrect EVENT type and adding Callbacks to elements correctly.

@ranile ranile force-pushed the into-event-callback branch from 4723fff to a978a0a Compare August 7, 2021 17:33
Copy link
Contributor

@mc1098 mc1098 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :)

@siku2 siku2 merged commit c6458f1 into yewstack:master Aug 9, 2021
mc1098 pushed a commit to mc1098/yew that referenced this pull request Aug 31, 2021
PR yewstack#1542 didn't merge in the changes from yewstack#1989 correctly, this is fixed
in this commit.
siku2 pushed a commit that referenced this pull request Aug 31, 2021
PR #1542 didn't merge in the changes from #1989 correctly, this is fixed
in this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Listeners should take Into<Callback<Event>> instead of Callback<Event>

3 participants