-
Notifications
You must be signed in to change notification settings - Fork 32
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/explorer-icons #1549
fix/explorer-icons #1549
Conversation
Note Reviews PausedUse the following commands to manage reviews:
WalkthroughThe changes primarily revolve around the handling and display of blockchain explorer images and transaction data in various components. They include the addition of console logs for debugging, the use of optional chaining for safer property access, and the modification of image source attributes. Changes
Poem
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/synapse-interface/constants/chains/master.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/synapse-interface/constants/chains/master.tsx
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1549 +/- ##
=============================================
Coverage 51.23324% 51.23324%
=============================================
Files 362 362
Lines 24691 24691
Branches 283 283
=============================================
Hits 12650 12650
Misses 10813 10813
Partials 1228 1228
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying with Cloudflare Pages
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/synapse-interface/components/Portfolio/PortfolioContent/components/PortfolioTokenAsset.tsx (1 hunks)
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/synapse-interface/components/Portfolio/PortfolioContent/components/PortfolioTokenAsset.tsx
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- packages/synapse-interface/components/Portfolio/Transaction/MostRecentTransaction.tsx (3 hunks)
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (2 hunks)
Additional comments: 4
packages/synapse-interface/components/Portfolio/Transaction/MostRecentTransaction.tsx (3)
160-166: The
startedTimestamp
property is now being assigned eithertransaction.timestamp
ortransaction.id
iftransaction.timestamp
is not available. Ensure thattransaction.id
is a valid timestamp.174-183: Optional chaining has been added to the properties of
transaction
to prevent potentialundefined
errors. This is a good practice for handling potentially undefined properties.218-230: Optional chaining has been added to the properties of
transaction
to prevent potentialundefined
errors. This is a good practice for handling potentially undefined properties.packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (1)
- 388-394: The
src
attribute of theImage
component is using optional chaining fororiginChain.explorerImg
. This is a good practice as it prevents runtime errors iforiginChain
orexplorerImg
is undefined. However, ensure that there is appropriate error handling or a default image in caseoriginChain?.explorerImg
is undefined.
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (2 hunks)
Additional comments: 1
packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx (1)
- 388-394: The
Image
component is using optional chaining forsrc
attribute. This is a good practice to avoid potential null reference errors. However, ensure that there is a valid placeholder or default image to be used whenoriginChain?.explorerImg?.src
is null or undefined.
packages/synapse-interface/components/Portfolio/Transaction/PendingTransaction.tsx
Outdated
Show resolved
Hide resolved
@coderabbitai pause |
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.
Don't need this anymore
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.
removed in bb2f3ec
@@ -181,7 +181,7 @@ export const POLYGON: Chain = { | |||
nativeCurrency: { name: 'Matic', symbol: 'MATIC', decimals: 18 }, | |||
explorerUrl: 'https://polygonscan.com', | |||
explorerName: 'PolygonScan', | |||
explorerImg: polygonExplorerImg, | |||
explorerImg: polygonImg, |
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.
We're using the SVG in @assets/chains/...
vs @assets/explorer/...
here, meaning can we do that throughout so we don't have duplicates?
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 catching this! I was just testing out a few diff things to try and get the .png file to work. Didn't end up working so I converted all of @assets/explorer/
files from .png to .svg
Switched back in dde17ad
Summary by CodeRabbit
MostRecentTransaction
component.e2a0265671dc954cce3a1b6dde459bd6c05c9c88: synapse-interface preview link
9e49e01cd6a147c2a6d321e7e81be8d61fa2720a: synapse-interface preview link
8741647344f758eb3259814f5b1b1fe65c1d1abd: synapse-interface preview link
dca5442dda9b6d0edccb0f50292f244d6f196090: synapse-interface preview link
eaecd72ac240271406becb2664441f0c7501ae97: synapse-interface preview link
4382d21f1c5560ec9236ac1eb53246bca50ea1e0: synapse-interface preview link
7143cea36a819452eec22d50370e420896774ead: synapse-interface preview link
03fd33bfaf8ad4a0ef788ffa286761218d2599dc: synapse-interface preview link
91baa6d136ce521005724d9141bf6d8ceff459da: synapse-interface preview link
875afea83c5846b72b83407378e4e5a7a90155fc: synapse-interface preview link