Skip to content

fix(iOS): fix rendering RCTRedBoxExtraData#43102

Closed
krozniata wants to merge 2 commits into
facebook:mainfrom
krozniata:fix/rct-red-box-extra-data
Closed

fix(iOS): fix rendering RCTRedBoxExtraData#43102
krozniata wants to merge 2 commits into
facebook:mainfrom
krozniata:fix/rct-red-box-extra-data

Conversation

@krozniata
Copy link
Copy Markdown
Contributor

Summary:

This PR fixes rendering of RCTRedBoxExtraData

I noticed that it wasn't displaying the reload and dismiss buttons, which made it impossible to close modal and to reload JS on e.g. visionOS (on iOS it could only be closed by swiping).

PR adds these buttons back and also introduces some refactoring

Before & After:

pr-img

Changelog:

[IOS] [FIXED] - Fix rendering RCTRedBoxExtraData

Test Plan:

Make sure that RCTRedBoxExtraData displays and works as expected

@facebook-github-bot 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 Feb 19, 2024
Comment thread packages/react-native/React/Modules/RCTRedBoxExtraDataViewController.m Outdated
Comment thread packages/react-native/React/Modules/RCTRedBoxExtraDataViewController.m Outdated
@lunaleaps
Copy link
Copy Markdown
Contributor

Could you clarify what broke and how this fixes it?

@krozniata
Copy link
Copy Markdown
Contributor Author

krozniata commented Feb 20, 2024

@lunaleaps
Sure! Before, when user opened the "Extra data" section in RedBox it was impossible to reload JS from it, because due to a bug in the layout, buttons were not visible and the modal could only be dismissed by dragging it down (which wasn't possible to do in visionOS, so it couldn't be dismissed at all).

This PR brings back the missing buttons by fixing the RCTRedBoxExtraData layout, as they were already added before, they just weren't visible.

@krozniata
Copy link
Copy Markdown
Contributor Author

krozniata commented Mar 5, 2024

@lunaleaps
Is there anything else that's needs to be done to get this PR merged?

@Biki-das
Copy link
Copy Markdown
Contributor

@krozniata could you show me when this appears with an example? like the bug

@krozniata
Copy link
Copy Markdown
Contributor Author

@cipolleschi 👀

Copy link
Copy Markdown
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Thank you so much for this changes and for your patience.
This PR slipped from me until today.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 28, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@cipolleschi merged this pull request in c9d589d.

@github-actions
Copy link
Copy Markdown

This pull request was successfully merged by @krozniata in c9d589d.

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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants