-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[Pre RN 0.83] Update react-native-image-size
#80876
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
base: main
Are you sure you want to change the base?
[Pre RN 0.83] Update react-native-image-size
#80876
Conversation
|
|
|
@war-in do you think the failure in the Reassure Performance Tests is related to our PR? Thank you. |
|
I don't think so, could you rerun it? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
brunovjk
left a comment
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.
LGTM, build and runs fine, without errors on my site.
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #80883 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
It seems that it didn't rerun, @pecanoro can you rerun it to see if the warnings disappear? I don't believe it's related to this PR, thank you. |
|
I merged main to rerun it. Let's see if it fails again |
Codecov Report✅ All modified and coverable lines are covered by tests. |
|
@pecanoro all yours! |
|
What is |
|
Fair, that change should not affect logic but I'll add android tests and videos 👍 |
|
Ok! Let me know when you update it and I will approve it |
react-native-image-sizereact-native-image-size
|
Perfect timing, done! |
|
I also noticed a regression (verified that it also occurs on main)
Expected behaviour: Actual result: Screen_Recording_20260130_160954_Expensify.Dev.mp4 |
|
Are you aware of it? Should I report it in |
|
@war-in, I am not seeing it on staging on my phone, but I haven't checked on main. Feel free to report it on #expensify-bugs, but if it's on staging, I am not sure if the QA team will be able to replicate it. I am going to check myself on dev. |
|
Reported! I think we can safely merge this one as the bug is not related :) |
Explanation of Change
We recently merged a change to our fork of
react-native-image-sizethat adds compatibility with RN 0.83. We should bump it to unblock the RN update in E/AppFixed Issues
$ #80883
PROPOSAL:
Tests
Verify that Android app builds correctly
Android only:
Offline tests
QA Steps
Android only:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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
Screen_Recording_20260130_155712_Expensify.Dev.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari