-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix: adjust the apiLogoUrl logic to handles cases when icon is an object with uri key #7917
fix: adjust the apiLogoUrl logic to handles cases when icon is an object with uri key #7917
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7917 +/- ##
=======================================
Coverage 37.39% 37.39%
=======================================
Files 1052 1052
Lines 28175 28175
Branches 2517 2517
=======================================
Hits 10536 10536
Misses 17040 17040
Partials 599 599 ☔ View full report in Codecov by Sentry. |
This seems to be related to #7920 Reminder to describe the purpose of the change in the PR title/description, not just what has changed. I didn't realize this was related to fixing a crash at first. See here for more about that: https://github.com/MetaMask/contributor-docs/blob/main/docs/pull-requests.md#use-the-description-to-explain-the-need-and-solution |
Kudos, SonarCloud Quality Gate passed! |
Fixed for both Android and iOS |
@@ -62,7 +62,7 @@ class WebsiteIcon extends PureComponent { | |||
/** | |||
* Icon image to use, this substitutes getting the icon from the url | |||
*/ | |||
icon: PropTypes.string, | |||
icon: PropTypes.oneOfType([PropTypes.string | PropTypes.object]), |
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.
This is wrong, this is returning a number as a bitwise operation result.
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.
The expected parameter is an array of PropTypes
type properties
icon: PropTypes.oneOfType([PropTypes.string | PropTypes.object]), | |
icon: PropTypes.oneOfType([PropTypes.string, PropTypes.object]), |
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.
fixed in #7953
Description
Adjust the
apiLogoUrl
logic to handles cases whenicon
is an object withuri
keyRelated issues
Fixes: #
Manual testing steps
Screenshots/Recordings
Before
After
Screen.Recording.2023-11-23.at.14.22.22.mov
Pre-merge author checklist
Pre-merge reviewer checklist