-
Notifications
You must be signed in to change notification settings - Fork 87
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
chore(discover): Update rewards icon #6237
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6237 +/- ##
==========================================
- Coverage 88.93% 88.93% -0.01%
==========================================
Files 737 737
Lines 31433 31432 -1
Branches 5835 5839 +4
==========================================
- Hits 27954 27953 -1
Misses 3279 3279
Partials 200 200
Continue to review full report in Codecov by Sentry.
|
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.
Looking closely at the image of your emulator, it seems like the bottom and right-hand side of the image is cropped (ever so slightly) compared to the designs
@jophish ahh I was using the size from figma but the size in the converted svg was slightly different. Just updated and updated the image in the test plan and looks good now. |
src/icons/Reward.tsx
Outdated
import Svg, { Path } from 'react-native-svg' | ||
|
||
const Reward = ({ size = 34 }: { size?: number } = {}) => ( | ||
<Svg width={size} height={size} fill="none"> |
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.
does this need a viewbox too if you want to allow customizing size? Also I notice we use pngs for other similar icons. Is there a reason to use svg for this?
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.
Added viewbox
in e06a6a7, the reason I did an svg component instead of a png is that the image it was replacing was an svg component. I can change to a png instead if that is preferred though.
src/icons/Reward.tsx
Outdated
import Svg, { Path } from 'react-native-svg' | ||
|
||
const Reward = ({ size = 34 }: { size?: number } = {}) => ( | ||
<Svg width={size} height={size} viewBox={`0 0 ${size} ${size}`} fill="none"> |
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.
I don't believe this will work correctly if you specify different size params. Here's some useful info on how viewbox / width / height work https://valora-app.slack.com/archives/C025V1D6F3J/p1724705958982739
Description
Add new icon and use in the rewards card
Test plan
Related issues
Backwards compatibility
Yes
Network scalability
If a new NetworkId and/or Network are added in the future, the changes in this PR will: