Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove default tip of 21% for pos custom tip #2712

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

myxmaster
Copy link
Contributor

Description

This fixes #2698.

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Configuration change
  • Locales update
  • Quality assurance
  • Other

Checklist

  • I’ve run yarn run tsc and made sure my code compiles correctly
  • I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
  • I’ve run yarn run prettier and made sure my code is formatted correctly
  • I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

  • Android
  • iOS

I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):

  • Embedded LND
  • LND (REST)
  • LND (Lightning Node Connect)
  • Core Lightning (CLNRest)
  • LndHub
  • [DEPRECATED] Core Lightning (c-lightning-REST)
  • [DEPRECATED] Core Lightning (Spark)
  • [DEPRECATED] Eclair

Locales

  • I’ve added new locale text that requires translations
  • I’m aware that new translations should be made on the ZEUS Transfix page and not directly to this repo

Third Party Dependencies and Packages

  • Contributors will need to run yarn after this PR is merged in
  • 3rd party dependencies have been modified:
    • verify that package.json and yarn.lock have been properly updated
    • verify that dependencies are installed for both iOS and Android platforms

Other:

  • Changes were made that require an update to the README
  • Changes were made that require an update to onboarding

@kaloudis
Copy link
Contributor

kaloudis commented Jan 8, 2025

I think I'd prefer to add 21% as a placeholder AND fall back to 21% if the user thinks it's populated and submits with no value set

@myxmaster
Copy link
Contributor Author

Ah right, that's the perfect solution.

Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

I think the default fees in sats should be 21% of the value, and not 2100 sats in all cases.

@myxmaster
Copy link
Contributor Author

But using absolute amounts is the idea behind that vs. percentage...?

@shubhamkmr04
Copy link
Contributor

Screenshot 2025-01-09 at 8 06 27 PM

So here the default fee is coming out as 2100 sats when we tap on it but the order value was only 396 sats ($0.37) that seems too much high for this amount.
it should be equivalent to the deafult fees in percentage, 21% of 396 sats, that is ~ 83 sats

@myxmaster
Copy link
Contributor Author

Yeah, I know what you mean, but not sure if the absolute placeholder should be dynamic. What do you think @kaloudis ?

@myxmaster
Copy link
Contributor Author

(rebased)

@kaloudis
Copy link
Contributor

Yeah, I know what you mean, but not sure if the absolute placeholder should be dynamic. What do you think @kaloudis ?

I think dynamic placeholder would indeed be a nice touch

@kaloudis kaloudis added this to the v0.10.0 milestone Jan 29, 2025
Copy link
Contributor

@shubhamkmr04 shubhamkmr04 left a comment

Choose a reason for hiding this comment

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

tACK

@kaloudis kaloudis merged commit b47dad5 into ZeusLN:master Feb 3, 2025
4 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

POS Order view: use placeholder for custom tip "21 %"
3 participants