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

Add support for resource-id #18527

Closed
wants to merge 2 commits into from
Closed

Add support for resource-id #18527

wants to merge 2 commits into from

Conversation

lightboys22
Copy link

@lightboys22 lightboys22 commented Mar 23, 2018

Summary

Track react tag using android.view.View.tag. Allow developers to set resource id by setting component.testID

Fixes #9777

Test Plan

First, you need to declare the resource id by creating /your_android_studio_folder/res/values/ids.xml

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <item type="id" name="test_id"/>
</resources>

Second. Optionally, you can expose the resource id names so we can access them in our JSX code. Or you can just hard code a list of resource id names.

public class ConstantsModule extends BaseJavaModule {
  @Override
  public String getName() {
    return REACT_NAME;
  }

  @Override
  public Map<String, Object> getConstants() {
    return // return the map with resource ids here.
  }
}

Third, set the testID of your component to the name of a resource id.

<Button
    onPress={onButtonPress}
    title="Press Me"
    testID='test_id'
    />

Forth, run the app and observe the resource id using UIAutomator viewer (/Android/sdk/tools/bin/uiautomatorviewer)

resource_id_snapshot

Related PRs

This PR doesn't require a documentation change.

Changelog

[ANDROID] [Fixes] - BaseViewManager: Allow developers to set resource id by setting component.testID

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 23, 2018
@hramos hramos added the Platform: Android Android applications. label Mar 27, 2018
Track react tag using android.view.View.tag.  Allow developers to set resource id by setting component.testID

This pull request fixes the following issue.
#9777

First, you need to declare the resource id by creating /_your_android_studio_folder_/res/values/ids.xml

```
<?xml version="1.0" encoding="utf-8"?>
<resources>
    <item type="id" name="my_test_id"/>
</resources>
```

Second, set the testID of your component to a resource name.

```
<Button
    onPress={onButtonPress}
    title="Press Me"
    testID='test_id'
    />
```

Third, run the app and observe the resource id using UIAutomator viewer (/Android/sdk/tools/bin/uiautomatorviewer)

This PR doesn't require a documentation change.

[ANDROID] [BUGFIX] [com/facebook/react/uimanager/BaseViewManager] - Allow developers to set resource id by setting component.testID

Fix merge conflict.

Trying to fix build fail.
@nes123
Copy link

nes123 commented Sep 10, 2018

any plans to continue with that?

@erez-guesty
Copy link

hi
whats the status of this feature?

@ronilitman
Copy link

@lightboys22 any news on that?

Copy link
Contributor

@mdvacca mdvacca left a comment

Choose a reason for hiding this comment

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

@lightboys22 thanks for working on this!
I agree that it would be good to not use the View.id field to store the reactTag, but I have few concerns about this change:

  • This change is non backward compatible: the View.id field is used as part of events, merging this PR as it is will produce bugs in viewManager that are not part of the Core of React Native.
    Usually for changes like this we first deprecate old methods in one version, we document and communicate this change to the community and then in one or two versions we change the behavior.

  • Performance: I wonder if this change will regress performance in any way during rendering. View.id is just a long variable, but when you set a tag, View internally creates a SparseArray where it stores the Tag, in this case the ReactTag.
    This means that every time we need to access the 'reactTag' we need to search in this SparseArray instead of just accessing a variable. This cost will be paid by all the views in React Native. Let's just double check this.

  • There are changes in classes that do not exist anymore and classes has changed a lot, you should rebase

I also would like to hear from other core contributors about this change: @dulmandakh @gengjiawen

@facebook facebook deleted a comment from facebook-github-bot Feb 14, 2019
@facebook facebook deleted a comment from facebook-github-bot Feb 14, 2019
@hramos hramos changed the title Fixes facebook/react-native#9777 Add support for resource-id Add support for resource-id Feb 14, 2019
@icew1nd
Copy link

icew1nd commented Feb 19, 2019

Is there any progress on this? Would help a lot with bot testing 🎉

@lightboys22
Copy link
Author

lightboys22 commented Feb 19, 2019

Sorry, I won’t be working on this. You can try using the Espresso JavaScript framework to find view based on tags, (which is the testID)

@LuisBonsembiante
Copy link

Waht happen with this PR?... why everithing is complicated in RN?

@lightboys22
Copy link
Author

lightboys22 commented Mar 16, 2020 via email

@LuisBonsembiante
Copy link

@lightboys22 ...It is for robo test... so I need a resource-id....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for resource-id