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_0][TIMOB-24181] Android: fix ListItem template property inconsistencies #9017

Merged
merged 3 commits into from May 17, 2017

Conversation

frankieandone
Copy link
Contributor

@frankieandone frankieandone commented May 4, 2017

TEST CASE

var win = Ti.UI.createWindow({
	title: "classic_app",
	backgroundColor: "#000"
});

var customListItemTemplate = {
	properties: {
		name: "template",
		accessoryType: Ti.UI.LIST_ACCESSORY_TYPE_DISCLOSURE,
		backgroundColor: "red"
	},
	childTemplates: [
		{
			type: "Ti.UI.Label",
			bindId: "title",
			properties: {
				bindId: "title"
			}
		}
	]
};
var listView = Ti.UI.createListView({
	templates: {
		"template": customListItemTemplate
	},
	defaultItemTemplate: "template"
});

var sectionArray = [], sectionOne, itemArray = [], itemOne;
itemOne = {
	properties: {
		accessoryType: undefined,
		backgroundColor: undefined
	},
	title: {
		text: "itemOne title"
	}
};
itemArray.push(itemOne);
sectionOne = Ti.UI.createListSection({
	headerTitle: "Section title #1",
	items: itemArray
});
sectionArray.push(sectionOne);
listView.setSections(sectionArray);

win.add(listView);
win.open();

EXPECTED
Should get red background for list item.

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

@frankieandone frankieandone added this to the 6.1.0 milestone May 4, 2017
@frankieandone frankieandone self-assigned this May 4, 2017
@mukherjee2 mukherjee2 self-requested a review May 8, 2017 22:31
Copy link
Contributor

@mukherjee2 mukherjee2 left a comment

Choose a reason for hiding this comment

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

FAILED FR with this environment:
Node Version: 6.10.1
NPM Version: 3.10.10
Mac OS: 10.12.4
Appc CLI: 6.2.0
Appc CLI NPM: 4.2.9
Titanium SDK version: 6.1.0 locally built with this PR
Appcelerator Studio, build: 4.8.1.201612050850
Xcode 8.3.2
Android device 6.0.1
iOS Device 10.3

On iOS device, the background of the Title is red as expected. On the Android device it is black. Per the ticket, it should be red.

if (entry.getValue() != null) {
newData.put(entry.getKey(), entry.getValue());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Your code change in the "else" statement here no longer updates the "data" argument. It's making changes to a temporary "newData" local and then throwing it away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@ssjsamir ssjsamir self-requested a review May 17, 2017 20:32
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

Master: #8670

Verified FR - Was able to see the red background on a ListItem as stated in the description.

Test steps

  • Build the SDK using this PR
  • Created a titanium application
  • Pasted the code in in the application from the description
  • Ran the program
  • Saw red background on a ListItem

screen shot 2017-05-17 at 2 00 04 pm

Test environment
Appcelerator Command-Line Interface, version 6.2.1
Google Nexus 6P (7.1.2)
Operating System Name: Mac OS X El Capitan
Operating System Version: 10.11.6
Node.js Version: 4.6.0
npm: 4.2.8
Xcode: 8.2
Appcelerator Studio: 4.8.1.201612050850

@ssjsamir ssjsamir merged commit 50a0b80 into tidev:6_1_X May 17, 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

4 participants