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-20286] iOS/Android: Expose Ti.UI.View.getViewById method #8677

Merged
merged 7 commits into from Jan 5, 2017

Conversation

hansemannn
Copy link
Collaborator

@hansemannn hansemannn changed the title [TIMOB-20286] iOS: Expose Ti.UI.View.getViewById method [TIMOB-20286] iOS/Android: Expose Ti.UI.View.getViewById method Dec 11, 2016
* @module.api
*/
@Kroll.method
public TiViewProxy getViewById(Object arg)
Copy link
Contributor

Choose a reason for hiding this comment

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

public TiViewProxy getViewById(String id)

// no need to use
TiConvert.toString(arg)

}
}

if (child.getProperty(TiC.PROPERTY_ID) != null && child.getProperty(TiC.PROPERTY_ID).equals(TiConvert.toString(arg))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (child.hasProperty(TiC.PROPERTY_ID) && child.getProperty(TiC.PROPERTY_ID).equals(id)) {

@hansemannn
Copy link
Collaborator Author

@garymathews Updated PR!

}
}

if (child.getProperty(TiC.PROPERTY_ID) && child.getProperty(TiC.PROPERTY_ID).equals(arg)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Builds failing, getProperty should be hasProperty

if (child.hasProperty(TiC.PROPERTY_ID) && child.getProperty(TiC.PROPERTY_ID).equals(id)) {

Copy link
Contributor

Choose a reason for hiding this comment

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

And this :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But hasProperty returns a boolean and getProperty the property itself. Please go ahead and edit it

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but you can only perform an && operation on a Boolean in Java. So either:

if (child.hasProperty(TiC.PROPERTY_ID) && child.getProperty(TiC.PROPERTY_ID).equals(id)) {

// OR

if (child.getProperty(TiC.PROPERTY_ID) != null && child.getProperty(TiC.PROPERTY_ID).equals(id)) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But that's how it was before all that. You can edit it on Github, so we can ensure it doesn't cause problems again.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, I think you might have missed my change to hasProperty here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Gotcha, changed it. But the original implementation was like the second suggestion above already (except the string-casting), that's why I wondered. Sorry for the confusion, let's finish this 🙂

* @module.api
*/
@Kroll.method
public TiViewProxy getViewById(String arg)
Copy link
Contributor

Choose a reason for hiding this comment

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

String id // same name as docs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does this work on Android? id is a type on iOS. Will change it here, but primarily followed other method argument lists.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

@garymathews
Copy link
Contributor

CR: PASS
FT: PASS

👍

@garymathews garymathews merged commit 87edde9 into tidev:master Jan 5, 2017
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

2 participants