From 08e0e27f7eb1aad2fd073ed7a27638b6fd8004d9 Mon Sep 17 00:00:00 2001 From: Josh Tynjala Date: Mon, 28 Oct 2024 16:03:57 -0700 Subject: [PATCH] PageNavigator, TabNavigator: fix setting dataProvider to new collection resulting in wrong selectedItem --- .../controls/navigators/PageNavigator.hx | 13 ++----- .../controls/navigators/TabNavigator.hx | 13 ++----- .../controls/navigators/PageNavigatorTest.hx | 39 +++++++++++++++++++ .../controls/navigators/TabNavigatorTest.hx | 39 +++++++++++++++++++ 4 files changed, 86 insertions(+), 18 deletions(-) diff --git a/src/feathers/controls/navigators/PageNavigator.hx b/src/feathers/controls/navigators/PageNavigator.hx index a9f0252f..4eddf910 100644 --- 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 index a20346b3..a8dfe4a0 100644 --- 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 index 699b6ff5..8d7c53d3 100644 --- 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 index f5936dbd..75bf2029 100644 --- 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 = () -> {