Improve menu items for clarity, reduce copy-pasted Strings and bugfix fiat conversion #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ThomasFarstrike
commented
Dec 10, 2024
ThomasFarstrike
commented
- Fix fiat conversion API call compatibility with LNBits ("from" changed to "from")
- Change "LNPoS", "Offline PoS", "OnChain" and "ATM" menu items to more explicit "Receive Online", "Receive Offline", "Receive OnChain" and "Send Offline" (more detailed reasoning in separate commit messages)
- Move strings that are used more than once to a #define to avoid copy-paste, because this often causes bugs/inconsistencies due to changing one, but not the other(s).
This reduces copy-paste.
To reduce copy-paste.
This reduces copy-paste.
It was always returning "sats": 1 and it turned out that's because it should be "from_" (and not "from") as stated in the docs.
Notice users that just want to receive payments don't know the difference between the old menu items: LNPoS vs Offline PoS vs OnChain... In the case of non-native English speakers, they don't even know what a PoS is. Replacing these with "Receive Online", "Receive Offline" and "Receive OnChain" makes it much clearer of what they will do and how they will do it.
This menu option is used to send Bitcoin over the Lightning network in an offline manner, using LNURL-withdraw. Only US English speakers use the expression "ATM", as Canadians use "ABM", while the British call them cashpoints or cash machines. Non-English speakers also use different names, such as 'geldautomaat' in Flanders or 'pinautomaat' in the Netherlands. The "Send Offline" functionality *can* be used to sell Bitcoin, not unlike Automated Teller Machine, although in this case it's the opposite of automated, as it's very much human operated. But this functionality might just as well be used to withdraw funds from the device, or to refund a customer. Therefore, it's more clear and in line with the other menu items to rename "ATM" to simply "Send Offline".
|
I've tested all the changes on the actual hardware, including all the menu entries. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.