Created TokenIcon component and added it throughout the app. I am having trouble getting the Popover in SendToken.js to render the component, but I can continue to work on it.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project
cd web/cashtab
npm start
Ensure that there are token txs within recent history
Check that custom token icons and generated icons are appearing
Naviagate to eTokens tab
Check that custom token icons and generated icons are appearing
Navigate to SentToken page
Check that custom icons and generated icons are appearing in the amount field of the send form
Currently, the popover icon in the SendToken page is not rendering the component, which is why it not using the TokenIcon component. I am aiming to fix this issue.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- token-icon-component
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 17865 Build 35556: Build Diff cashtab-tests Build 35555: arc lint + arc unit
Event Timeline
| web/cashtab/src/components/Tokens/TokenIcon.js | ||
|---|---|---|
| 8 ↗ | (On Diff #31793) | Is this empty div necessary? Seems like <Wrapper> is still appearing in the files hosting the new <TokenIcon> component |
| web/cashtab/src/components/Send/SendToken.js | ||
|---|---|---|
| 371 ↗ | (On Diff #31834) | Does this <span> have to be here? |
| web/cashtab/src/components/Tokens/TokenIcon.js | ||
| 11 ↗ | (On Diff #31834) | It's okay to assume that this will never be the case going forward. This was a placeholder from when we didn't have the icon backend up. So, remove this conditional logic. Component can just be the Img with unloader. |
Comment Actions
Responding to review feedback, removed conditional logic from TokenIcon component. Initially I thought the span wrapping the component was necessary for displaying it inside the popover, but it now appears to be working fine without the span. Additionally, updated snapshots.
| web/cashtab/src/components/Send/SendToken.js | ||
|---|---|---|
| 379 ↗ | (On Diff #31876) | Do we need this <span>? |
| web/cashtab/src/components/Send/SendToken.js | ||
|---|---|---|
| 379 ↗ | (On Diff #31876) | Without a parent element the popover will not render with the TokenIcon component inside. |
Comment Actions
responding to review feedback, removed span that wrapped TokenIconComponent in SendToken component.
| web/cashtab/src/components/Send/SendToken.js | ||
|---|---|---|
| 387 ↗ | (On Diff #32064) | why is token.tokenId used here but former code (and 2 lines below) uses tokenId? If tokenId is available, just use that everywhere. |
| web/cashtab/src/components/Tokens/TokenIcon.js | ||
| 32 ↗ | (On Diff #32064) | Must be 32, 64, 128, 256, or 512 -- not just any arbitrary number size: PropTypes.oneOf([32, 64, 128, 256, 512]), |
Comment Actions
Local repo pushed files that have been deleted since the UI overhaul, abandoning and re-submitting.