Skip to main content

Search and Top Navigation

#10213 closed bug (notabug)

Opened July 15, 2014 07:17PM UTC

Closed July 15, 2014 07:23PM UTC

Last modified July 16, 2014 03:41PM UTC

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.

Attachments (0)
Change History (8)

Changed July 15, 2014 07:23PM UTC by tj.vantoll comment:1

resolution: → notabug
status: newclosed

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

Changed July 15, 2014 07:25PM UTC by spjonez comment:2

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

Changed July 15, 2014 07:53PM UTC by tj.vantoll comment:3

Replying to [comment:2 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.

Changed July 16, 2014 11:49AM UTC by spjonez comment:4

_comment0: Replying to [comment:3 tj.vantoll]: \ > Replying to [comment:2 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 another 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 the case where you want to bind an event to "change" on every input in a form. 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. 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?1405511515665419
_comment1: Replying to [comment:3 tj.vantoll]: \ > Replying to [comment:2 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 another 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?1405512066356352

Replying to [comment:3 tj.vantoll]:

Replying to [comment:2 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?

Changed July 16, 2014 01:04PM UTC by scottgonzalez comment:5

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" );
});

Changed July 16, 2014 02:09PM UTC by spjonez comment:6

_comment0: Replying to [comment:5 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.1405519841993369

Replying to [comment:5 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.

Changed July 16, 2014 03:08PM UTC by scottgonzalez comment:7

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.

Changed July 16, 2014 03:41PM UTC by spjonez comment:8

Replying to [comment:7 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.