FLEX-35043 PROBLEM: The assumptions in SF_ORIG_list_events_tester.List_events_collectionKind_move made obvious another problem caused by FLEX-34885: calling itemUpdated(item) did not reposition the item according to the sorting rules (unless the Sort had a customCompareFunction).
CAUSE: FLEX-34885 assumed that the initial logic of checking for !updateEntry.property was to detect when entire objects were replaced with others in the collection. However, it was also detecting when the developer signalled that something (undefined) about an object had changed, and they wanted the sorting / filtering to be reapplied. SOLUTION: now we call moveItemInView() also when updateEntry.property is null and there are no oldValue or newValue specified in the CollectionEvent. NOTES: -also updated ListCollectionView_PropertyChangeEvent_Tests and added a new function for objects which are not bindable. As expected, there is a failure without this change to ListCollectionView, and with the change all unit tests pass. -made explicit a conversion from Object into Boolean in a call to moveItemInView(). -reformated event instantiations to be on one line. -made some minor changes to SF_ORIG_ListBasic.mxml, which is used by SF_ORIG_list_events_tester. Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/2c14851e Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/2c14851e Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/2c14851e Branch: refs/heads/develop Commit: 2c14851e9549ec390e45c0609257e10bda7cc0dd Parents: f7077c6 Author: Mihai Chira <mih...@apache.org> Authored: Thu Mar 31 15:17:18 2016 +0200 Committer: Mihai Chira <mih...@apache.org> Committed: Thu Mar 31 15:17:18 2016 +0200 ---------------------------------------------------------------------- .../framework/src/mx/collections/ArrayList.as | 3 +- .../src/mx/collections/ListCollectionView.as | 20 ++-- ...tCollectionView_PropertyChangeEvent_Tests.as | 113 ++++++++++++++++--- .../Sort/SWFs/SF_ORIG_ListBasic.mxml | 5 +- 4 files changed, 109 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/2c14851e/frameworks/projects/framework/src/mx/collections/ArrayList.as ---------------------------------------------------------------------- diff --git a/frameworks/projects/framework/src/mx/collections/ArrayList.as b/frameworks/projects/framework/src/mx/collections/ArrayList.as index dbfbb42..2a89718 100644 --- a/frameworks/projects/framework/src/mx/collections/ArrayList.as +++ b/frameworks/projects/framework/src/mx/collections/ArrayList.as @@ -620,8 +620,7 @@ public class ArrayList extends EventDispatcher oldValue:Object = null, newValue:Object = null):void { - var event:PropertyChangeEvent = - new PropertyChangeEvent(PropertyChangeEvent.PROPERTY_CHANGE); + var event:PropertyChangeEvent = new PropertyChangeEvent(PropertyChangeEvent.PROPERTY_CHANGE); event.kind = PropertyChangeEventKind.UPDATE; event.source = item; http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/2c14851e/frameworks/projects/framework/src/mx/collections/ListCollectionView.as ---------------------------------------------------------------------- diff --git a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as index 15fc15d..f03556f 100644 --- a/frameworks/projects/framework/src/mx/collections/ListCollectionView.as +++ b/frameworks/projects/framework/src/mx/collections/ListCollectionView.as @@ -1489,7 +1489,9 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, { var oldOrNewValueSpecified:Boolean = updateInfo.oldValue != null || updateInfo.newValue != null; var objectReplacedInCollection:Boolean = updateInfo.property == null && oldOrNewValueSpecified; + var somethingUnknownAboutTheObjectChanged:Boolean = updateInfo.property == null && !oldOrNewValueSpecified; updateEntry = {item: item, move: defaultMove, events: [updateInfo], + undefinedChange:somethingUnknownAboutTheObjectChanged, objectReplacedWithAnother: objectReplacedInCollection, oldItem: updateInfo.oldValue}; updatedItems.push(updateEntry); } @@ -1500,6 +1502,7 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, updateEntry.move = updateEntry.move || filterFunction != null || updateEntry.objectReplacedWithAnother + || updateEntry.undefinedChange || (sort && sort.propertyAffectsSort(String(updateInfo.property))); } @@ -1515,7 +1518,7 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, { removeItemsFromView([updateEntry.oldItem], -1, true); } - moveItemInView(updateEntry.item, updateEntry.item, eventItems); + moveItemInView(updateEntry.item, updateEntry.item != null, eventItems); } else { @@ -1648,8 +1651,7 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, pendingUpdates = null; if (dispatch) { - var refreshEvent:CollectionEvent = - new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); + var refreshEvent:CollectionEvent = new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); refreshEvent.kind = CollectionEventKind.REFRESH; dispatchEvent(refreshEvent); } @@ -1690,8 +1692,7 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, if (dispatch) { - var event:CollectionEvent = - new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); + var event:CollectionEvent = new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); event.items.push(item); if (updateEventItems && addLocation == removeLocation && addLocation > -1) { @@ -1782,8 +1783,7 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, } if (dispatch && removedItems.length > 0) { - var event:CollectionEvent = - new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); + var event:CollectionEvent = new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); event.kind = CollectionEventKind.REMOVE; event.location = (!localIndex || removedItems.length == 1) ? removeLocation @@ -1823,8 +1823,7 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, } else { - var event:CollectionEvent = - new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); + var event:CollectionEvent = new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); event.kind = CollectionEventKind.REPLACE; event.location = location; event.items = changeEvents; @@ -1841,8 +1840,7 @@ public class ListCollectionView extends Proxy implements ICollectionView, IList, internalRefresh(false); if (dispatchResetEvent) { - var event:CollectionEvent = - new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); + var event:CollectionEvent = new CollectionEvent(CollectionEvent.COLLECTION_CHANGE); event.kind = CollectionEventKind.RESET; dispatchEvent(event); } http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/2c14851e/frameworks/projects/framework/tests/mx/collections/ListCollectionView_PropertyChangeEvent_Tests.as ---------------------------------------------------------------------- diff --git a/frameworks/projects/framework/tests/mx/collections/ListCollectionView_PropertyChangeEvent_Tests.as b/frameworks/projects/framework/tests/mx/collections/ListCollectionView_PropertyChangeEvent_Tests.as index 56d0b40..a51827a 100644 --- a/frameworks/projects/framework/tests/mx/collections/ListCollectionView_PropertyChangeEvent_Tests.as +++ b/frameworks/projects/framework/tests/mx/collections/ListCollectionView_PropertyChangeEvent_Tests.as @@ -22,6 +22,8 @@ package mx.collections { import flash.events.UncaughtErrorEvent; import mx.core.FlexGlobals; + import mx.events.CollectionEvent; + import mx.events.CollectionEventKind; import mx.events.PropertyChangeEvent; import mx.events.PropertyChangeEventKind; import mx.utils.ObjectUtil; @@ -32,6 +34,8 @@ package mx.collections { public class ListCollectionView_PropertyChangeEvent_Tests { + private static const NO_WORKOUTS:int = 10; + private static var _sut:ListCollectionView; private static var _firstWorkout:WorkoutVO; @@ -76,6 +80,7 @@ package mx.collections { { _sut = null; _lastFilteredObject = null; + _firstWorkout = null; PROPERTY_CHANGE_EVENT = null; PROPERTY_CHANGE_EVENT_UPDATE = null; @@ -136,7 +141,7 @@ package mx.collections { assertEquals(positionOfFirstWorkout, _sut.getItemIndex(_firstWorkout)); } - [Test] //FLEX-35043 + [Test] //FLEX-35043 FLEX-34885 public function test_when_collection_item_dispatches_PropertyChangeEvent_null_is_not_removed_from_list():void { //given @@ -174,7 +179,7 @@ package mx.collections { assertEquals(positionOfFirstWorkout, _sut.getItemIndex(_firstWorkout)); } - [Test] //FLEX-35043 + [Test] //FLEX-35043 FLEX-34885 public function test_when_item_is_changed_bypassing_binding_and_collection_notified_of_itemUpdated_with_property_null_is_not_removed_from_list():void { //given @@ -194,7 +199,7 @@ package mx.collections { assertEquals(positionOfFirstWorkout, _sut.getItemIndex(_firstWorkout)); } - [Test] //FLEX-35043 + [Test] //FLEX-35043 FLEX-34885 public function test_when_item_is_changed_bypassing_binding_and_collection_notified_of_itemUpdated_without_property_null_is_not_removed_from_list():void { //given @@ -215,7 +220,7 @@ package mx.collections { assertEquals(positionOfFirstWorkout, _sut.getItemIndex(_firstWorkout)); } - [Test] //FLEX-35043 + [Test] //FLEX-35043 FLEX-34885 public function test_when_collection_item_dispatches_PropertyChangeEvent_with_UPDATE_null_is_not_removed_from_list():void { //given @@ -275,7 +280,7 @@ package mx.collections { assertEquals(positionOfFirstWorkout, _sut.getItemIndex(_firstWorkout)); } - [Test] //FLEX-35043 + [Test] //FLEX-35043 FLEX-34885 public function test_when_collection_item_dispatches_PropertyChangeEvent_item_is_added_in_correct_place_based_on_sort_and_there_is_no_fatal():void { //given @@ -296,7 +301,7 @@ package mx.collections { assertEquals(positionOfFirstWorkout, _sut.getItemIndex(_firstWorkout)); } - [Test] //FLEX-35043 + [Test] //FLEX-35043 FLEX-34885 public function test_when_collection_item_dispatches_PropertyChangeEvent_sort_compare_function_not_called_with_null():void { function compareWorkouts(a:Object, b:Object, fields:Array = null):int @@ -320,30 +325,91 @@ package mx.collections { assertNull(_uncaughtError); } - [Test] //FLEX-35043 - public function test_when_collection_notified_of_itemUpdated_without_property_or_oldValue_sort_compare_function_not_called_with_null():void + [Test] //FLEX-35043 FLEX-34885 + public function test_when_collection_notified_of_itemUpdated_without_property_or_oldValue_sort_compare_function_not_called_with_null_and_item_repositioned():void { - function compareWorkouts(a:Object, b:Object, fields:Array = null):int + function orderByDurationDescending(a:Object, b:Object, fields:Array = null):int { if(a.duration > b.duration) - return 1; - if(a.duration < b.duration) return -1; + if(a.duration < b.duration) + return 1; return 0; } + + //make sure the workout is on the first position + assertEquals("Initial assumption wrong - position", 0, _sut.getItemIndex(_firstWorkout)); + assertEquals("Initial assumption wrong - duration", 0, _firstWorkout.duration); + //given - var sort:InspectableSort = new InspectableSort([], compareWorkouts); + var sort:InspectableSort = new InspectableSort([], orderByDurationDescending); _sut.sort = sort; _sut.refresh(); + //then + assertEquals("The first item should have the highest duration", NO_WORKOUTS - 1, WorkoutVO(_sut.getItemAt(0)).duration); + assertEquals("The first workout should have been moved to the end of the list", _sut.length - 1, _sut.getItemIndex(_firstWorkout)); + //when _firstWorkout.setMuscleGroupsWithoutTriggeringBinding("chest"); _firstWorkout.setMinAgeWithoutTriggeringBinding(20); + _firstWorkout.setDurationWithoutTriggeringBinding(int.MAX_VALUE); _sut.itemUpdated(_firstWorkout); - //then - no fatal because compareWorkouts was not called with null + //then + // 1. no fatal because compare function was not called with null assertNull(_uncaughtError); + // 2. the workout was repositioned because its duration changed + assertEquals("The workout should have been moved to the start of the list, as it has the highest duration", 0, _sut.getItemIndex(_firstWorkout)); + } + + [Test] //FLEX-35043 FLEX-34885 + public function test_when_collection_of_non_binding_objects_notified_of_itemUpdated_without_property_or_oldValue_item_is_repositioned():void + { + function onCollectionChange(event:CollectionEvent):void + { + moveDispatched = event.kind == CollectionEventKind.MOVE; + } + + function orderByDurationAscending(a:Object, b:Object, fields:Array = null):int + { + if(a.duration > b.duration) + return 1; + if(a.duration < b.duration) + return -1; + + return 0; + } + + //given + var moveDispatched:Boolean = false; + + var testWorkout:Object = {duration:10}; + _sut.removeAll(); + _sut.addAll(new ArrayList([testWorkout, {duration:20}, {duration:5}])); + + var sortField:SortField = new SortField("duration"); + sortField.sortCompareType = SortFieldCompareTypes.NUMERIC; + var sort:InspectableSort = new InspectableSort([sortField]); + _sut.sort = sort; + _sut.refresh(); + + _sut.addEventListener(CollectionEvent.COLLECTION_CHANGE, onCollectionChange); + + //then + assertEquals("The ten minute workout should be in the second position", 1, _sut.getItemIndex(testWorkout)); + + //when + testWorkout.duration = int.MAX_VALUE; + _sut.itemUpdated(testWorkout); + + //then + // 1. no fatal because compare function was not called with null + assertNull(_uncaughtError); + // 2. the workout was repositioned because its duration changed + assertTrue(moveDispatched); + assertEquals("The workout should have been moved to the end of the list, as it now has the highest duration", _sut.length - 1, _sut.getItemIndex(testWorkout)); } private static function allowAll(object:Object):Boolean @@ -356,7 +422,7 @@ package mx.collections { private static function createWorkouts():IList { var result:ArrayList = new ArrayList(); - for (var i:int = 0; i < 10; i++) + for (var i:int = 0; i < NO_WORKOUTS; i++) { result.addItem(new WorkoutVO("Workout" + i, i)); } @@ -380,15 +446,15 @@ import spark.collections.Sort; [Bindable] class WorkoutVO { - public var duration:int; public var name:String; + private var _duration:int; private var _muscleGroups:String; private var _minAge:uint; public function WorkoutVO(name:String, duration:int) { this.name = name; - this.duration = duration; + this._duration = duration; } public function get muscleGroups():String @@ -420,6 +486,21 @@ class WorkoutVO { _minAge = age; } + + public function setDurationWithoutTriggeringBinding(duration:int):void + { + _duration = duration; + } + + public function get duration():int + { + return _duration; + } + + public function set duration(value:int):void + { + _duration = value; + } } class InspectableSort extends Sort http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/2c14851e/mustella/tests/spark/collections/Sort/SWFs/SF_ORIG_ListBasic.mxml ---------------------------------------------------------------------- diff --git a/mustella/tests/spark/collections/Sort/SWFs/SF_ORIG_ListBasic.mxml b/mustella/tests/spark/collections/Sort/SWFs/SF_ORIG_ListBasic.mxml index 46cb932..8f38745 100644 --- a/mustella/tests/spark/collections/Sort/SWFs/SF_ORIG_ListBasic.mxml +++ b/mustella/tests/spark/collections/Sort/SWFs/SF_ORIG_ListBasic.mxml @@ -247,8 +247,7 @@ var sortField:SortField = new SortField("value",false); this.addStyleClient(sort); this.addStyleClient(sortField); -// var sort:mx.collections.Sort = new mx.collections.Sort(); -// var sortField:mx.collections.SortField = new mx.collections.SortField("value",true); + sort.fields = [sortField]; ArrayCollection(list1.dataProvider).sort = sort; ArrayCollection(list1.dataProvider).refresh(); @@ -259,7 +258,7 @@ lb.text = to.value; to.value = "yello1"; - list1.dataProvider.itemUpdated(list1.dataProvider.getItemAt(0)); + list1.dataProvider.itemUpdated(to); return 1; } ]]>
