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

teleport warning followup #8549

Merged
merged 5 commits into from
Dec 16, 2023
Merged

teleport warning followup #8549

merged 5 commits into from
Dec 16, 2023

Conversation

daiagi
Copy link
Contributor

@daiagi daiagi commented Dec 13, 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

Needs Design check

Context

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

Screenshot 📸

  • My fix has changed UI

image

image

image

Copilot Summary

🤖[deprecated] Generated by Copilot at e454f94

This pull request enhances the user experience and safety of the teleport feature by adding a warning modal and a tooltip to inform the user about the existential deposit risk. It also fixes a minor bug with the currency value display. The changes affect the Teleport.vue and EDWarningModal.vue components and the en.json localization file.

🤖[deprecated] Generated by Copilot at e454f94

To teleport funds, you must know
The existential deposit, though
If you send too few
A modal will show
And warn you before you can go

@daiagi daiagi requested a review from a team as a code owner December 13, 2023 12:21
@daiagi daiagi requested review from preschian and Jarsen136 and removed request for a team December 13, 2023 12:21
Copy link

netlify bot commented Dec 13, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 39ef0ec
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/657bc175719772000844fda9
😎 Deploy Preview https://deploy-preview-8549--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.

@kodabot
Copy link
Collaborator

kodabot commented Dec 13, 2023

SUCCESS @daiagi PR for issue #8527 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime!

@kodabot
Copy link
Collaborator

kodabot commented Dec 13, 2023

SUCCESS @daiagi PR for issue #8548 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime!

@reviewpad reviewpad bot added medium Pull request is medium waiting-for-review labels Dec 13, 2023
@daiagi daiagi changed the title teleport ED modal + bug fix teleport warning followup Dec 13, 2023
Copy link
Contributor

reviewpad bot commented Dec 13, 2023

AI-Generated Summary: This pull request introduces a new warning modal for the teleport feature. It includes a major update to the teleportation process, ensuring that users are informed about the risk of fund loss when the existential deposit is insufficient. To accomplish this, a new component, EDWarningModal.vue, was created, and changes were made to the Teleport.vue component. These modifications ensure that a warning modal appears when users attempt to teleport with an insufficient existential deposit, letting them know about the potential permanent fund loss. Additionally, some localization text in locales/en.json was updated to support the new features.

@exezbcz
Copy link
Member

exezbcz commented Dec 13, 2023

can we maybe add "DOT" and also "(existential deposit)" so its clear why

image

@prury
Copy link
Member

prury commented Dec 13, 2023

  • Cursor style when hovering over Funds at risk message

  • mobile:
    image

@prury prury added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Dec 13, 2023
@daiagi
Copy link
Contributor Author

daiagi commented Dec 14, 2023

  • Cursor style when hovering over Funds at risk message

fixed - although it is same as all text in the app

mobile

i whipped up some solution. @exezbcz wdyt?

image

@prury
Copy link
Member

prury commented Dec 14, 2023

  • Cursor style when hovering over Funds at risk message

fixed - although it is same as all text in the app

forgive me, I've failed to specify that it was only on why? and it was pointer style, but since its minor let's go.

@exezbcz
Copy link
Member

exezbcz commented Dec 14, 2023

i whipped up some solution. @exezbcz wdyt?

works for me thanks!

feedback

Transferring Less Than 1 + Fees

-> Transferring Less Than 1 DOT (existential deposit) + Fees

@daiagi
Copy link
Contributor Author

daiagi commented Dec 15, 2023

forgive me, I've failed to specify that it was only on why? and it was pointer style, but since its minor let's go.

it isn't clickable. only hoverable

@prury
Copy link
Member

prury commented Dec 15, 2023

forgive me, I've failed to specify that it was only on why? and it was pointer style, but since its minor let's go.

it isn't clickable. only hoverable

True

@daiagi
Copy link
Contributor Author

daiagi commented Dec 15, 2023

@exezbcz

image

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

codeclimate bot commented Dec 15, 2023

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

View more on Code Climate.

@prury
Copy link
Member

prury commented Dec 15, 2023

@daiagi strange, i'm still not seeing the currency, even after cache clear or browser change
and now message appears even if i funds on destination chain

image

@exezbcz
Copy link
Member

exezbcz commented Dec 15, 2023

Can we push this one ASAP, please as more people are losing their funds

@yangwao
Copy link
Member

yangwao commented Dec 16, 2023

i'm still not seeing the currency, even after cache clear or browser change
and now message appears even if i funds on destination chain

@prury let's do follow up, merging it for now, hope it improves over time

@yangwao
Copy link
Member

yangwao commented Dec 16, 2023

Thanks!
pay 50 usd

@yangwao yangwao merged commit 5267dc1 into main Dec 16, 2023
@yangwao yangwao deleted the teleport-warning branch December 16, 2023 12:29
@yangwao
Copy link
Member

yangwao commented Dec 16, 2023

😍 Perfect, I’ve sent the payout
💵 $50 @ 7.16 USD/DOT ~ 6.983 $DOT
🧗 1cAsKZYNRb8dkSCpn4eVkEn6ycNZTGoDo5dGDgB8J1UUWK8
🔗 0x229cd8a10d454d22fc8d32c87be6eaa05997ce68df3fac5b1618805569c7dede

🪅 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 Dec 16, 2023
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-changes-requested-🤞 PR is almost good to go, just some fine tunning waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bad button label in telport Teleport warning followups
6 participants