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

timob-10080: Android: Parity for Ti.Blob methods #2833

Merged
merged 8 commits into from Aug 30, 2012

Conversation

pingwang2011
Copy link
Contributor

…arity with iOS.Reset image when width or height is changed in TiUIImageView class.
@@ -936,6 +936,25 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP
} else {
super.propertyChanged(key, oldValue, newValue, proxy);
}

View parentView = getParentView();
Copy link
Contributor

Choose a reason for hiding this comment

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

While I understand the point of this is to avoid having the line in two places, I think we should avoid hitting the getParentView() call every single time that propertyChange is called. Please move this inside the if and else if blocks so we only hit it when we need it.

@pingwang2011
Copy link
Contributor Author

Updated. Ready for review again.

@rusticphilosopher
Copy link
Contributor

Updated code reviewed and minor comments left.

@@ -936,6 +936,26 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP
} else {
super.propertyChanged(key, oldValue, newValue, proxy);
}

if (key.equals(TiC.PROPERTY_WIDTH)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

From what we talked about, it looks like you want to do this in a separate 'if' block because you are relying on logic from the super class that deals with height/width. If that is the case, can we move this whole block into the 'else' statement above to make this more clear.

Also a comment on why you are doing this would be nice.

@rusticphilosopher
Copy link
Contributor

Spoke to Ping and she is going to make an additional performance optimization.

@pingwang2011
Copy link
Contributor Author

Updated. Ready for review.

@rusticphilosopher
Copy link
Contributor

Updated code reviewed and single comment left.

@pingwang2011
Copy link
Contributor Author

Updated. Ready for review.

@rusticphilosopher
Copy link
Contributor

Code reviewed and functional test passed on both runtimes. Accepted

rusticphilosopher pushed a commit that referenced this pull request Aug 30, 2012
timob-10080: Android: Parity for Ti.Blob methods
@rusticphilosopher rusticphilosopher merged commit 6323dc4 into tidev:master Aug 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants