Skip to content
This repository was archived by the owner on Oct 8, 2021. It is now read-only.

Commit ea0533f

Browse files
author
Gabriel Schulhof
committed
Selectmenu: Delay trigger until after the list closes
(cherry picked from commit dde4873) Closes gh-7197 Fixes gh-7076
1 parent 14d2343 commit ea0533f

File tree

3 files changed

+214
-41
lines changed

3 files changed

+214
-41
lines changed

‎js/widgets/forms/select.custom.js‎

Lines changed: 60 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
110110
},
111111

112112
_handleMenuPageHide: function() {
113+
114+
// After the dialog's done, we may want to trigger change if the value has actually changed
115+
this._delayedTrigger();
116+
113117
// TODO centralize page removal binding / handling in the page plugin.
114118
// Suggestion from @jblas to do refcounting
115119
//
@@ -132,18 +136,55 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
132136
}
133137
},
134138

139+
_handleListItemClick: function( event ) {
140+
var listItem = $( event.target ).closest( "li" ),
141+
142+
// Index of option tag to be selected
143+
oldIndex = this.select[ 0 ].selectedIndex,
144+
newIndex = $.mobile.getAttribute( listItem, "option-index" ),
145+
option = this._selectOptions().eq( newIndex )[ 0 ];
146+
147+
// Toggle selected status on the tag for multi selects
148+
option.selected = this.isMultiple ? !option.selected : true;
149+
150+
// Toggle checkbox class for multiple selects
151+
if ( this.isMultiple ) {
152+
listItem.find( "a" )
153+
.toggleClass( "ui-checkbox-on", option.selected )
154+
.toggleClass( "ui-checkbox-off", !option.selected );
155+
}
156+
157+
// If it's not a multiple select, trigger change after it has finished closing
158+
if ( !this.isMultiple && oldIndex !== newIndex ) {
159+
this._triggerChange = true;
160+
}
161+
162+
// Trigger change if it's a multiple select
163+
// Hide custom select for single selects only - otherwise focus clicked item
164+
// We need to grab the clicked item the hard way, because the list may have been rebuilt
165+
if ( this.isMultiple ) {
166+
this.select.trigger( "change" );
167+
this.list.find( "li:not(.ui-li-divider)" ).eq( newIndex )
168+
.find( "a" ).first().focus();
169+
}
170+
else {
171+
this.close();
172+
}
173+
174+
event.preventDefault();
175+
},
176+
135177
build: function() {
136178
var selectId, popupId, dialogId, label, thisPage, isMultiple, menuId,
137179
themeAttr, overlayTheme, overlayThemeAttr, dividerThemeAttr,
138180
menuPage, listbox, list, header, headerTitle, menuPageContent,
139-
menuPageClose, headerClose, self,
181+
menuPageClose, headerClose,
140182
o = this.options;
141183

142184
if ( o.nativeMenu ) {
143185
return this._super();
144186
}
145187

146-
self = this;
147188
selectId = this.selectId;
148189
popupId = selectId + "-listbox";
149190
dialogId = selectId + "-dialog";
@@ -221,52 +262,18 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
221262
// Events for list items
222263
this.list.attr( "role", "listbox" );
223264
this._on( this.list, {
224-
focusin : "_handleListFocus",
225-
focusout : "_handleListFocus",
226-
keydown: "_handleListKeydown"
265+
"focusin": "_handleListFocus",
266+
"focusout": "_handleListFocus",
267+
"keydown": "_handleListKeydown",
268+
"click li:not(.ui-disabled,.ui-state-disabled,.ui-li-divider)": "_handleListItemClick"
227269
});
228-
this.list
229-
.delegate( "li:not(.ui-disabled,.ui-state-disabled,.ui-li-divider)", "click", function( event ) {
230-
231-
// index of option tag to be selected
232-
var oldIndex = self.select[ 0 ].selectedIndex,
233-
newIndex = $.mobile.getAttribute( this, "option-index" ),
234-
option = self._selectOptions().eq( newIndex )[ 0 ];
235-
236-
// toggle selected status on the tag for multi selects
237-
option.selected = self.isMultiple ? !option.selected : true;
238-
239-
// toggle checkbox class for multiple selects
240-
if ( self.isMultiple ) {
241-
$( this ).find( "a" )
242-
.toggleClass( "ui-checkbox-on", option.selected )
243-
.toggleClass( "ui-checkbox-off", !option.selected );
244-
}
245-
246-
// trigger change if value changed
247-
if ( self.isMultiple || oldIndex !== newIndex ) {
248-
self.select.trigger( "change" );
249-
}
250-
251-
// hide custom select for single selects only - otherwise focus clicked item
252-
// We need to grab the clicked item the hard way, because the list may have been rebuilt
253-
if ( self.isMultiple ) {
254-
self.list.find( "li:not(.ui-li-divider)" ).eq( newIndex )
255-
.find( "a" ).first().focus();
256-
}
257-
else {
258-
self.close();
259-
}
260-
261-
event.preventDefault();
262-
});
263270

264271
// button refocus ensures proper height calculation
265272
// by removing the inline style and ensuring page inclusion
266273
this._on( this.menuPage, { pagehide: "_handleMenuPageHide" } );
267274

268275
// Events on the popup
269-
this._on( this.listbox, { popupafterclose: "close" } );
276+
this._on( this.listbox, { popupafterclose: "_popupClosed" } );
270277

271278
// Close button on small overlays
272279
if ( this.isMultiple ) {
@@ -276,6 +283,18 @@ $.widget( "mobile.selectmenu", $.mobile.selectmenu, {
276283
return this;
277284
},
278285

286+
_popupClosed: function() {
287+
this.close();
288+
this._delayedTrigger();
289+
},
290+
291+
_delayedTrigger: function() {
292+
if ( this._triggerChange ) {
293+
this.element.trigger( "change" );
294+
}
295+
this._triggerChange = false;
296+
},
297+
279298
_isRebuildRequired: function() {
280299
var list = this.list.find( "li" ),
281300
options = this._selectOptions().not( ".ui-screen-hidden" );

‎tests/integration/select/index.html‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,64 @@
3636
<div id="qunit"></div>
3737

3838
<div id="default" data-nstest-role="page" data-nstest-theme="c">
39+
<select name="small-select-change-after-close" id="small-select-change-after-close" data-nstest-native-menu="false">
40+
<option value="1">One</option>
41+
<option value="2">Two</option>
42+
<option value="3">Three</option>
43+
</select>
44+
<select name="large-select-change-after-close" id="large-select-change-after-close" data-nstest-native-menu="false">
45+
<option value="AL">Alabama</option>
46+
<option value="AK">Alaska</option>
47+
<option value="AZ">Arizona</option>
48+
<option value="AR">Arkansas</option>
49+
<option value="CA">California</option>
50+
<option value="CO">Colorado</option>
51+
<option value="CT">Connecticut</option>
52+
<option value="DE">Delaware</option>
53+
<option value="FL">Florida</option>
54+
<option value="GA">Georgia</option>
55+
<option value="HI">Hawaii</option>
56+
<option value="ID">Idaho</option>
57+
<option value="IL">Illinois</option>
58+
<option value="IN">Indiana</option>
59+
<option value="IA">Iowa</option>
60+
<option value="KS">Kansas</option>
61+
<option value="KY">Kentucky</option>
62+
<option value="LA">Louisiana</option>
63+
<option value="ME">Maine</option>
64+
<option value="MD">Maryland</option>
65+
<option value="MA">Massachusetts</option>
66+
<option value="MI">Michigan</option>
67+
<option value="MN">Minnesota</option>
68+
<option value="MS">Mississippi</option>
69+
<option value="MO">Missouri</option>
70+
<option value="MT">Montana</option>
71+
<option value="NE">Nebraska</option>
72+
<option value="NV">Nevada</option>
73+
<option value="NH">New Hampshire</option>
74+
<option value="NJ">New Jersey</option>
75+
<option value="NM">New Mexico</option>
76+
<option value="NY">New York</option>
77+
<option value="NC">North Carolina</option>
78+
<option value="ND">North Dakota</option>
79+
<option value="OH">Ohio</option>
80+
<option value="OK">Oklahoma</option>
81+
<option value="OR">Oregon</option>
82+
<option value="PA">Pennsylvania</option>
83+
<option value="RI">Rhode Island</option>
84+
<option value="SC">South Carolina</option>
85+
<option value="SD">South Dakota</option>
86+
<option value="TN">Tennessee</option>
87+
<option value="TX">Texas</option>
88+
<option value="UT">Utah</option>
89+
<option value="VT">Vermont</option>
90+
<option value="VA">Virginia</option>
91+
<option value="WA">Washington</option>
92+
<option value="WV">West Virginia</option>
93+
<option value="WI">Wisconsin</option>
94+
<option value="WY">Wyoming</option>
95+
</select>
96+
3997
<div data-nstest-role="fieldcontain" id="select-choice-few-container">
4098
<select name="select-choice-few" id="select-choice-few.dotTest" data-nstest-native-menu="false">
4199
<option value="standard">Standard: 7 day</option>

‎tests/integration/select/select_core.js‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,4 +536,100 @@
536536
]);
537537
});
538538

539+
// Utensils for logging calls to $.event.trigger()
540+
var callLog, origTrigger,
541+
replacementTrigger = function( event, data, element, onlyHandlers ) {
542+
callLog.push({
543+
event: event,
544+
data: data,
545+
element: element,
546+
onlyHandlers: onlyHandlers
547+
});
548+
return origTrigger.apply( this, arguments );
549+
};
550+
551+
module( "Custom select change comes after closing list", {
552+
setup: function() {
553+
callLog = [];
554+
origTrigger = $.event.trigger;
555+
$.event.trigger = replacementTrigger;
556+
},
557+
teardown: function() {
558+
$.event.trigger = origTrigger;
559+
}
560+
});
561+
562+
function testChangeAfterClose( select, ns, openEvent, closeEvent, tail ) {
563+
var closeComesBeforeChange = false,
564+
closeEventName = closeEvent.event;
565+
566+
openEvent.event += ns + "1";
567+
closeEvent.event += ns + "2";
568+
569+
$.testHelper.detailedEventCascade([
570+
function() {
571+
$( "#" + select.attr( "id" ) + "-button" ).click();
572+
},
573+
{
574+
openevent: openEvent
575+
},
576+
function() {
577+
$( "#" + select.attr( "id" ) + "-menu" ).find( "a" ).eq( 2 ).click();
578+
},
579+
{
580+
closeevent: closeEvent,
581+
change: { src: select, event: "change" + ns + "2" }
582+
},
583+
function() {
584+
$.each( callLog, function( index, value ) {
585+
var name = ( typeof callLog[ index ].event === "string" ?
586+
callLog[ index ].event :
587+
callLog[ index ].event.type ),
588+
target = callLog[ index ].element;
589+
590+
if ( name === "change" && target === select[ 0 ] ) {
591+
return false;
592+
}
593+
594+
if ( name === closeEventName &&
595+
target === ( typeof closeEvent.src === "function" ?
596+
closeEvent.src()[ 0 ] :
597+
closeEvent.src[ 0 ] ) ) {
598+
599+
closeComesBeforeChange = true;
600+
return false;
601+
}
602+
});
603+
604+
deepEqual( closeComesBeforeChange, true,
605+
"close event is triggered before change event" );
606+
tail();
607+
}
608+
]);
609+
}
610+
611+
asyncTest( "Small select triggers change after popup closes", function() {
612+
testChangeAfterClose( $( "#small-select-change-after-close" ),
613+
".smallSelectTriggersChangeAfterPopupCloses",
614+
{
615+
src: $( "#small-select-change-after-close-listbox" ),
616+
event: "popupafteropen"
617+
},
618+
{
619+
src: $( "#small-select-change-after-close-listbox" ),
620+
event: "popupafterclose"
621+
}, start);
622+
});
623+
624+
asyncTest( "Large select triggers change after dialog closes", function() {
625+
testChangeAfterClose( $( "#large-select-change-after-close" ),
626+
".largeSelectTriggersChangeAfterPopupCloses",
627+
{ src: $( document ), event: "pageshow" },
628+
{
629+
src: function() { return $( "#large-select-change-after-close-dialog" ); },
630+
event: "pagehide"
631+
},
632+
start);
633+
});
634+
539635
})(jQuery);

0 commit comments

Comments
 (0)