Skip to content
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

feat: set multi chain balances in stores #6052

Merged
merged 9 commits into from
May 24, 2023

Conversation

preschian
Copy link
Member

@preschian preschian commented May 20, 2023

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

👇 __ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Had issue bounty label?

  • Fill up your KSM address: Payout

Copilot Summary

🤖 Generated by Copilot at c4402e4

Refactored the balance component and the identity store to handle multiple balances for different chains and tokens. Used a new ref and a getter to access and display the balance data in a consistent and reactive way. Improved the component performance by removing unnecessary computations.

🤖 Generated by Copilot at c4402e4

Sing, O Muse, of the skillful refactorer
Who improved the component of balances
By using the store of identity
And its new ref of multiBalances

@reviewpad reviewpad bot added the large Pull request is large label May 20, 2023
@preschian preschian changed the title Feat/multi-chain-balances-to-stores feat: set multi chain balances in stores May 20, 2023
…llery into feat/multi-chain-balances-to-stores
@reviewpad reviewpad bot added medium Pull request is medium and removed large Pull request is large labels May 20, 2023
@yangwao yangwao mentioned this pull request May 20, 2023
10 tasks
Base automatically changed from feat/multi-chain-balances to main May 20, 2023 13:43
@netlify
Copy link

netlify bot commented May 22, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 2a17f0a
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/646deb90cf9c03000868e992
😎 Deploy Preview https://deploy-preview-6052--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@preschian preschian marked this pull request as ready for review May 22, 2023 08:25
@preschian preschian requested a review from a team as a code owner May 22, 2023 08:25
@preschian preschian requested review from daiagi and Jarsen136 and removed request for a team May 22, 2023 08:25
Copy link
Contributor

@daiagi daiagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works nicely

just small stuff

stores/identity.ts Outdated Show resolved Hide resolved
stores/identity.ts Show resolved Hide resolved
stores/identity.ts Outdated Show resolved Hide resolved
Co-authored-by: Luke Fishman <daiagi@gmail.com>
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a small issue, it doesn't show BSX for my wallet: (low balance)

on beta
Capture d’écran 2023-05-22 à 11 29 12 AM

on preview
Capture d’écran 2023-05-22 à 11 28 54 AM

Co-authored-by: roiLeo <medina.leo42@gmail.com>
@codeclimate
Copy link

codeclimate bot commented May 22, 2023

Code Climate has analyzed commit 7a85462 and detected 0 issues on this pull request.

View more on Code Climate.

@preschian
Copy link
Member Author

I noticed a small issue, it doesn't show BSX for my wallet: (low balance)

is this your address? seems like on subcan and sub.id also show zero

let me check if a change to maximumFractionDigits: 4,

@roiLeo
Copy link
Contributor

roiLeo commented May 22, 2023

I noticed a small issue, it doesn't show BSX for my wallet: (low balance)

is this your address? seems like on subcan and sub.id also show zero

* https://basilisk.subscan.io/account/bXhWeVLtArWauM8UmbaVdDmXmo5uSDk6DAa2YxQ5zbt2sDcbW

* https://sub.id/bXhWeVLtArWauM8UmbaVdDmXmo5uSDk6DAa2YxQ5zbt2sDcbW/balances

Yes it's mine! as you can see some KSM has been transfered
Screenshot 2023-05-22 at 11-46-30 Subscan Basilisk Account Detail bXhWeVLtArWauM8UmbaVdDmXmo5uSDk6DAa2YxQ5zbt2sDcbW

let me check if a change to maximumFractionDigits: 4,

If it only happens in my case I don't know if it is necessary to change it

@preschian
Copy link
Member Author

preschian commented May 22, 2023

If it only happens in my case I don't know if it is necessary to change it

let's keep it for now? in case the designers or our users prefer more digits we can just change maximumFractionDigits to the number we want

this is the result with

{
  minimumFractionDigits: 0,
  maximumFractionDigits: 4,
}

image

@preschian preschian requested a review from daiagi May 22, 2023 10:08
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ wfm

@roiLeo roiLeo added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label May 22, 2023
@daiagi daiagi added S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved and removed waiting-for-review labels May 22, 2023
@prury
Copy link
Member

prury commented May 23, 2023

Working fine!

@yangwao
Copy link
Member

yangwao commented May 24, 2023

@preschian let's resolve conflicts and it's up for merge! :)

@yangwao
Copy link
Member

yangwao commented May 24, 2023

pay 100 usd extra for smashing 5 issues in one PR! 🤩

@yangwao yangwao enabled auto-merge May 24, 2023 10:19
@yangwao
Copy link
Member

yangwao commented May 24, 2023

😍 Perfect, I’ve sent the payout
💵 $100 @ 25.05 USD/KSM ~ 3.992 $KSM
🧗 DY4SQF2iD456tH89aQtz5wv1EV3BbSW8wKKuMcwbmXaj1pM
🔗 0x6b3fdd9c7a381e1d23aaca71ae77fd5a3766bccf8d1f53220432a417d7b5dd81

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label May 24, 2023
@yangwao yangwao merged commit 5bf0696 into main May 24, 2023
@yangwao yangwao deleted the feat/multi-chain-balances-to-stores branch May 24, 2023 10:48
@@ -95,15 +85,20 @@ function calculateUsd(amount: string, token = 'KSM') {

return calculateExactUsdFromToken(
amountToNumber,
Number(fiatStore.getCurrentTokenValue(token))
Number(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Number(fiatStore.getCurrentTokenValue(token))

Polkadot: 'dot',
kusama: 'ksm',
basilisk: 'bsx',
statemine: 'stmn',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing polkadot

@@ -27,6 +28,26 @@ type ChangeAddressRequest = {
apiUrl?: string
}

type ChainType = 'kusama' | 'basilisk' | 'statemine'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

polkadot

selected: boolean
address: string
}
type ChainToken = Partial<Record<'ksm' | 'bsx', ChainDetail>>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'dot'

multiBalances: DEFAULT_MULTI_BALANCE_STATE,
multiBalanceAssets: [
{ chain: 'kusama' },
{ chain: 'statemine' },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ chain: 'polkadot' },

Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@preschian preschian mentioned this pull request May 24, 2023
4 tasks
This was referenced May 25, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Pull request is medium paid pull-request has been paid S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked
Projects
None yet
7 participants