Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Selectmenu: Delay trigger until after the list closes

Closes gh-7197
Fixes gh-7076
  • Loading branch information...
commit dde487317c032685dd1bc16750d6b06a6032e8de 1 parent a9472ce
@gabrielschulhof gabrielschulhof authored
View
101 js/widgets/forms/select.custom.js
@@ -110,6 +110,10 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
},
_handleMenuPageHide: function() {
+
+ // After the dialog's done, we may want to trigger change if the value has actually changed
+ this._delayedTrigger();
+
// TODO centralize page removal binding / handling in the page plugin.
// Suggestion from @jblas to do refcounting
//
@@ -132,18 +136,55 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
}
},
+ _handleListItemClick: function( event ) {
+ var listItem = $( event.target ).closest( "li" ),
+
+ // Index of option tag to be selected
+ oldIndex = this.select[ 0 ].selectedIndex,
+ newIndex = $.mobile.getAttribute( listItem, "option-index" ),
+ option = this._selectOptions().eq( newIndex )[ 0 ];
+
+ // Toggle selected status on the tag for multi selects
+ option.selected = this.isMultiple ? !option.selected : true;
+
+ // Toggle checkbox class for multiple selects
+ if ( this.isMultiple ) {
+ listItem.find( "a" )
+ .toggleClass( "ui-checkbox-on", option.selected )
+ .toggleClass( "ui-checkbox-off", !option.selected );
+ }
+
+ // If it's not a multiple select, trigger change after it has finished closing
+ if ( !this.isMultiple && oldIndex !== newIndex ) {
+ this._triggerChange = true;
+ }
+
+ // Trigger change if it's a multiple select
+ // Hide custom select for single selects only - otherwise focus clicked item
+ // We need to grab the clicked item the hard way, because the list may have been rebuilt
+ if ( this.isMultiple ) {
+ this.select.trigger( "change" );
+ this.list.find( "li:not(.ui-li-divider)" ).eq( newIndex )
+ .find( "a" ).first().focus();
+ }
+ else {
+ this.close();
+ }
+
+ event.preventDefault();
+ },
+
build: function() {
var selectId, popupId, dialogId, label, thisPage, isMultiple, menuId,
themeAttr, overlayTheme, overlayThemeAttr, dividerThemeAttr,
menuPage, listbox, list, header, headerTitle, menuPageContent,
- menuPageClose, headerClose, self,
+ menuPageClose, headerClose,
o = this.options;
if ( o.nativeMenu ) {
return this._super();
}
- self = this;
selectId = this.selectId;
popupId = selectId + "-listbox";
dialogId = selectId + "-dialog";
@@ -221,52 +262,18 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
// Events for list items
this.list.attr( "role", "listbox" );
this._on( this.list, {
- focusin : "_handleListFocus",
- focusout : "_handleListFocus",
- keydown: "_handleListKeydown"
+ "focusin": "_handleListFocus",
+ "focusout": "_handleListFocus",
+ "keydown": "_handleListKeydown",
+ "click li:not(.ui-disabled,.ui-state-disabled,.ui-li-divider)": "_handleListItemClick"
});
- this.list
- .delegate( "li:not(.ui-disabled,.ui-state-disabled,.ui-li-divider)", "click", function( event ) {
-
- // index of option tag to be selected
- var oldIndex = self.select[ 0 ].selectedIndex,
- newIndex = $.mobile.getAttribute( this, "option-index" ),
- option = self._selectOptions().eq( newIndex )[ 0 ];
-
- // toggle selected status on the tag for multi selects
- option.selected = self.isMultiple ? !option.selected : true;
-
- // toggle checkbox class for multiple selects
- if ( self.isMultiple ) {
- $( this ).find( "a" )
- .toggleClass( "ui-checkbox-on", option.selected )
- .toggleClass( "ui-checkbox-off", !option.selected );
- }
-
- // trigger change if value changed
- if ( self.isMultiple || oldIndex !== newIndex ) {
- self.select.trigger( "change" );
- }
-
- // hide custom select for single selects only - otherwise focus clicked item
- // We need to grab the clicked item the hard way, because the list may have been rebuilt
- if ( self.isMultiple ) {
- self.list.find( "li:not(.ui-li-divider)" ).eq( newIndex )
- .find( "a" ).first().focus();
- }
- else {
- self.close();
- }
-
- event.preventDefault();
- });
// button refocus ensures proper height calculation
// by removing the inline style and ensuring page inclusion
this._on( this.menuPage, { pagehide: "_handleMenuPageHide" } );
// Events on the popup
- this._on( this.listbox, { popupafterclose: "close" } );
+ this._on( this.listbox, { popupafterclose: "_popupClosed" } );
// Close button on small overlays
if ( this.isMultiple ) {
@@ -276,6 +283,18 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
return this;
},
+ _popupClosed: function() {
+ this.close();
+ this._delayedTrigger();
+ },
+
+ _delayedTrigger: function() {
+ if ( this._triggerChange ) {
+ this.element.trigger( "change" );
+ }
+ this._triggerChange = false;
+ },
+
_isRebuildRequired: function() {
var list = this.list.find( "li" ),
options = this._selectOptions().not( ".ui-screen-hidden" );
View
58 tests/integration/select/index.html
@@ -36,6 +36,64 @@
<div id="qunit"></div>
<div id="default" data-nstest-role="page" data-nstest-theme="c">
+ <select name="small-select-change-after-close" id="small-select-change-after-close" data-nstest-native-menu="false">
+ <option value="1">One</option>
+ <option value="2">Two</option>
+ <option value="3">Three</option>
+ </select>
+ <select name="large-select-change-after-close" id="large-select-change-after-close" data-nstest-native-menu="false">
+ <option value="AL">Alabama</option>
+ <option value="AK">Alaska</option>
+ <option value="AZ">Arizona</option>
+ <option value="AR">Arkansas</option>
+ <option value="CA">California</option>
+ <option value="CO">Colorado</option>
+ <option value="CT">Connecticut</option>
+ <option value="DE">Delaware</option>
+ <option value="FL">Florida</option>
+ <option value="GA">Georgia</option>
+ <option value="HI">Hawaii</option>
+ <option value="ID">Idaho</option>
+ <option value="IL">Illinois</option>
+ <option value="IN">Indiana</option>
+ <option value="IA">Iowa</option>
+ <option value="KS">Kansas</option>
+ <option value="KY">Kentucky</option>
+ <option value="LA">Louisiana</option>
+ <option value="ME">Maine</option>
+ <option value="MD">Maryland</option>
+ <option value="MA">Massachusetts</option>
+ <option value="MI">Michigan</option>
+ <option value="MN">Minnesota</option>
+ <option value="MS">Mississippi</option>
+ <option value="MO">Missouri</option>
+ <option value="MT">Montana</option>
+ <option value="NE">Nebraska</option>
+ <option value="NV">Nevada</option>
+ <option value="NH">New Hampshire</option>
+ <option value="NJ">New Jersey</option>
+ <option value="NM">New Mexico</option>
+ <option value="NY">New York</option>
+ <option value="NC">North Carolina</option>
+ <option value="ND">North Dakota</option>
+ <option value="OH">Ohio</option>
+ <option value="OK">Oklahoma</option>
+ <option value="OR">Oregon</option>
+ <option value="PA">Pennsylvania</option>
+ <option value="RI">Rhode Island</option>
+ <option value="SC">South Carolina</option>
+ <option value="SD">South Dakota</option>
+ <option value="TN">Tennessee</option>
+ <option value="TX">Texas</option>
+ <option value="UT">Utah</option>
+ <option value="VT">Vermont</option>
+ <option value="VA">Virginia</option>
+ <option value="WA">Washington</option>
+ <option value="WV">West Virginia</option>
+ <option value="WI">Wisconsin</option>
+ <option value="WY">Wyoming</option>
+ </select>
+
<div data-nstest-role="fieldcontain" id="select-choice-few-container">
<select name="select-choice-few" id="select-choice-few.dotTest" data-nstest-native-menu="false">
<option value="standard">Standard: 7 day</option>
View
96 tests/integration/select/select_core.js
@@ -536,4 +536,100 @@
]);
});
+ // Utensils for logging calls to $.event.trigger()
+ var callLog, origTrigger,
+ replacementTrigger = function( event, data, element, onlyHandlers ) {
+ callLog.push({
+ event: event,
+ data: data,
+ element: element,
+ onlyHandlers: onlyHandlers
+ });
+ return origTrigger.apply( this, arguments );
+ };
+
+ module( "Custom select change comes after closing list", {
+ setup: function() {
+ callLog = [];
+ origTrigger = $.event.trigger;
+ $.event.trigger = replacementTrigger;
+ },
+ teardown: function() {
+ $.event.trigger = origTrigger;
+ }
+ });
+
+ function testChangeAfterClose( select, ns, openEvent, closeEvent, tail ) {
+ var closeComesBeforeChange = false,
+ closeEventName = closeEvent.event;
+
+ openEvent.event += ns + "1";
+ closeEvent.event += ns + "2";
+
+ $.testHelper.detailedEventCascade([
+ function() {
+ $( "#" + select.attr( "id" ) + "-button" ).click();
+ },
+ {
+ openevent: openEvent
+ },
+ function() {
+ $( "#" + select.attr( "id" ) + "-menu" ).find( "a" ).eq( 2 ).click();
+ },
+ {
+ closeevent: closeEvent,
+ change: { src: select, event: "change" + ns + "2" }
+ },
+ function() {
+ $.each( callLog, function( index, value ) {
+ var name = ( typeof callLog[ index ].event === "string" ?
+ callLog[ index ].event :
+ callLog[ index ].event.type ),
+ target = callLog[ index ].element;
+
+ if ( name === "change" && target === select[ 0 ] ) {
+ return false;
+ }
+
+ if ( name === closeEventName &&
+ target === ( typeof closeEvent.src === "function" ?
+ closeEvent.src()[ 0 ] :
+ closeEvent.src[ 0 ] ) ) {
+
+ closeComesBeforeChange = true;
+ return false;
+ }
+ });
+
+ deepEqual( closeComesBeforeChange, true,
+ "close event is triggered before change event" );
+ tail();
+ }
+ ]);
+ }
+
+ asyncTest( "Small select triggers change after popup closes", function() {
+ testChangeAfterClose( $( "#small-select-change-after-close" ),
+ ".smallSelectTriggersChangeAfterPopupCloses",
+ {
+ src: $( "#small-select-change-after-close-listbox" ),
+ event: "popupafteropen"
+ },
+ {
+ src: $( "#small-select-change-after-close-listbox" ),
+ event: "popupafterclose"
+ }, start);
+ });
+
+ asyncTest( "Large select triggers change after dialog closes", function() {
+ testChangeAfterClose( $( "#large-select-change-after-close" ),
+ ".largeSelectTriggersChangeAfterPopupCloses",
+ { src: $( document ), event: "pageshow" },
+ {
+ src: function() { return $( "#large-select-change-after-close-dialog" ); },
+ event: "pagehide"
+ },
+ start);
+ });
+
})(jQuery);
Please sign in to comment.
Something went wrong with that request. Please try again.