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

fix: receive back button #2103

Merged
merged 2 commits into from
Feb 18, 2023
Merged

Conversation

manavdesai27
Copy link
Contributor

Describe the changes you have made in this PR

Changed back button functionality to go back to previous screen rather than home in receive screen.

Link this PR to an issue [optional]

Fixes #1542

Type of change

  • fix: Bug fix (non-breaking change which fixes an issue)

Screenshots of the changes [optional]

receive.back.button.mp4

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Checklist

  • My code follows the style guidelines of this project and performed a self-review of my own code
  • New and existing tests pass locally with my changes
  • I checked if I need to make corresponding changes to the documentation (and made those changes if needed)

@manavdesai27
Copy link
Contributor Author

@escapedcat @im-adithya I am closing the other pull request and opening this one in place of it, for a clean working tree.

@github-actions
Copy link

github-actions bot commented Feb 13, 2023

🚀 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.


This build is brought to you by: channel.ninja (who recently dropped 1000 sats):


Want to sponsor the next build? send some sats to ⚡️builds@getalby.com (don't forget to provide your name)

Don't forget: keep earning sats!

@im-adithya
Copy link
Member

im-adithya commented Feb 13, 2023

Can you please add me as a collaborator on your fork?

@manavdesai27
Copy link
Contributor Author

Can you please add me as a collaborator on your fork?

Sure I don't mind. But may I ask why?

@escapedcat
Copy link
Contributor

Sure I don't mind. But may I ask why?

@im-adithya didn't (doesn't yet?) have the permissions via the Alby team. The Alby team can pushg into your PR. He wanted to support you with the merghe/rebase I believe.

@manavdesai27
Copy link
Contributor Author

Sure I don't mind. But may I ask why?

@im-adithya didn't (doesn't yet?) have the permissions via the Alby team. The Alby team can pushg into your PR. He wanted to support you with the merghe/rebase I believe.

Ahh! That would be really helpful. But now I have just created new branch and new Pull Request. :)

@manavdesai27
Copy link
Contributor Author

@escapedcat Could you provide a review on this PR?

setPollingForPayment(false);
setInvoice(null);
}

Copy link
Member

Choose a reason for hiding this comment

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

We can DRY this up by using a function called

setDefaults() {
    setFormData({
      amount: "0",
      description: "",
      expiration: "",
    });
    setPaid(false);
    setPollingForPayment(false);
    setInvoice(null);
}

Copy link
Contributor Author

@manavdesai27 manavdesai27 Feb 17, 2023

Choose a reason for hiding this comment

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

Will do! And welcome back! XD

@@ -223,7 +238,7 @@ function Receive() {
title={t("title")}
headerLeft={
<IconButton
onClick={() => navigate("/")}
onClick={handleBack}
Copy link
Member

@im-adithya im-adithya Feb 17, 2023

Choose a reason for hiding this comment

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

Also, we can use it like this here:

onClick={() => {
    invoice ? setDefaults() : navigate("/");
}}

and in the receive again button:

onClick={() => {
    setDefaults();
    navigate("/receive");
}}

to avoid having handleBack and receiveAnotherPayment functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! @im-adithya

@bumi bumi merged commit cf18b36 into getAlby:master Feb 18, 2023
@manavdesai27 manavdesai27 deleted the fix/receive_back_button branch February 23, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking on back button should go to the previous window
4 participants