PageNavigator, TabNavigator: fix setting dataProvider to new collecti…

…on resulting in wrong selectedItem
feathersui · Oct 28, 2024 · 08e0e27 · 08e0e27
1 parent ab47127
commit 08e0e27
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 18 deletions.
diff --git a/src/feathers/controls/navigators/PageNavigator.hx b/src/feathers/controls/navigators/PageNavigator.hx
@@ -175,9 +175,8 @@ class PageNavigator extends BaseNavigator implements IIndexSelector implements I
 	}
 
 	private function set_selectedIndex(value:Int):Int {
-		var currentSelectedIndex = this._pendingSelectedIndex != NO_PENDING_SELECTED_INDEX ? this._pendingSelectedIndex : this._selectedIndex;
-		if (currentSelectedIndex == value) {
-			return currentSelectedIndex;
+		if (this._pendingSelectedIndex != NO_PENDING_SELECTED_INDEX && this._pendingSelectedIndex == value) {
+			return this._pendingSelectedIndex;
 		}
 		this._pendingSelectedIndex = value;
 		this._pendingSelectedItem = NO_PENDING_SELECTED_ITEM;
@@ -219,12 +218,8 @@ class PageNavigator extends BaseNavigator implements IIndexSelector implements I
 	}
 
 	private function set_selectedItem(value:#if flash Dynamic #else PageItem #end):#if flash Dynamic #else PageItem #end {
-		var currentSelectedItem:Any = this._pendingSelectedItem;
-		if (currentSelectedItem == NO_PENDING_SELECTED_ITEM) {
-			currentSelectedItem = this._selectedItem;
-		}
-		if (currentSelectedItem == value) {
-			return cast(currentSelectedItem, PageItem);
+		if (this._pendingSelectedItem != NO_PENDING_SELECTED_ITEM && this._pendingSelectedItem == value) {
+			return cast(this._pendingSelectedItem, PageItem);
 		}
 		this._pendingSelectedItem = value;
 		this._pendingSelectedIndex = NO_PENDING_SELECTED_INDEX;

diff --git a/src/feathers/controls/navigators/TabNavigator.hx b/src/feathers/controls/navigators/TabNavigator.hx
@@ -176,9 +176,8 @@ class TabNavigator extends BaseNavigator implements IIndexSelector implements ID
 	}
 
 	private function set_selectedIndex(value:Int):Int {
-		var currentSelectedIndex = this._pendingSelectedIndex != NO_PENDING_SELECTED_INDEX ? this._pendingSelectedIndex : this._selectedIndex;
-		if (currentSelectedIndex == value) {
-			return currentSelectedIndex;
+		if (this._pendingSelectedIndex != NO_PENDING_SELECTED_INDEX && this._pendingSelectedIndex == value) {
+			return this._pendingSelectedIndex;
 		}
 		this._pendingSelectedIndex = value;
 		this._pendingSelectedItem = NO_PENDING_SELECTED_ITEM;
@@ -220,12 +219,8 @@ class TabNavigator extends BaseNavigator implements IIndexSelector implements ID
 	}
 
 	private function set_selectedItem(value:#if flash Dynamic #else TabItem #end):#if flash Dynamic #else TabItem #end {
-		var currentSelectedItem:Any = this._pendingSelectedItem;
-		if (currentSelectedItem == NO_PENDING_SELECTED_ITEM) {
-			currentSelectedItem = this._selectedItem;
-		}
-		if (currentSelectedItem == value) {
-			return cast(currentSelectedItem, TabItem);
+		if (this._pendingSelectedItem != NO_PENDING_SELECTED_ITEM && this._pendingSelectedItem == value) {
+			return cast(this._pendingSelectedItem, TabItem);
 		}
 		this._pendingSelectedItem = value;
 		this._pendingSelectedIndex = NO_PENDING_SELECTED_INDEX;

diff --git a/test/src/feathers/controls/navigators/PageNavigatorTest.hx b/test/src/feathers/controls/navigators/PageNavigatorTest.hx
@@ -156,6 +156,45 @@ class PageNavigatorTest extends Test {
 		Assert.isNull(dispatchedChangeSelectedItem);
 	}
 
+	public function testValidateWithDataProviderThenNewDataProvider():Void {
+		var dispatchedChangeSelectedIndex = -2;
+		var dispatchedChangeSelectedItem = {};
+		var dispatchedChangeCount = 0;
+		this._navigator.addEventListener(Event.CHANGE, function(event:Event):Void {
+			dispatchedChangeCount++;
+			dispatchedChangeSelectedIndex = this._navigator.selectedIndex;
+			dispatchedChangeSelectedItem = this._navigator.selectedItem;
+		});
+
+		var one = new LayoutGroup();
+		var two = new LayoutGroup();
+		var pageOne = PageItem.withDisplayObject(one);
+		var pageTwo = PageItem.withDisplayObject(two);
+		this._navigator.dataProvider = new ArrayCollection([pageOne, pageTwo]);
+		this._navigator.validateNow();
+		Assert.equals(0, this._navigator.selectedIndex);
+		Assert.equals(pageOne, this._navigator.selectedItem);
+		Assert.equals(one, this._navigator.activeItemView);
+		Assert.equals(1, dispatchedChangeCount);
+		Assert.equals(0, dispatchedChangeSelectedIndex);
+		Assert.equals(pageOne, dispatchedChangeSelectedItem);
+
+		var three = new LayoutGroup();
+		var four = new LayoutGroup();
+		var pageThree = PageItem.withDisplayObject(three);
+		var pageFour = PageItem.withDisplayObject(four);
+		this._navigator.dataProvider = new ArrayCollection([pageThree, pageFour]);
+		this._navigator.validateNow();
+		Assert.equals(0, this._navigator.selectedIndex);
+		Assert.equals(pageThree, this._navigator.selectedItem);
+		Assert.equals(three, this._navigator.activeItemView);
+		// setting data provider when a view is active clears that view,
+		// so there are two changes and not only one
+		Assert.equals(3, dispatchedChangeCount);
+		Assert.equals(0, dispatchedChangeSelectedIndex);
+		Assert.equals(pageThree, dispatchedChangeSelectedItem);
+	}
+
 	public function testPageIndicatorDefaultVariant():Void {
 		var pageIndicator:PageIndicator = null;
 		this._navigator.pageIndicatorFactory = () -> {

diff --git a/test/src/feathers/controls/navigators/TabNavigatorTest.hx b/test/src/feathers/controls/navigators/TabNavigatorTest.hx
@@ -156,6 +156,45 @@ class TabNavigatorTest extends Test {
 		Assert.isNull(dispatchedChangeSelectedItem);
 	}
 
+	public function testValidateWithDataProviderThenNeweDataProvider():Void {
+		var dispatchedChangeSelectedIndex = -2;
+		var dispatchedChangeSelectedItem = {};
+		var dispatchedChangeCount = 0;
+		this._navigator.addEventListener(Event.CHANGE, function(event:Event):Void {
+			dispatchedChangeCount++;
+			dispatchedChangeSelectedIndex = this._navigator.selectedIndex;
+			dispatchedChangeSelectedItem = this._navigator.selectedItem;
+		});
+
+		var one = new LayoutGroup();
+		var two = new LayoutGroup();
+		var tabOne = TabItem.withDisplayObject("one", one);
+		var tabTwo = TabItem.withDisplayObject("two", two);
+		this._navigator.dataProvider = new ArrayCollection([tabOne, tabTwo]);
+		this._navigator.validateNow();
+		Assert.equals(0, this._navigator.selectedIndex);
+		Assert.equals(tabOne, this._navigator.selectedItem);
+		Assert.equals(one, this._navigator.activeItemView);
+		Assert.equals(1, dispatchedChangeCount);
+		Assert.equals(0, dispatchedChangeSelectedIndex);
+		Assert.equals(tabOne, dispatchedChangeSelectedItem);
+
+		var three = new LayoutGroup();
+		var four = new LayoutGroup();
+		var tabThree = TabItem.withDisplayObject("three", three);
+		var tabFour = TabItem.withDisplayObject("four", four);
+		this._navigator.dataProvider = new ArrayCollection([tabThree, tabFour]);
+		this._navigator.validateNow();
+		Assert.equals(0, this._navigator.selectedIndex);
+		Assert.equals(tabThree, this._navigator.selectedItem);
+		Assert.equals(three, this._navigator.activeItemView);
+		// setting data provider when a view is active clears that view,
+		// so there are two changes and not only one
+		Assert.equals(3, dispatchedChangeCount);
+		Assert.equals(0, dispatchedChangeSelectedIndex);
+		Assert.equals(tabThree, dispatchedChangeSelectedItem);
+	}
+
 	public function testTabBarDefaultVariant():Void {
 		var tabBar:TabBar = null;
 		this._navigator.tabBarFactory = () -> {