fix(android): Fix setting an active tab for TabGroup before opening. #10740
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| /* | ||
| * Appcelerator Titanium Mobile | ||
| * Copyright (c) 2011-Present by Appcelerator, Inc. All Rights Reserved. | ||
| * Licensed under the terms of the Apache Public License | ||
| * Please see the LICENSE included with this distribution for details. | ||
| */ | ||
| /* eslint-env mocha */ | ||
| /* global Ti */ | ||
| /* eslint no-unused-expressions: "off" */ | ||
| 'use strict'; | ||
| const should = require('./utilities/assertions'); // eslint-disable-line no-unused-vars | ||
|
|
||
| // skipping many test on Windows due to lack of event firing, see https://jira.appcelerator.org/browse/TIMOB-26690 | ||
| describe('Titanium.UI.TabGroup', () => { | ||
| let tabGroup; | ||
| let tab; | ||
|
|
||
| afterEach(() => { | ||
| if (tabGroup) { | ||
| if (tab) { | ||
| tabGroup.removeTab(tab); | ||
| } | ||
| tabGroup.close(); | ||
| tabGroup = null; | ||
| } | ||
| tab = null; | ||
| }); | ||
|
|
||
| it.windowsBroken('#setActiveTab()', finish => { | ||
| const winA = Ti.UI.createWindow(), | ||
| tabA = Ti.UI.createTab({ | ||
| title: 'Tab A', | ||
| window: winA | ||
| }), | ||
| winB = Ti.UI.createWindow(), | ||
| tabB = Ti.UI.createTab({ | ||
| title: 'Tab B', | ||
| window: winB | ||
| }); | ||
| tabGroup = Ti.UI.createTabGroup(); | ||
|
|
||
| // Does windows fire this event? | ||
| // Can we test this without even opening tab group? | ||
| tabGroup.addEventListener('open', () => { | ||
| try { | ||
| tabGroup.setActiveTab(tabB); | ||
| should(tabGroup.getActiveTab().title).be.a.String; | ||
| should(tabGroup.getActiveTab().title).eql('Tab B'); | ||
| finish(); | ||
| } catch (err) { | ||
| finish(err); | ||
| } finally { | ||
| tabGroup.removeTab(tabA); | ||
| tabGroup.removeTab(tabB); | ||
| } | ||
| }); | ||
|
|
||
| tabGroup.addTab(tabA); | ||
| tabGroup.addTab(tabB); | ||
| tabGroup.open(); | ||
| }); | ||
|
|
||
| it.windowsBroken('#setActiveTab()_before_open', finish => { | ||
| const winA = Ti.UI.createWindow(), | ||
| tabA = Ti.UI.createTab({ | ||
| title: 'Tab A', | ||
| window: winA | ||
| }), | ||
| winB = Ti.UI.createWindow(), | ||
| tabB = Ti.UI.createTab({ | ||
| title: 'Tab B', | ||
| window: winB | ||
| }); | ||
| tabGroup = Ti.UI.createTabGroup(); | ||
| tabGroup.setActiveTab(1); | ||
| // Does windows fire this event? | ||
| // Can we test this without even opening tab group? | ||
| tabGroup.addEventListener('open', () => { | ||
| try { | ||
| should(tabGroup.getActiveTab().title).be.a.String; | ||
| should(tabGroup.getActiveTab().title).eql('Tab B'); | ||
| finish(); | ||
| } catch (err) { | ||
| finish(err); | ||
| } finally { | ||
| tabGroup.removeTab(tabA); | ||
| tabGroup.removeTab(tabB); | ||
| } | ||
| }); | ||
|
|
||
| tabGroup.addTab(tabA); | ||
| tabGroup.addTab(tabB); | ||
| tabGroup.open(); | ||
| }); | ||
|
|
||
| it.windowsBroken('#change_activeTab_property', finish => { | ||
| const winA = Ti.UI.createWindow(), | ||
| tabA = Ti.UI.createTab({ | ||
| title: 'Tab A', | ||
| window: winA | ||
| }), | ||
| winB = Ti.UI.createWindow(), | ||
| tabB = Ti.UI.createTab({ | ||
| title: 'Tab B', | ||
| window: winB | ||
| }); | ||
| tabGroup = Ti.UI.createTabGroup(); | ||
|
|
||
| // Does windows fire this event? | ||
| // Can we test this without even opening tab group? | ||
| tabGroup.addEventListener('open', () => { | ||
| try { | ||
| tabGroup.activeTab = tabB; | ||
| should(tabGroup.getActiveTab().title).be.a.String; | ||
| should(tabGroup.getActiveTab().title).eql('Tab B'); | ||
| finish(); | ||
| } catch (err) { | ||
| finish(err); | ||
| } finally { | ||
| tabGroup.removeTab(tabA); | ||
| tabGroup.removeTab(tabB); | ||
| } | ||
| }); | ||
|
|
||
| tabGroup.addTab(tabA); | ||
| tabGroup.addTab(tabB); | ||
| tabGroup.open(); | ||
| }); | ||
|
|
||
| it.windowsBroken('#change_activeTab_property_before_open', finish => { | ||
| const winA = Ti.UI.createWindow(), | ||
| tabA = Ti.UI.createTab({ | ||
| title: 'Tab A', | ||
| window: winA | ||
| }), | ||
| winB = Ti.UI.createWindow(), | ||
| tabB = Ti.UI.createTab({ | ||
| title: 'Tab B', | ||
| window: winB | ||
| }); | ||
| tabGroup = Ti.UI.createTabGroup(); | ||
| tabGroup.activeTab = 1; | ||
| // Does windows fire this event? | ||
| // Can we test this without even opening tab group? | ||
| tabGroup.addEventListener('open', () => { | ||
| try { | ||
| should(tabGroup.getActiveTab().title).be.a.String; | ||
| should(tabGroup.getActiveTab().title).eql('Tab B'); | ||
| finish(); | ||
| } catch (err) { | ||
| finish(err); | ||
| } finally { | ||
| tabGroup.removeTab(tabA); | ||
| tabGroup.removeTab(tabB); | ||
| } | ||
| }); | ||
|
|
||
| tabGroup.addTab(tabA); | ||
| tabGroup.addTab(tabB); | ||
| tabGroup.open(); | ||
| }); | ||
|
|
||
| it.windowsBroken('#set_activeTab_in_creation_dictionary', finish => { | ||
| const winA = Ti.UI.createWindow(), | ||
| tabA = Ti.UI.createTab({ | ||
| title: 'Tab A', | ||
| window: winA | ||
| }), | ||
| winB = Ti.UI.createWindow(), | ||
| tabB = Ti.UI.createTab({ | ||
| title: 'Tab B', | ||
| window: winB | ||
| }); | ||
| tabGroup = Ti.UI.createTabGroup({ | ||
| activeTab: 1 | ||
| }); | ||
| // Does windows fire this event? | ||
| // Can we test this without even opening tab group? | ||
| tabGroup.addEventListener('open', () => { | ||
| try { | ||
| should(tabGroup.getActiveTab().title).be.a.String; | ||
| should(tabGroup.getActiveTab().title).eql('Tab B'); | ||
| finish(); | ||
| } catch (err) { | ||
| finish(err); | ||
| } finally { | ||
| tabGroup.removeTab(tabA); | ||
| tabGroup.removeTab(tabB); | ||
| } | ||
| }); | ||
|
|
||
| tabGroup.addTab(tabA); | ||
| tabGroup.addTab(tabB); | ||
| tabGroup.open(); | ||
| }); | ||
|
|
||
| }); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class'
onTabSelected()method is missing anullcheck on theselectedTabmember variable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to this
onTabSelected()method?https://github.com/appcelerator/titanium_mobile/blob/master/android/modules/ui/src/java/ti/modules/titanium/ui/TabGroupProxy.java#L529
If that is the case, it is intended for the
selectedTabto be null in order to skip the sending of thefocus/blurevents when a tab is set as an active one before the opening of the TabGroup. But I may have not got what you mean.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right. Never mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the unit test around blur events is now failing, and is likely related to the changes in this area...
https://github.com/appcelerator/titanium-mobile-mocha-suite/blob/master/Resources/ti.ui.tabgroup.test.js#L232-L247
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some digging I found that the "blur" unit test started failing because this PR fixed firing a pair of
focus/blurevents that are not supposed to be fired. That problem was introduced at the same time when the unit test was added (Both by me) and this is the reason it didn't fail every single time in other PRs. With my latest update this should be cleared up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unit test is passing now.