Change assetStatusCards to the new assetStatusCards2 from info server #5239
Conversation
Jon-edge
force-pushed
the
jon/assetstatuscards2
branch
from
3a6b52a to
a50668d
Compare
Jon-edge
force-pushed
the
jon/assetstatuscards2
branch
4 times, most recently
from
cedca72 to
17f9215
Compare
paullinator
force-pushed
the
jon/assetstatuscards2
branch
from
17f9215 to
30527ff
Compare
|
|
||
| export const PromoCards = (props: Props) => { | ||
| const { countryCode, navigation, screenWidth } = props | ||
| export const InfoCardCarousel = (props: Props) => { |
There was a problem hiding this comment.
Danger. Don't rename the file AND make changes in the same commit. Many times git can't tell it was a rename and you'll have a diff that shows a file deletion and file creation making a near impossible review. It happened to work out in this case
There was a problem hiding this comment.
I saw the diff worked out, or else I would have separated the commits. Will
default to separating to be safe in the future
| /> | ||
| {assetStatuses.length > 0 && !isSearching | ||
| ? assetStatuses.map(assetStatus => ( | ||
| <EdgeAnim enter={fadeInDown10} key={`${String(assetStatus.localeStatusTitle)}-${String(assetStatus.localeStatusBody)}`}> |
There was a problem hiding this comment.
The fadeInDown10 was effectively replaced by fadeInUp110 which is baked into the carousel. This needs to be pulled out of the carousel and used in the parent component.
|
|
||
| const handleCreateWallet = useHandler(async (walletId: string, tokenId: EdgeTokenId) => { | ||
| const wallet = account.currencyWallets[walletId] | ||
| const countryCode = await getCountryCodeByIp().catch(() => '') |
There was a problem hiding this comment.
For another PR: Need to always return undefined if we can't get the countryCode. Otherwise the filter will think the countryCode is a blank string and the excludeCountryCodes will fail. This needs to change everywhere we call getCountryCodeByIp
| export const PromoCards = (props: Props) => { | ||
| const { countryCode, navigation, screenWidth } = props | ||
| export const InfoCardCarousel = (props: Props) => { | ||
| const { countryCode, navigation, screenWidth, cards = [] } = props |
There was a problem hiding this comment.
Yikes! danger, setting a default cards = [] will cause an infinite loop if props.cards is undefined.
That's because you depend on cards in the useEffect below which then causes a re-render since you call setFilteredCards which will create a new cards array causing useEffect to get called again. And repeat forever
There was a problem hiding this comment.
Just accept an undefined and handle it in the useEffect
There was a problem hiding this comment.
Didn't realize that can trigger an infinite loop. Tested behavior before and
after the fix and it checks out.
Jon-edge
force-pushed
the
jon/assetstatuscards2
branch
from
b020938 to
de15850
Compare
3 participants
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
https://github.com/EdgeApp/edge-info-server/pull/131
Requirements
If you have made any visual changes to the GUI. Make sure you have: