Mobile: Fixes #14835: Upgrade react-native-popup-menu to remove deprecated SafeAreaView warning#14865
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpgraded react-native-popup-menu to a patched 0.19.0, set MenuProvider to use View as SafeAreaComponent, added a MenuProvider test asserting no SafeAreaView deprecation warnings, updated lint/git ignores for the test, and updated pluginAssets (hash and an embedded base64 render change). Changes
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I have read the CLA Document and I hereby sign the CLA |
personalizedrefrigerator
left a comment
There was a problem hiding this comment.
Thank you for working on this! I've left a few comments.
| # This patch improves the note actions menu (the kebab menu)'s accessibility | ||
| # by labelling its dismiss button. |
There was a problem hiding this comment.
Please keep a comment at the top of this file. The comment in this file helps us determine why the patch is necessary.
| backHandler?: boolean | Function; | ||
| skipInstanceCheck?: boolean; | ||
| children: React.ReactNode; | ||
| SafeAreaComponent?: React.ComponentType<any>; |
There was a problem hiding this comment.
At least for me (testing on web), the menu is now slightly offset:

Based on this upstream comment, it may be necessary to provide the SafeAreaComponent={} prop to the <MenuProvider when rendering.
| it('should accept closeButtonLabel prop without TypeScript errors', () => { | ||
| const { getByText } = render( | ||
| <MenuProvider closeButtonLabel='Close menu'> | ||
| <Text>Content</Text> | ||
| </MenuProvider>, | ||
| ); | ||
|
|
||
| expect(getByText('Content')).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
I don't think this test is necessary: TypeScript issues with the closeButtonLabel prop should be caught during the build step (since closeButtonLabel is used elsewhere).
|
Thank you for the review! I've addressed all three points: Added the descriptive comment back to the top of the new patch file Please let me know if anything else needs to be changed! |
…h SafeAreaComponent, remove redundant test
Problem
Several warnings similar to the following were logged during CI test runs in
packages/app-mobile:SafeAreaView has been deprecated and will be removed in a future release.
Please use 'react-native-safe-area-context' instead.
This warning originated from
react-native-popup-menu@0.17.0, which internally used the deprecated<SafeAreaView>component from React Native core.Fixes #14835
Solution
Upgraded
react-native-popup-menufrom0.17.0to0.19.0, which removes the deprecated<SafeAreaView>usage.The existing Yarn patch (which adds
closeButtonLabelandaccessibilityRoleto the backdrop dismiss button for accessibility) was migrated to the new version. The patch changes are functionally identical — only the line offsets changed due to the version difference.The old
0.17.0patch file has been removed and replaced with a new0.19.0patch file.Test Plan
Added
components/testing/MenuProvider.test.tsxwhich verifies:MenuProviderrenders without triggering anySafeAreaView has been deprecatedconsole warningsMenuProvidercorrectly accepts thecloseButtonLabelprop without TypeScript errorsManual verification:
yarn workspace @joplin/app-mobile jest --forceExitSafeAreaView has been deprecatedwarnings appear in outputAI Assistance Disclosure
I used Claude (Anthropic) to help understand the Yarn patch migration workflow and to assist with writing the test file. All code was reviewed, understood, and manually verified by me