-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Revert "Upgrade react-native-svg to 13.4.0" #12176
Conversation
|
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.
Looks good, I'll test on Android 👍
LGTM
|
17c5c4d
34fb9c7
to
17c5c4d
Compare
Following the CP here |
Revert "Upgrade react-native-svg to 13.4.0" (cherry picked from commit 97545ed)
…-12176 🍒 Cherry pick PR #12176 to staging 🍒
@Julesssss could you check if applying software-mansion/react-native-svg#1903 fixes the issue? |
🚀 Cherry-picked to staging by @Julesssss in version: 1.2.20-1 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
@WoLewicki yep, that resolves our issue. Thanks! |
🚀 Deployed to production by @chiragsalian in version: 1.2.20-3 🚀
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.28-5 🚀
|
Details
Simply reverts an SVG library bump, which introduced some issues specifically on Android:
Fixed Issues
$ #12173
$ #12171
#12172 <-- possibly this too, but we'll manually test/confirm once this is merged
Tests
Test the cases which are known to have caused this crash
QA Steps
Run the above test cases
PR Review Checklist
PR Author Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Screenshots
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android
Emulator
![Screenshot_1666869525](https://user-images.githubusercontent.com/10736861/198270945-0715eaff-c84b-4237-a05a-206eb5827ff4.png)
Physical Device
![Screenshot_android](https://user-images.githubusercontent.com/10736861/198269488-7f233105-ca7b-4106-9cb8-64dce8ad2346.png)