lightning-browser-extension
lightning-browser-extension copied to clipboard
fix: remove Remember & set budget from send #1510
Describe the changes you have made in this PR
Remove "Remember and set a budget" option from send screen when opting for lightning invoice
Link this PR to an issue
Fixes #1510
Type of change (Remove other not matching type)
-
fix
: Bug fix (non-breaking change which fixes an issue)
Screenshots of the changes (If any)
-
Previous behaviour
-
After removal of "Remember and set a budget"
How has this been tested?
Manual Testing
- I used ln invoice from Phoenix and tapped Send on popup and website, which shows "Remember and set a budget" but with the changes it has been removed
- Lightning invoice used: lnbc1m1p3ngewjpp578s6uh2gvmsuldc7khppc57spggzjuy9aj0cg4633put5x20p6cqdqqxqyjw5q9q7sqqqqqqqqqqqqqqqqqqqqqqqqq9qsqsp5e5a6h39y85fnvlenly6dmfx97y32tru4hkn84x0wcgjf8egyfk4qrzjqwryaup9lh50kkranzgcdnn2fgvx390wgj5jd07rwr3vxeje0glclll0mxh6jgh2xuqqqqlgqqqqqeqqjqfs7gjqntggjnpgxp9rjh78gx2cczjuha65wdz9w2462chm8a7d78s2gwpdndsra4lf7940dh03dw67xcr8dd605lm68j6pusjj7n9tspm0mtde
Checklist
- [x] My code follows the style guidelines of this project and performed a self-review of my own code
- [x] New and existing tests pass locally with my changes
- [x] I checked if I need to make corresponding changes to the documentation (and made those changes if needed)
Does it still maintain the Remember & set budget for the other cases, right?
Does it still maintain the Remember & set budget for the other cases, right?
I noticed it removed the set budget for other cases like subscribing to some project on geyser.fund. I am rechecking the codebase and cases where it should and should not show the set budget option.
Edit: Figured out above problem, working on tests...
The issue should have more specific, sorry.
The important part here is, that we do not just want to remove this.
For example in the prompt case we will always need it:
- Go to https://nakaphoto.vercel.app/accounts/[email protected]
- Click on a picture
- Prompt shows up, click "connect"
- Here we need this functionality:
Try if this works:
- This block needs to be hidden if the extension-popup is being used but shown if the prompt is being used: https://github.com/getAlby/lightning-browser-extension/blob/master/src/app/screens/ConfirmPayment/index.tsx/#L135
- Try moving the
!navState.origin
out and put this with anif
around the block. Because only using the prompt we have anorigin
https://github.com/getAlby/lightning-browser-extension/blob/master/src/app/screens/ConfirmPayment/index.tsx/#L104
Does this help? Let me know if you have any questions.
For example in the prompt case we will always need it:
Yeah, I got that one. I was still figuring out for it's tests ... That's why I did some rough changes in a separate branch to hide the option when someone is using send, and show when in case as described above https://github.com/vaibhavgarg237/lightning-browser-extension/commit/e685ce8af043d37840b811bb15b9c60e07410380
🚀 Thanks for the pull request!
Here are the current build files for testing:
Download and unzip the file for your browser. Refer to the readme for detailed install instructions.
Don't forget: keep stacking sats!
@vaibhavgarg237 tests are green here 🚀
Might be good to add a test for your change. I can help with that.
@vaibhavgarg237 tests are green here 🚀 Might be good to add a test for your change. I can help with that.
I think tests are passing because it's only checking for demo page as it is creating a OriginData in starting itself. I think we need to add another test that only runs when in condition when the origin is not present and the current one should only run when in presence of origin.
I was facing some issues related to running original tests locally. I have dm'ed you on slack regarding the same, kindly lemme know whenever you are available. Thanks
ack