Skip to content

[ios][interoplayer] fix adding children #51213

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

Conversation

chrfalch
Copy link
Collaborator

@chrfalch chrfalch commented May 9, 2025

Summary:

See #51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Test Plan:

Previous output:

After fix:

See facebook#51212 - children aren't updated correctly in a old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212
@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 May 9, 2025
@chrfalch chrfalch marked this pull request as ready for review May 9, 2025 11:22
@facebook-github-bot
Copy link
Contributor

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

cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request May 12, 2025
Summary:
See facebook#51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

bypass-github-export-checks

## Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Pull Request resolved: facebook#51213

Test Plan:
**Previous output**:

<image src="https://github.com/user-attachments/assets/472b95e7-0921-46c9-be6a-f31759c0cd26" width="200px" />

**After fix**:

<image src="https://github.com/user-attachments/assets/554387cd-c264-483e-9c52-d9cd40b42601" width="200px" />

Differential Revision: D74471278

Reviewed By: sammy-SC

Pulled By: sammy-SC
cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request May 13, 2025
Summary:
See facebook#51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

bypass-github-export-checks

## Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Pull Request resolved: facebook#51213

Test Plan:
**Previous output**:

<image src="https://github.com/user-attachments/assets/472b95e7-0921-46c9-be6a-f31759c0cd26" width="200px" />

**After fix**:

<image src="https://github.com/user-attachments/assets/554387cd-c264-483e-9c52-d9cd40b42601" width="200px" />

Differential Revision: D74471278

Reviewed By: sammy-SC

Pulled By: sammy-SC
cipolleschi pushed a commit to cipolleschi/react-native that referenced this pull request May 14, 2025
Summary:
See facebook#51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

bypass-github-export-checks

## Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Pull Request resolved: facebook#51213

Test Plan:
**Previous output**:

<image src="https://github.com/user-attachments/assets/472b95e7-0921-46c9-be6a-f31759c0cd26" width="200px" />

**After fix**:

<image src="https://github.com/user-attachments/assets/554387cd-c264-483e-9c52-d9cd40b42601" width="200px" />

Differential Revision: D74471278

Reviewed By: sammy-SC

Pulled By: sammy-SC
@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label May 14, 2025
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in d53a60d.

react-native-bot pushed a commit that referenced this pull request May 16, 2025
Summary:
See #51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

bypass-github-export-checks

## Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Pull Request resolved: #51213

Test Plan:
**Previous output**:

<image src="https://github.com/user-attachments/assets/472b95e7-0921-46c9-be6a-f31759c0cd26" width="200px" />

**After fix**:

<image src="https://github.com/user-attachments/assets/554387cd-c264-483e-9c52-d9cd40b42601" width="200px" />

Reviewed By: sammy-SC

Differential Revision: D74471278

Pulled By: cipolleschi

fbshipit-source-id: 798f9e7be389359bd6e3aa1b6a6e9fb799fcb369
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @chrfalch in 0a76d9f

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

react-native-bot pushed a commit that referenced this pull request Jun 4, 2025
Summary:
See #51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

bypass-github-export-checks

## Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Pull Request resolved: #51213

Test Plan:
**Previous output**:

<image src="https://github.com/user-attachments/assets/472b95e7-0921-46c9-be6a-f31759c0cd26" width="200px" />

**After fix**:

<image src="https://github.com/user-attachments/assets/554387cd-c264-483e-9c52-d9cd40b42601" width="200px" />

Reviewed By: sammy-SC

Differential Revision: D74471278

Pulled By: cipolleschi

fbshipit-source-id: 798f9e7be389359bd6e3aa1b6a6e9fb799fcb369
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @chrfalch in 2760184

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

react-native-bot pushed a commit that referenced this pull request Jul 1, 2025
Summary:
See #51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

bypass-github-export-checks

## Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Pull Request resolved: #51213

Test Plan:
**Previous output**:

<image src="https://github.com/user-attachments/assets/472b95e7-0921-46c9-be6a-f31759c0cd26" width="200px" />

**After fix**:

<image src="https://github.com/user-attachments/assets/554387cd-c264-483e-9c52-d9cd40b42601" width="200px" />

Reviewed By: sammy-SC

Differential Revision: D74471278

Pulled By: cipolleschi

fbshipit-source-id: 798f9e7be389359bd6e3aa1b6a6e9fb799fcb369
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @chrfalch in ab38559

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

fabriziocucci pushed a commit that referenced this pull request Jul 9, 2025
Summary:
See #51212 - children aren't updated correctly in an old arch native view using the interop layer under Fabric.

This is caused by the mountChildComponentView method not updating the view, only adding the new view to a list that will be used on the next update to mount the child.

This commit fixes this by adding the same pattern as in unmountChildComponentView where children are inserted directly if the underlying paperview is available in the adapter - otherwise it uses the mounting list as before.

#Closes 51212

bypass-github-export-checks

## Changelog:

[IOS] [FIXED] - fixed adding child views to a native view using the interop layer

Pull Request resolved: #51213

Test Plan:
**Previous output**:

<image src="https://github.com/user-attachments/assets/472b95e7-0921-46c9-be6a-f31759c0cd26" width="200px" />

**After fix**:

<image src="https://github.com/user-attachments/assets/554387cd-c264-483e-9c52-d9cd40b42601" width="200px" />

Reviewed By: sammy-SC

Differential Revision: D74471278

Pulled By: cipolleschi

fbshipit-source-id: 798f9e7be389359bd6e3aa1b6a6e9fb799fcb369
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @chrfalch in d59159d

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

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. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants