-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[Pre RN 0.79] Bump react-native-live-markdown
#60664
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
[Pre RN 0.79] Bump react-native-live-markdown
#60664
Conversation
|
|
|
@shubham1206agra Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
@shubham1206agra in case of this issue, I found out that adding spaces is also present on the main branch. Steps to reproduce
|
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@war-in jest tests are failing |
|
@roryabraham those tests are randomly failing sometimes 🤷 |
|
Ok, so I found out that issues reported by @shubham1206agra are also present on main branches so we shouldn't be blocked by them. Screen.Recording.2025-05-08.at.14.34.15.mov |
|
@shubham1206agra can you complete the checklist here? @war-in can you report the bug you found on main in react-native-live-markdown? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-05-08.at.10.48.34.PM.movAndroid: mWeb ChromeScreen.Recording.2025-04-23.at.5.33.43.PM.moviOS: HybridAppScreen.Recording.2025-05-08.at.10.37.51.PM.moviOS: mWeb SafariScreen.Recording.2025-04-23.at.5.29.53.PM.movMacOS: Chrome / SafariScreen.Recording.2025-04-23.at.5.23.44.PM.movMacOS: DesktopScreen.Recording.2025-04-23.at.5.40.48.PM.mov |
|
@roryabraham Related |
|
@roryabraham Bump here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the bump and testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing @shubham1206agra The QA has done regression testing on this one on the RN bump and the issues found are fixed with the patch.
However, since this is our package, I am not sure if it makes sense to add a patch, lets try to get it merged and released upstream sooner and avoid the patch on our package
|
@war-in Can you please bump the RN Live Markdown version here? |
|
@shubham1206agra yes, just did! |
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
@shubham1206agra @mountiny We're gonna need another build due to Mobile-Expensify branch issues which are now fixed |
|
🚧 @roryabraham has triggered a test app build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Seems like we need another retest after the version was bumped again |
Screen.Recording.2025-05-17.at.8.57.16.PM.mov@war-in Unable to put the cursor in between the words at all. |
Screen.Recording.2025-05-17.at.8.59.52.PM.mov@war-in Unable to put multiple emojis via keyboard |
I think this is the default iOS behaviour 😅 |
|
Agreed you need to hold to add the cursor using a magnifier |
1 similar comment
|
Agreed you need to hold to add the cursor using a magnifier |
@shubham1206agra I used a build from main, and this behaviour is reproducible there. So let's not resolve it here Screen.Recording.2025-05-19.at.11.40.58.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the additional testing and review, i think we can go ahead with this pr and ensure we catch any potential issues in staging
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.1.47-0 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.1.47-6 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.1.47-6 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.1.47-6 🚀
|
Explanation of Change
I'm working on RN 0.79 support #60421 and I found out that the react-native-live-markdown has to be bumped because it supports React Native 0.79 since 0.1.260
It seems that we can do it effortlessly because it's backward compatible (not a major bump)
Fixed Issues
$ #57511
PROPOSAL:
MOBILE-EXPENSIFY: https://github.com/Expensify/Mobile-Expensify/pull/13542
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop