Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#10213 closed bug (notabug)

Change event does not always fire

Reported by: spjonez Owned by:
Priority: minor Milestone: none
Component: ui.selectmenu Version: 1.11.0
Keywords: Cc:
Blocked by: Blocking:

Description

Something is bugged with the 'change' event on the original element. When you change items, $.Widget._trigger is called with the correct type=change, however event.type=selectmenuchange so any handlers that have been bound to the original select with 'change' do not fire.

Change History (8)

comment:1 Changed 6 years ago by tj.vantoll

Resolution: notabug
Status: newclosed

We only support binding to the custom event. See http://contribute.jquery.org/wont-fix/#change-event.

comment:2 Changed 6 years ago by spjonez

Why create a custom event when a native one exists for that exact action? Doesn't make any sense.

comment:3 in reply to:  2 ; Changed 6 years ago by tj.vantoll

Replying to spjonez:

Why create a custom event when a native one exists for that exact action? Doesn't make any sense.

See the discussion in #8878.

comment:4 in reply to:  3 Changed 6 years ago by spjonez

Replying to tj.vantoll:

Replying to spjonez:

Why create a custom event when a native one exists for that exact action? Doesn't make any sense.

See the discussion in #8878.

For custom functionality like draggable that makes perfect sense, but as the last reply in that thread stated creating a custom event to replace a native event is not intuitive in the slightest. All my code is bound to native events, the previous version of selectmenu worked with native events, now to use this version I have to find them all and change them to your version of the event.

Isn't the purpose to jQuery to reduce complexity? This change is the opposite of that. I fail to see what improvement using "selectmenuchange" provides over "change". It's more confusing and for what benefit?

Consider your own documentation on the change event: http://api.jquery.com/change/

Your example code won't work with your plugin. It will fail silently as change is never triggered on the select.

Consider the case where you want to bind an event to "change" on every input in a form. This is the exact situation I'm in. I have a form, any time a field is modified I need to trigger a function. Originally I could bind it at once;

$( form ).find( 'input, select' ).on( 'change', function( ) { } );

Now I have to write it as two lines;

$( form ).find( 'input' ).on( 'change', function( ) { } ); $( form ).find( 'select' ).on( 'selectmenuchange', function( ) { } );

And what if all the selects aren't using selectmenu? In that case I'd have to bind it 3 times... see where I'm going with this and why it's a bad idea?

Last edited 6 years ago by spjonez (previous) (diff)

comment:5 Changed 6 years ago by Scott González

We cannot guarantee the expected behavior of the native change event because complex widgets may have native controls focus and blur at various points during what is considered a single interaction. Therefore, the only reliable event is a custom one.

As for having to make everything so complex with event bindings, you're fooling yourself. If you were going to do all three bindings, you could just do:

$( form ).find( "input, select" ).on( "change selectmenuchange", function() { ... } );

Or you could get more sophisticated if you want to guard against potentially incorrect native change events on custom widgets (and while you're at it, avoid the cost of direct bindings):

$( form ).on( "change selectmenuchange", "input, select", function( event ) {
    if ( $( event.target ).selectmenu( "instance" ) && event.type === "change" ) {
        return;
    }

    // Handle all real changes
});

Or you can just trigger the native event yourself if that's the road you really want to go down:

$( form ).on( "selectmenuchange", "select", function( event ) {
    $( this ).trigger( "change" );
});

comment:6 in reply to:  5 Changed 6 years ago by spjonez

Replying to scott.gonzalez:

We cannot guarantee the expected behavior of the native change event because complex widgets may have native controls focus and blur at various points during what is considered a single interaction. Therefore, the only reliable event is a custom one.

Is this actually the case for this plugin? If yes, what code prevents you from using the original event, and will you accept a patch to fix it? The original plugin did not have this flaw so a custom event cannot be necessary.

As for having to make everything so complex with event bindings, you're fooling yourself. If you were going to do all three bindings, you could just do:

$( form ).find( "input, select" ).on( "change selectmenuchange", function() { ... } );

I could do that yes but why am I binding events on elements that will never fire..? Easy != correct.

Or you could get more sophisticated if you want to guard against potentially incorrect native change events on custom widgets (and while you're at it, avoid the cost of direct bindings):

$( form ).on( "change selectmenuchange", "input, select", function( event ) {
    if ( $( event.target ).selectmenu( "instance" ) && event.type === "change" ) {
        return;
    }

    // Handle all real changes
});

Or you can just trigger the native event yourself if that's the road you really want to go down:

$( form ).on( "selectmenuchange", "select", function( event ) {
    $( this ).trigger( "change" );
});

Doesn't that seem a bit convoluted for something that could be easily solved by simply triggering the native event? It seems this is being implemented "just because" and the solution is not intuitive which is the entire point of using a library like jQuery.

Last edited 6 years ago by spjonez (previous) (diff)

comment:7 Changed 6 years ago by Scott González

Is this actually the case for this plugin?

It shouldn't be, since you never interact with the original element. But we have a policy that we don't try to preserve the functionality of the native controls because of what I've already said.

I could do that yes but why am I binding events on elements that will never fire..? Easy != correct.

There is nothing incorrect about this.

Doesn't that seem a bit convoluted for something that could be easily solved by simply triggering the native event?

I'm not sure what's convoluted about this. The exact same logic needs to happen if we implement it instead of you. However, if you implement it, then we don't need to deal with any "bugs" that occur because of discrepancies with native change events that occur at unexpected times. Again, this doesn't apply to this widget, but we deal with the library as a whole.

comment:8 in reply to:  7 Changed 6 years ago by spjonez

Replying to scott.gonzalez:

Is this actually the case for this plugin?

It shouldn't be, since you never interact with the original element. But we have a policy that we don't try to preserve the functionality of the native controls because of what I've already said.

Well that sucks, seems like a poor design choice to enforce even when it's not applicable.

I could do that yes but why am I binding events on elements that will never fire..? Easy != correct.

There is nothing incorrect about this.

Increased memory usage? We can agree to disagree, but I would never recommend a dev bind events that will never fire just to save a few characters.

Doesn't that seem a bit convoluted for something that could be easily solved by simply triggering the native event?

I'm not sure what's convoluted about this. The exact same logic needs to happen if we implement it instead of you. However, if you implement it, then we don't need to deal with any "bugs" that occur because of discrepancies with native change events that occur at unexpected times. Again, this doesn't apply to this widget, but we deal with the library as a whole.

I expected a library like jQuery UI would want to use as much built-in functionality as possible and only extend it when necessary. Having a blanket policy like that screams of something MS would do just to have IE be different.

It's your library but design choices like this will make myself and other developers look for other solutions that don't require library specific events to function.

Note: See TracTickets for help on using tickets.