[Android] ScrollView: handling `testID` correctly for horizontal scroll view by mateoguzmana · Pull Request #48254 · react/react-native

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

Fixes#46180

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

Fixes#46180

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

Fixes#46180

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

Fixes#46180

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

Fixes#46180

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

Fixes#46180

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

mateoguzmana · 2024-12-13T10:54:06Z

Summary:

Fixes#46180

This PR fixes the testID not being set as a resource-id in the HorizontalScrollView.

Currently the resource-id is being set correctly when we use a vertical scroll view (this is done here for reference) but we still miss setting this when we use a horizontal one as the managers for both components are different.

Changelog:

[ANDROID][FIXED] - Handling testID correctly for horizontal scroll view

Test Plan:

Render a simple ScrollView component with horizontal set as true and pass a testID property as shown:

functionPlayground(){return(<ScrollViewtestID="customScrollViewTestId"horizontal><Viewstyle={{marginVertical: 400,backgroundColor: 'white',padding: 14,margin: 50,width: 200,height: 200,}}/></ScrollView>);}

Open Maestro Studio and search for customScrollViewTestId in the search bar.

Before the fix: The `testID` is not found. (See screenshot)

See the same in Appium. (See screenshot)

Apply this fix, and search again in Maestro Studio.

After the fix: The `testID` is now recognised and can be found in the search bar. (See screenshot)

See the same in Appium. (See screenshot)

javache · 2024-12-13T12:11:55Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

…iew and reverting the test id prop setter overwrite

mateoguzmana · 2024-12-13T15:16:40Z

ReactScrollViewManager derives from BaseViewManager which already overrides setTestId. Could you please debug why that method isn't getting invoked?

@javache thanks to your observation, I went back and checked the full flow and actually the methods were called correctly. When doing my fix I did some clean up in the repro example and actually made me think it got fixed (my mistake during testing 😄)

With the above said, I narrowed down the issue and discovered it only happens when we use a horizontal scroll view. For the vertical one it was working already correctly.

I've updated the PR info as well to reflect this, used also Appium to double check it's not a problem of a single e2e tool.

javache

Awesome, thanks for digging in!

facebook-github-bot · 2024-12-13T15:53:51Z

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-13T20:47:27Z

@javache merged this pull request in 81c74cd.

react-native-bot · 2024-12-13T20:47:28Z

This pull request was successfully merged by @mateoguzmana in 81c74cd

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Follow up from #48271 and #48254, I noticed that the Modal component also doesn't map the `resource-id` from the `testID` on Android. This PR addresses that. ## Changelog: [ANDROID] [FIXED] - Modal: Setting `resource-id` from `testID` prop Pull Request resolved: #48313 Test Plan: Alternatively do: ``` $ adb shell uiautomator dump UI hierchary dumped to: /sdcard/window_dump.xml $ adb pull /sdcard/window_dump.xml /sdcard/window_dump.xml: 1 file pulled, 0 skipped. 1.1 MB/s (3505 bytes in 0.003s) ``` and check in XML: ` resource-id="playground-modal" class="android.view.ViewGroup" ` ------- Using Appium, check that the `testID` prop passed from JS is mapped as `resource-id` in the rendered view group of the Modal. <details> <summary>Example of the code implementation in the RNTester Playground:</summary> ```tsx function Playground() { const [modalVisible, setModalVisible] = React.useState(false); return ( <> <Modal visible={modalVisible} testID="playground-modal"> <Text testID="inner-text-test-id">Hello World!</Text> </Modal> <Button title="Open Modal" onPress={() => { setModalVisible(true); }} /> </> ); } ``` </details> <details> <summary>Output in Appium Inspector:</summary> <img width="913" alt="image" src="https://github.com/user-attachments/assets/514ae2b3-35a8-4a1a-8efc-1ca6bd73f189" /> </details> Reviewed By: javache Differential Revision: D67369350 Pulled By: alanleedev fbshipit-source-id: a799ad5b974895a39d9287e3d76d1139a6ef6a83

feat(scrollview): [android] handling testID property setter
7bc3335

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Dec 13, 2024

mateoguzmana marked this pull request as draft December 13, 2024 14:49

fix(scrollview): [android] setting resource id for horizontal scrollv…
d4b2148
…iew and reverting the test id prop setter overwrite

mateoguzmana changed the title ~~[Android] ScrollView: support testID property~~[Android] ScrollView: handling testID correctly for horizontal scroll viewDec 13, 2024

mateoguzmana marked this pull request as ready for review December 13, 2024 15:16

mateoguzmana mentioned this pull request Dec 13, 2024
[Android] ScrollView does not properly handle testID property #46180
Closed

javache approved these changes Dec 13, 2024
View reviewed changes

facebook-github-bot closed this in 81c74cdDec 13, 2024

facebook-github-bot added the Merged This PR has been merged. label Dec 13, 2024

This was referenced Dec 14, 2024
[Android] TestID assigned to TextInput control not working #32969
Closed
[Android] Modal: Setting resource-id from testID prop #48313
Closed