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.

Sign up for GitHub

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

Jump to bottom

Normalize handler signature #80

Open
ryanve opened this issue Nov 27, 2012 · 5 comments
Open

Normalize handler signature #80

ryanve opened this issue Nov 27, 2012 · 5 comments

Comments

@ryanve
Copy link

ryanve commented Nov 27, 2012

To improve jQuery-compatibility and general consistency in the arguments passed to event handlers via different methods, I think that bean.fire() and .trigger() need modification. Consider:

(function ($) {
    var $html = $('html');

    function handler ()  {
        console.log(arguments);
    }

    $html.on('click', handler); // `handler` receives (eventData) when <html> is clicked
    $html.trigger('click');     // `handler` receives (eventData)

    // When triggered with extra params, bean omits the eventData, meaning
    // the line below sends just ('extra-0', 'extra-1') to `handler`.
    // In jQuery the line below sends (eventData, 'extra-0', 'extra-1')
    $html.trigger('click', ['extra-0', 'extra-1']);

}(ender)); // 2012-11-27 ender build jeesh

The arguments passed to handlers should be as normalized as possible between native / custom / triggers etc. The arguments[0] slot should be reserved for an eventData object. In situations without applicable eventData, it could be passed as null or (better) a simple object like:

{
    isBean: true, 
    isTrigger: true // jQuery includes this on triggers
}
@rvagg
Copy link
Collaborator

rvagg commented Nov 27, 2012

@ryanve I completely agree with you here, and not just for jQuery compatibility, the trigger arg stuff is a bit of a mess. Also, given the fact that we do a lot of manual handler triggering now we can do some Event object emulation stuff, like jQuery does.

Consider this jQuery, a pattern used a bit in Bootstrap:

var e = $.Event('show')
el.trigger(e)
if (!e.isDefaultPrevented()) {
  // do something
}

Being able to create and retain the event object has a heap of benefits.

Unfortunately the horse has bolted on that and it's a bit difficult to change the API. Perhaps we'll try and solicit some feedback on this to see if anyone is actually using the trigger arguments in Bean.

@rvagg
Copy link
Collaborator

rvagg commented Nov 27, 2012

@ded @connor @st-luke @iamdustan (or anyone else?) got an opinion on this?

@luk-
Copy link

luk- commented Nov 27, 2012

I'm not using trigger args in bean but normalization is a good thing if it can be done without totally screwing people.

@connor
Copy link
Contributor

connor commented Nov 28, 2012

I'm also not using trigger arguments in any projects using Bean (although may discover some during the conversion, Rod, like we talked about over email). I'm for the first element in the array being reserved for an eventData object.

@iamdustan
Copy link

I agree with the argument presented here that the first argument should be reserved for the eventData object and the second being optional arguments. It’s clean and provides an expected and consistent way to determine how the event was triggered and how to access your custom arguments.

I haven’t used the trigger arguments because of how it’s applied currently (I tried. I may still have used it, but it definitely caused some confusion at the time)

@ryanve ryanve mentioned this issue Jan 28, 2014
Open
ryanve added a commit to ryanve/bean that referenced this issue Feb 16, 2014
@ryanve
Add fire-demo.html to inspect handler arguments (fat#80, fat#102)
8f0daf5
ryanve added a commit to ryanve/bean that referenced this issue Feb 16, 2014
@ryanve
Normalize handler signature per fat#102 and fat#80
ea02b59
@ryanve ryanve mentioned this issue Feb 16, 2014
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@luk- @connor @iamdustan @rvagg @ryanve