Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6_1_X][TIMOB-24712] Android: Fixed TabGroup to not override ActionBar's "colorPrimary" XML theme. #9074

Merged
merged 4 commits into from
May 24, 2017

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented May 23, 2017

JIRA: https://jira.appcelerator.org/browse/TIMOB-24712

Notes:

  • Issue was caused by line number 342 in old code. It was returning a default color when no color was assigned to the Tab or TabGroup. Should not change the background drawable at all if no color was assigned in order to respect the default theme.
  • Removed the "actionBar.setBackgroundDrawable()" line. It wasn't working to begin with (tested in 6.0.2) because you can't share the same drawable instance between views. Plus, the developer probably wouldn't want the TabGroup color to override the ActionBar color set in the XML theme. I talked to Lokesh about this and he agrees.

Test Case:

  1. Add the below files to a classic Titanium app.
  2. Build and run on an Android device.
  3. Verify that the top ActionBar (ie: title bar) is blue. (Comes from the XML theme file.)
  4. Verify that the tabs are red. (This is assigned in JavaScript.)
  5. Tap the button that is centered inside of one of the tabs.
  6. A new window is displayed.
  7. Press the back button to exit the 2nd window.
  8. Verify that you were returned to the 1st window without issue. (This tests for TIMOB-24367.)

app.js:

function createTab(title, buttonText) {
	var win = Ti.UI.createWindow({
		title: title,
		backgroundColor: '#fff'
	});

	var button = Ti.UI.createButton();
	button.title = buttonText;
	button.addEventListener("click", function(e) {
		var newWindow = Ti.UI.createWindow({
			backgroundColor: "blue",
			title: buttonText + " Window",
		});
		newWindow.add(Ti.UI.createLabel({text: 'Tap Back Button'}));
		newWindow.open();
	});
	win.add(button);

	var tab = Ti.UI.createTab({
		title: title,
		backgroundColor: "red",
		backgroundFocusedColor: "red",
		window: win,
	});
	return tab;
}

var tabGroup = Ti.UI.createTabGroup();
tabGroup.addTab(createTab("Tab 1", "Button 1"));
tabGroup.addTab(createTab("Tab 2", "Button 2"));
tabGroup.open();

./platform/android/res/values/builtin_themes.xml:

<?xml version="1.0" encoding="UTF-8"?>
<resources>
	<style name="tracker" parent="@style/Theme.AppCompat">
		<item name="colorPrimary">#0000FF</item>
		<item name="colorAccent">#FFFFFF</item>
		<item name="android:statusBarColor">#004486</item>
		<item name="android:navigationBarColor">#004486</item>
	</style>
</resources>

- Deleted the tabs.remove() in the tabs for-each loop. Would have broken the iterator and cause an exception.
Copy link
Collaborator

@hansemannn hansemannn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR passed, the null-check guards are new.

@jquick-axway
Copy link
Contributor Author

Regarding the null checks, I added them because I removed the following line...
tabs.remove(tab);

...from a for-each loop which would have invalidated the iterator and caused an exception to be thrown. So, instead, I'm preventing nulls from being added to the "tabs" collection.

@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented May 24, 2017

FR Passed.

colorPrimary of a custom theme is respected & the tab backgroundColor does not override the actionbar color from the custom theme.

Studio Ver: 4.9.0.201705180402
SDK Ver: 6.1.0 local build
OS Ver: 10.12.3
Xcode Ver: Xcode 8.3.2
Appc NPM: 4.2.9
Appc CLI: 6.2.1
Ti CLI Ver: 5.0.14
Alloy Ver: 1.9.11
Node Ver: 6.10.1
Java Ver: 1.8.0_101
Devices: ⇨ google Nexus 6 --- Android 6.0.1, Samsung Galaxy S4 --- Android 4.4.4

@lokeshchdhry lokeshchdhry merged commit 6c3d547 into tidev:6_1_X May 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants