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: disable back action by gesture after accepting terms and conditions #1356

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fc-santos
Copy link
Contributor

Summary of Changes

This PR fixes a bug where after accepting the Terms & Conditions the user is able to go back from the PINCreate Screen and then is stuck on Terms & Conditions Screen. Only killing app gets the user back on the PINCreate screen.

Screenshots, videos, or gifs

N/A

Breaking change guide

N/A

Related Issues

N/A

Pull Request Checklist

Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.

  • All commits contain a DCO Signed-off-by line (we use the DCO GitHub app to enforce this)
  • If applicable, screenshots, gifs, or video are included for UI changes
  • If applicable, breaking changes are described above along with how to address them
  • Updated documentation as needed for changed code and new or modified features
  • Added sufficient tests so that overall code coverage is not reduced

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Pro Tip 🤓

  • Read our contribution guide at least once; it will save you a few review cycles!
  • Your PR will likely not be reviewed until all the above boxes are checked and all automated checks have passed

@cvarjao
Copy link
Contributor

cvarjao commented Dec 12, 2024

@fc-santos , can you elaborate a bit more on the problem. Is that on iOS, Android, both? is that on Bifold or is that on your own implementation of a wallet using bifold? It seems like getting stuck on a screen by going back seems the problem.

@fc-santos
Copy link
Contributor Author

fc-santos commented Dec 12, 2024

@cvarjao it's meant to fix this problem (occurs on both Android and iOS):

cropped.mp4

Copy link

sonarcloud bot commented Dec 12, 2024

@cvarjao
Copy link
Contributor

cvarjao commented Dec 12, 2024

I might be missing something, I don't think removing the ability to go back fix the problem, probably a workaround. It shouldn't get stuck as you are seeing. I think that is the problem that needs to be fixed

@bryce-mcmath
Copy link
Contributor

bryce-mcmath commented Dec 12, 2024

@fc-santos adding to what Clecio said, I suspect it has to do with the checkbox no longer existing when you come back to the Terms screen. Not having it checked is probably preventing the user from continuing. Probably better to fix that underlying bug that just preventing back nav. Might be as simple as changing line 23 of Terms.tsx to:

const [checked, setChecked] = useState<boolean>(store.onboarding.didAgreeToTerms ?? false)

Edit: Or you could simply remove the checkbox. We don't use one in our injected version of the Terms screen.

@fc-santos
Copy link
Contributor Author

@bryce-mcmath I agree that we should do what you suggested, but at the same time do we want to enable the user to go back though? The user has already accepted the conditions, why are allowing it to be accepted again?

@fc-santos
Copy link
Contributor Author

fc-santos commented Dec 13, 2024

@fc-santos adding to what Clecio said, I suspect it has to do with the checkbox no longer existing when you come back to the Terms screen. Not having it checked is probably preventing the user from continuing. Probably better to fix that underlying bug that just preventing back nav. Might be as simple as changing line 23 of Terms.tsx to:

const [checked, setChecked] = useState<boolean>(store.onboarding.didAgreeToTerms ?? false)

Edit: Or you could simply remove the checkbox. We don't use one in our injected version of the Terms screen.

@bryce-mcmath To answer your point about removing the checkbox, it's a legal requirement for us to have it.

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.

4 participants