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

Transfer Redesign handover - (1) Desktop design #6486

Merged
merged 14 commits into from
Aug 4, 2023

Conversation

Jarsen136
Copy link
Contributor

@Jarsen136 Jarsen136 commented Jul 29, 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

Needs Design check

Context

Did your issue had any of the "$" label on it?

Screenshot 📸

  • My fix has changed UI

/dot/transfer?redesign=true
/ksm/transfer?redesign=true

image image

Copilot Summary

🤖 Generated by Copilot at d2c862a

This pull request introduces a new version of the Transfer component, which allows users to send NFTs to multiple recipients, choose different units, and generate a pay me link. The new component is styled with CSS and has English translations. A feature flag is added to switch between the new and old versions of the transfer page. A utility function is added to convert balance values to USD.

🤖 Generated by Copilot at d2c862a

We added a new Transfer component
With features to make it more potent
It can send to a bunch
And show USD crunch
With calculateBalanceUsdValue content

@Jarsen136 Jarsen136 requested a review from a team as a code owner July 29, 2023 14:47
@Jarsen136 Jarsen136 requested review from roiLeo and daiagi and removed request for a team July 29, 2023 14:47
@kodabot
Copy link
Collaborator

kodabot commented Jul 29, 2023

WARNING @Jarsen136 PR for issue #6464 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #6464

@netlify
Copy link

netlify bot commented Jul 29, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit ff787c3
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64ccea8d690c4500086a5b9d
😎 Deploy Preview https://deploy-preview-6486--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 configuration.

@reviewpad
Copy link
Contributor

reviewpad bot commented Jul 29, 2023

AI-Generated Summary: This pull request introduces a major overhaul of the Transfer.vue component in a Vue application, which is related to the cryptocurrency transfer feature. Changes include a redesigned UI, improved interactivity, and support for multilingual UI. Crucial components such as BalanceInput and ReadOnlyBalaceInput were replaced by NeoInput and several new computed properties and methods were introduced.

A new TabItem.vue file was modified, with changes primarily focused on component properties and the addition of new styling rules. Moreover, an A/B testing implementation has been introduced in the transfer.vue file, where a redesign boolean flag dictates whether the new Transfer component or the old TransferOld one is used.

There is also an addition of a new calculation function calculateBalanceUsdValue added to balance.ts in the utils/format/ folder, which calculates the USD balance value based upon a specific value and decimals.

Lastly, a fresh block of English translations related to transfers was added to the locales/en.json file, involving phrases like "Recipient", "Sender", "Send same amount", and others. These additions and modifications contribute to enhancing the user interface and functionality of the application.

@reviewpad reviewpad bot added large Pull request is large waiting-for-review labels Jul 29, 2023
components/shared/TabItem.vue Outdated Show resolved Hide resolved
components/shared/TabItem.vue Outdated Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
components/transfer/Transfer.vue Show resolved Hide resolved
components/transfer/Transfer.vue Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
Copy link
Contributor Author

@Jarsen136 Jarsen136 left a comment

Choose a reason for hiding this comment

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

@daiagi Thanks for your comment. I have updated them. Could you pls review again ?

components/transfer/Transfer.vue Show resolved Hide resolved
components/transfer/Transfer.vue Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
@Jarsen136 Jarsen136 requested a review from daiagi July 31, 2023 12:56
@prury
Copy link
Member

prury commented Jul 31, 2023

i think amount increment could be 0.01 instead of 0.001
image

add recipient only works after i add an amount:

add.recipient.mp4

follow up for those? :

image

@prury prury added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jul 31, 2023
@exezbcz
Copy link
Member

exezbcz commented Jul 31, 2023

artist submission from me 🎨
IMG_BBC88E9128C4-1

As @prury mentioned, there is no delete button once you add a recipient, I think it was written somewhere that it's intentionally?

@Jarsen136 Jarsen136 marked this pull request as draft August 1, 2023 12:16
@Jarsen136 Jarsen136 marked this pull request as ready for review August 3, 2023 10:42
@Jarsen136
Copy link
Contributor Author

I would make a follow-up issue:

Display estimated transaction fees on different networks
image

@prury
Copy link
Member

prury commented Aug 3, 2023

I would make a follow-up issue:

Display estimated transaction fees on different networks image

will open

@prury
Copy link
Member

prury commented Aug 3, 2023

Pending problems from Exez image:
Continue button height: 50px
Continue button disabled when no recipients filled

for the other ones I've found i will open another issue

@Jarsen136
Copy link
Contributor Author

Pending problems from Exez image: Continue button height: 50px Continue button disabled when no recipients filled

Fixed

@prury
Copy link
Member

prury commented Aug 3, 2023

Continue button disabled when no recipients filled

it works, but is still buggy, will add to #6529

@prury prury added S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked and removed S-changes-requested-🤞 PR is almost good to go, just some fine tunning labels Aug 3, 2023
@exezbcz
Copy link
Member

exezbcz commented Aug 3, 2023

yup, works for me, good job!

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.

some smol things

plus there seems to be an issue with balance not showing up

image

composables/useIcon.ts Outdated Show resolved Hide resolved
utils/math.ts Show resolved Hide resolved
components/transfer/Transfer.vue Show resolved Hide resolved
components/transfer/Transfer.vue Outdated Show resolved Hide resolved
components/transfer/Transfer.vue Show resolved Hide resolved
@yangwao
Copy link
Member

yangwao commented Aug 4, 2023

I would merge it if so, we will be doing plenty of follow-ups, no worries 😄

@codeclimate
Copy link

codeclimate bot commented Aug 4, 2023

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

View more on Code Climate.

@Jarsen136
Copy link
Contributor Author

some smol things

plus there seems to be an issue with balance not showing up

image

It's not related to this PR. There seems some magic in the preview environment. The account balance on stmn network could not be fetched correctly on the env. I found the bug also appeals in other preview env.

for example https://deploy-preview-6501--koda-canary.netlify.app/stmn/transfer

image

On canary env the bug is disappear.

image

@Jarsen136 Jarsen136 requested a review from daiagi August 4, 2023 12:28
@yangwao
Copy link
Member

yangwao commented Aug 4, 2023

pay 50 usd

@yangwao
Copy link
Member

yangwao commented Aug 4, 2023

😍 Perfect, I’ve sent the payout
💵 $50 @ 4.97 USD/DOT ~ 10.06 $DOT
🧗 16SjUbGKSdjCdWTy3NNT3JxbRVGGqD4mwkHpc6BD9U2Rp29Z
🔗 0x60ab92fa2f00927b21137d2b3e007aa565ee278a9d95a32296d45cca0fc4ed9c

🪅 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 Aug 4, 2023
@yangwao yangwao merged commit a923f61 into kodadot:main Aug 4, 2023
18 of 19 checks passed
This was referenced Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
large Pull request is large paid pull-request has been paid S-visual-ok-✅ S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transfer Redesign handover - (1) Desktop design
7 participants