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

Commit bfb17bb

Browse files
author
Gabriel Schulhof
committed
Listview: Review refresh() method
1. Fix signature of public refresh() method 2. Move sparing _refresh() decision to backcompat The decision is delegated to extension points (one for the list item and one the anchor(s) it may contain) which are predicates that always return true if the backcompat extension is absent, indicating for every item that it is to be processed. When the backcompat extension is present, the extension points contain the class-examining regex-based decision of previous versions. 3. Add method updateItems() In _refresh() we still need to retrieve all list items, even when we get a list of items from updateItems(), because the followings require global knowledge: - Which items have been removed since last time, so we can untrack them - Which is the first resp. last item for use with addFirstLastClasses() Closes gh-8489 Fixes gh-8334
1 parent a94c5d9 commit bfb17bb

File tree

7 files changed

+196
-49
lines changed

7 files changed

+196
-49
lines changed

js/widgets/listview.backcompat.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,22 @@
3232
} )( function( $ ) {
3333

3434
if ( $.mobileBackcompat !== false ) {
35+
var listviewItemClassRegex = /\bui-listview-item-static\b|\bui-listview-item-divider\b/;
36+
var buttonClassRegex = /\bui-button\b/;
37+
3538
$.widget( "mobile.listview", $.mobile.listview, {
3639
options: {
3740
corners: true,
3841
shadow: true
3942
},
40-
classProp: "ui-listview-inset"
43+
classProp: "ui-listview-inset",
44+
_processListItem: function( item ) {
45+
return !listviewItemClassRegex.test( item[ 0 ].className );
46+
},
47+
48+
_processListItemAnchor: function( a ) {
49+
return !buttonClassRegex.test( a[ 0 ].className );
50+
}
4151
} );
4252
$.widget( "mobile.listview", $.mobile.listview, $.mobile.widget.backcompat );
4353
}

js/widgets/listview.js

Lines changed: 68 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,7 @@ function addItemToDictionary( itemClassDict, element, key, extra ) {
4343
}
4444

4545
var getAttribute = $.mobile.getAttribute,
46-
countBubbleClassRegex = /\bui-listview-item-count-bubble\b/,
47-
listviewItemClassRegex = /\bui-listview-item-static\b|\bui-listview-item-divider\b/,
48-
buttonClassRegex = /\bui-button\b/;
46+
countBubbleClassRegex = /\bui-listview-item-count-bubble\b/;
4947

5048
function filterBubbleSpan() {
5149
var child, parentNode,
@@ -94,7 +92,7 @@ return $.widget( "mobile.listview", $.extend( {
9492
if ( this.options.inset ) {
9593
this._addClass( "ui-listview-inset" );
9694
}
97-
this.refresh( true );
95+
this._refresh( true );
9896
},
9997

10098
// We only handle the theme option through the theme extension. Theme options concerning list
@@ -131,40 +129,55 @@ return $.widget( "mobile.listview", $.extend( {
131129
_beforeListviewRefresh: $.noop,
132130
_afterListviewRefresh: $.noop,
133131

134-
refresh: function( create ) {
132+
updateItems: function( items ) {
133+
this._refresh( false, items );
134+
},
135+
136+
refresh: function() {
137+
this._refresh();
138+
},
139+
140+
_processListItem: function( /* item */ ) {
141+
return true;
142+
},
143+
144+
_processListItemAnchor: function( /* a */ ) {
145+
return true;
146+
},
147+
148+
_refresh: function( create, items ) {
135149
var buttonClass, pos, numli, item, itemClass, itemExtraClass, itemTheme, itemIcon, icon, a,
136-
isDivider, startCount, newStartCount, value, last, splittheme, splitThemeClass, li, ol,
137-
altButtonClass, dividerTheme, start, itemClassDict, dictionaryKey, span, spliticon,
150+
isDivider, value, last, splittheme, li, dictionaryKey, span, allItems, newSpan,
138151
currentOptions = this.options,
139-
createEnhanced = create && this.options.enhanced,
140-
list = this.element;
141-
142-
ol = !!$.nodeName( list[ 0 ], "ol" );
143-
start = list.attr( "start" );
144-
itemClassDict = {};
152+
list = this.element,
153+
ol = !!$.nodeName( list[ 0 ], "ol" ),
154+
start = list.attr( "start" ),
155+
itemClassDict = {};
145156

146157
// Check if a start attribute has been set while taking a value of 0 into account
147158
if ( ol && ( start || start === 0 ) ) {
148-
startCount = parseInt( start, 10 ) - 1;
149-
list.css( "counter-reset", "listnumbering " + startCount );
159+
list.css( "counter-reset", "listnumbering " + ( parseInt( start, 10 ) - 1 ) );
150160
}
151161

152162
this._beforeListviewRefresh();
153163

154-
li = this._getChildrenByTagName( list[ 0 ], "li", "LI" );
164+
// We need all items even if a set was passed in - we just won't iterate over them in the
165+
// main refresh loop.
166+
allItems = this._getChildrenByTagName( list[ 0 ], "li", "LI" );
167+
li = items || allItems;
155168

156169
for ( pos = 0, numli = li.length; pos < numli; pos++ ) {
157170
item = li.eq( pos );
158171
itemClass = "ui-listview-item";
159172
itemExtraClass = undefined;
160173

161-
if ( create || !listviewItemClassRegex.test( item[ 0 ].className ) ) {
174+
if ( create || this._processListItem( item ) ) {
162175
a = this._getChildrenByTagName( item[ 0 ], "a", "A" );
163176
isDivider = ( getAttribute( item[ 0 ], "role" ) === "list-divider" );
164177
value = item.attr( "value" );
165178
itemTheme = getAttribute( item[ 0 ], "theme" );
166179

167-
if ( a.length && ( ( !buttonClassRegex.test( a[ 0 ].className ) && !isDivider ) ||
180+
if ( a.length && ( ( this._processListItemAnchor( a ) && !isDivider ) ||
168181
create ) ) {
169182
itemIcon = getAttribute( item[ 0 ], "icon" );
170183
icon = ( itemIcon === false ) ? false : ( itemIcon || currentOptions.icon );
@@ -176,35 +189,48 @@ return $.widget( "mobile.listview", $.extend( {
176189
}
177190

178191
if ( a.length > 1 ) {
179-
itemClass = "ui-listview-item-has-alternate";
192+
itemClass += " ui-listview-item-has-alternate";
180193

181194
last = a.last();
182195
splittheme = getAttribute( last[ 0 ], "theme" ) ||
183-
currentOptions.splitTheme || getAttribute( item[ 0 ], "theme", true );
184-
splitThemeClass = splittheme ? " ui-button-" + splittheme : "";
185-
spliticon = getAttribute( last[ 0 ], "icon" ) ||
186-
getAttribute( item[ 0 ], "icon" ) || currentOptions.splitIcon;
187-
altButtonClass = "ui-button ui-button-icon-only" + splitThemeClass;
188-
189-
span = createEnhanced ? last.children( ".ui-listview-item-split-icon" ) :
190-
$( "<span>" );
196+
currentOptions.splitTheme || itemTheme;
197+
198+
newSpan = false;
199+
span = last.children( ".ui-listview-item-split-icon" );
200+
if ( !span.length ) {
201+
span = $( "<span>" );
202+
newSpan = true;
203+
}
204+
191205
addItemToDictionary( itemClassDict, span[ 0 ],
192-
"ui-listview-item-split-icon", "ui-icon ui-icon-" + spliticon );
206+
"ui-listview-item-split-icon", "ui-icon ui-icon-" +
207+
( getAttribute( last[ 0 ], "icon" ) || itemIcon ||
208+
currentOptions.splitIcon ) );
193209
addItemToDictionary( itemClassDict, last[ 0 ],
194-
"ui-listview-item-split-button", altButtonClass );
210+
"ui-listview-item-split-button",
211+
"ui-button ui-button-icon-only" +
212+
( splittheme ? " ui-button-" + splittheme : "" ) );
195213
last.attr( "title", $.trim( last.getEncodedText() ) );
196-
if ( !createEnhanced ) {
214+
215+
if ( newSpan ) {
197216
last.empty().prepend( span );
198217
}
199218

200219
// Reduce to the first anchor, because only the first gets the buttonClass
201220
a = a.first();
202221
} else if ( icon ) {
203-
span = createEnhanced ? a.children( ".ui-listview-item-icon" ) :
204-
$( "<span>" );
222+
223+
newSpan = false;
224+
span = a.children( ".ui-listview-item-icon" );
225+
if ( !span.length ) {
226+
span = $( "<span>" );
227+
newSpan = true;
228+
}
229+
205230
addItemToDictionary( itemClassDict, span[ 0 ], "ui-listview-item-icon",
206231
"ui-icon ui-icon-" + icon + " ui-widget-icon-floatend" );
207-
if ( !createEnhanced ) {
232+
233+
if ( newSpan ) {
208234
a.prepend( span );
209235
}
210236
}
@@ -213,21 +239,17 @@ return $.widget( "mobile.listview", $.extend( {
213239
addItemToDictionary( itemClassDict, a[ 0 ], "ui-listview-item-button",
214240
buttonClass );
215241
} else if ( isDivider ) {
216-
dividerTheme = ( getAttribute( item[ 0 ], "theme" ) ||
217-
currentOptions.dividerTheme || currentOptions.theme );
218-
219-
itemClass = "ui-listview-item-divider";
220-
itemExtraClass = "ui-bar-" + ( dividerTheme ? dividerTheme : "inherit" );
242+
itemClass += " ui-listview-item-divider";
243+
itemExtraClass = "ui-bar-" + ( itemTheme || currentOptions.dividerTheme ||
244+
currentOptions.theme || "inherit" );
221245

222246
item.attr( "role", "heading" );
223247
} else if ( a.length <= 0 ) {
224-
itemClass = "ui-listview-item-static";
248+
itemClass += " ui-listview-item-static";
225249
itemExtraClass = "ui-body-" + ( itemTheme ? itemTheme : "inherit" );
226250
}
227251
if ( ol && value ) {
228-
newStartCount = parseInt( value, 10 ) - 1;
229-
230-
item.css( "counter-reset", "listnumbering " + newStartCount );
252+
item.css( "counter-reset", "listnumbering " + ( parseInt( value, 10 ) - 1 ) );
231253
}
232254
}
233255

@@ -259,7 +281,9 @@ return $.widget( "mobile.listview", $.extend( {
259281

260282
this._afterListviewRefresh();
261283

262-
this._addFirstLastClasses( li, this._getVisibles( li, create ), create );
284+
// NOTE: Using the extension addFirstLastClasses is deprecated as of 1.5.0 and this and the
285+
// extension itself will be removed in 1.6.0.
286+
this._addFirstLastClasses( allItems, this._getVisibles( allItems, create ), create );
263287

264288
// Untrack removed items
265289
if ( this._oldListItems ) {
@@ -269,7 +293,7 @@ return $.widget( "mobile.listview", $.extend( {
269293
} ),
270294
"ui-listview-item ui-listview-item-static ui-listview-item-has-count " +
271295
"ui-listview-item-has-alternate ui-listview-item-divider" );
272-
this._oldListItems = li;
296+
this._oldListItems = allItems;
273297
}
274298
}
275299
}, $.mobile.behaviors.addFirstLastClasses ) );

tests/integration/listview/backcompat-tests.html

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@
3434
<li>Item 2</li>
3535
</ul>
3636
</div>
37+
38+
<ul id="sparing-check" data-nstest-role="listview">
39+
<li id="sparing-check-first-item">Item 1</li>
40+
<li id="sparing-check-second-item">Item 2</li>
41+
</ul>
3742
</div>
3843
</body>
3944
</html>

tests/integration/listview/backcompat_core.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
define( [
22
"qunit",
3-
"jquery"
4-
], function( QUnit, $ ) {
3+
"jquery",
4+
"../tests/integration/listview/sparing-setup-teardown"
5+
], function( QUnit, $, sparingSetupTeardown ) {
56

67
var helper = $.testHelper;
78

@@ -106,4 +107,17 @@ QUnit.test( "Turning on class ui-shadow", function( assert ) {
106107
"Option shadow is true whenever ui-shadow is present in ui-listview-inset" );
107108
} );
108109

110+
QUnit.module( "Sparingness of refresh()", sparingSetupTeardown );
111+
112+
QUnit.test( "refresh()", function( assert ) {
113+
assert.expect( 2 );
114+
115+
this.listview.refresh();
116+
117+
assert.strictEqual( this.calls[ 1 ][ 0 ].length, 1,
118+
"One element was passed to _addClass()" );
119+
assert.deepEqual( this.calls[ 1 ][ 0 ].get( 0 ), this.newItem[ 0 ],
120+
"The single item in the call is as expected" );
121+
} );
122+
109123
} );

tests/integration/listview/index.html

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,21 @@
180180
</a>
181181
</li>
182182
</ul>
183+
184+
<ul data-ui-role="listview" id="icon-refresh">
185+
<li data-ui-icon="home"><a href="#">Home</a></li>
186+
<li data-ui-icon="delete"><a href="#">Delete</a></li>
187+
</ul>
188+
189+
<ul data-ui-role="listview" id="icon-update-items">
190+
<li data-ui-icon="home"><a href="#">Home</a></li>
191+
<li data-ui-icon="delete"><a href="#">Delete</a></li>
192+
</ul>
193+
194+
<ul data-ui-role="listview" id="sparing-check">
195+
<li id="sparing-check-first-item">Item 1</li>
196+
<li id="sparing-check-second-item">Item 2</li>
197+
</ul>
183198
</div>
184199
</div>
185200

tests/integration/listview/listview_core.js

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
// TODO split out into separate test files
66
define( [
77
"qunit",
8-
"jquery"
9-
], function( QUnit, $ ) {
8+
"jquery",
9+
"../tests/integration/listview/sparing-setup-teardown"
10+
], function( QUnit, $, sparingSetupTeardown ) {
1011

1112
QUnit.module( "Basic Linked list" );
1213

@@ -458,4 +459,58 @@ QUnit.test( "basic pre-enhanced listview", function( assert ) {
458459
"Listview refresh() enhances new items" );
459460
} );
460461

462+
QUnit.module( "Sparingness of refresh() and updateItems()", sparingSetupTeardown );
463+
464+
QUnit.test( "refresh()", function( assert ) {
465+
assert.expect( 5 );
466+
467+
this.listview.refresh();
468+
469+
assert.strictEqual( this.calls[ 0 ][ 0 ].length, 3,
470+
"Three elements were passed to _addClass()" );
471+
assert.strictEqual( this.calls[ 0 ][ 1 ], "ui-listview-item ui-listview-item-static",
472+
"Class ui-listview-item-static was assigned" );
473+
assert.strictEqual( this.calls[ 0 ][ 0 ].first().attr( "id" ), "sparing-check-first-item",
474+
"The first item in the call is as expected" );
475+
assert.deepEqual( this.calls[ 0 ][ 0 ].get( 1 ), this.newItem[ 0 ],
476+
"The second item in the call is as expected" );
477+
assert.strictEqual( this.calls[ 0 ][ 0 ].last().attr( "id" ), "sparing-check-second-item",
478+
"The last item in the call is as expected" );
479+
} );
480+
481+
QUnit.test( "updateItems()", function( assert ) {
482+
assert.expect( 2 );
483+
484+
this.listview.updateItems( this.newItem );
485+
486+
assert.strictEqual( this.calls[ 0 ][ 0 ].length, 1,
487+
"One element was passed to _addClass()" );
488+
assert.deepEqual( this.calls[ 0 ][ 0 ].get( 0 ), this.newItem[ 0 ],
489+
"The single item in the call is as expected" );
490+
} );
491+
492+
QUnit.module( "Dynamic icon update" );
493+
494+
QUnit.test( "via refresh()", function( assert ) {
495+
var listview = $( "#icon-refresh" );
496+
497+
listview.children().first().attr( "data-ui-icon", "back" );
498+
listview.listview( "refresh" );
499+
500+
assert.hasClasses( listview.children().first().find( ".ui-listview-item-icon" ),
501+
"ui-icon-back",
502+
"Listview item icon is as expected" );
503+
} );
504+
505+
QUnit.test( "via updateItems()", function( assert ) {
506+
var listview = $( "#icon-update-items" );
507+
508+
listview.children().first().attr( "data-ui-icon", "back" );
509+
listview.listview( "updateItems", listview.children().first() );
510+
511+
assert.hasClasses( listview.children().first().find( ".ui-listview-item-icon" ),
512+
"ui-icon-back",
513+
"Listview item icon is as expected" );
514+
} );
515+
461516
} );
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
define( [ "jquery" ], function( $ ) {
2+
3+
return {
4+
setup: function() {
5+
var that = this;
6+
7+
this.template = $( "#sparing-check" ).clone();
8+
this.listview = $( "#sparing-check" ).listview().listview( "instance" );
9+
this.newItem = $( "<li>Something</li>" ).insertAfter( "#sparing-check-first-item" );
10+
this.calls = [];
11+
this._addClass = $.mobile.listview.prototype._addClass;
12+
13+
$.mobile.listview.prototype._addClass = function() {
14+
that.calls.push( arguments );
15+
return that._addClass.apply( this, arguments );
16+
};
17+
},
18+
teardown: function() {
19+
$( "#sparing-check" ).before( this.template ).remove();
20+
$.mobile.listview.prototype._addClass = this._addClass;
21+
}
22+
};
23+
24+
} );

0 commit comments

Comments
 (0)