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.

Sign up for GitHub

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

Jump to bottom

Enable back button if going from dapp to home & redirection problems #1472

Merged
merged 5 commits into from
Apr 16, 2020
Merged

Enable back button if going from dapp to home & redirection problems #1472

merged 5 commits into from
Apr 16, 2020

Conversation

andrepimenta
Copy link
Member

@andrepimenta andrepimenta commented Apr 3, 2020 •
edited
Loading

Description

This enables going back to the last url after you pressed home as mentioned in #1408. Also fixes a redirection problem, that happened sometimes when going to a dapp from home as mentioned on: #1408 comment.

(Note: While trying to fix this, there were other small issues found with the navigation that are not addressed on the issue #1408 so we should test the browser navigation a bit more and maybe create another issue for refactoring it.)

Issue

Resolves #1408

@andrepimenta andrepimenta added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Apr 3, 2020
danjm
danjm requested changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

Left one question about a case we might want to handle. But overall, these change look correct to me.

app/components/Views/BrowserTab/index.js
if(this.isHomepage() && lastUrlBeforeHome){
this.go(lastUrlBeforeHome);
}else{
current && current.goBack();
Copy link
Contributor

Choose a reason for hiding this comment

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

If this.isHomepage() is true and lastUrlBeforeHome is false, is it possible that current.goBack() could be called when there is nowhere to go back to? I am thinking of when this.handleAndroidBackPress is called.

@andrepimenta andrepimenta requested a review from danjm April 7, 2020 20:23
@ibrahimtaveras00
Copy link
Contributor

Positive test cases (happy flow) looks good

Just a few issues I saw when comparing to what's on playstore and testflight (version 0.2.14 389)

Issue 1:
These issues are only happening on Android, iOS works fine

When on homepage, then navigating to a dapp via "Featured Projects", searching via search engine (duck duck go or google), or navigating directly to a URL, then pressing back button, the forward button does not appear as an option.

FYI Screen casts have the emulator with this branch on left, and real device with playstore on the right...

Via Featured Projects seen here = https://recordit.co/yZF2jPKcit

Steps to reproduce:

  • on an android device go to browser and start on the homepage
  • tap on a dapp from Featured Projects
  • once on the dapp page, tap on the back arrow
  • once back on the homepage, notice the forward arrow is not enabled

Via search engine seen here = https://recordit.co/2JtrEla5yL

Steps to reproduce:

  • on an android device go to browser and start on the homepage
  • in the search text input field search anything or go directly to a URL
  • now tap the back arrow
  • once back on the homepage, notice the forward arrow is not enabled

Issue 2:
This one happens on both Android and iOS

When first arriving on homepage, or opening a new tab, and tapping the home button, the back arrow is enabled, and then tapping it shows the forward arrow, which keeps you on the homepage when tapping either.

Seen here = https://recordit.co/Flfx2NThCf

steps to reproduce:

  • navigate to browser or just open a new tab
  • tap the home button
  • notice the back arrow is enabled, now tap the back arrow and the forward arrow will be enabled

ibrahimtaveras00
ibrahimtaveras00 approved these changes Apr 16, 2020
View reviewed changes
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

Fixes look good on both OS's, QA Passed 👍

@andrepimenta andrepimenta merged commit 25e3910 into develop Apr 16, 2020
@andrepimenta andrepimenta deleted the improvement/back-button-browser-issue-1408 branch April 16, 2020 12:28
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
@andrepimenta
Enable back button if going from dapp to home & redirection problems (#…
32f95f9 
…1472)

* Enable back button if going from dapp to home & redirection problems

* fix for android

* prevent going back when it's not possible

* Fix can go back function and go forward
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibrahimtaveras00 ibrahimtaveras00 ibrahimtaveras00 approved these changes

@danjm danjm danjm approved these changes

Assignees
No one assigned
Labels
needs-qa Any New Features that needs a full manual QA prior to being added to a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable back button if going from dapp to home
3 participants
@andrepimenta @ibrahimtaveras00 @danjm