Skip to content

[WIP] Fix #650 Broken back button #897

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adityamudgil2505
Copy link
Collaborator

@adityamudgil2505 adityamudgil2505 commented Mar 11, 2020

What's this PR do?
Solve the issue of the broken back button #650 . When someone goes to a setting option, then want to return back to the previous tab of the organization, then the user can do using the back button.

Any background context you want to provide?
This is being solved by adding a new variable that will keep track of the last tab(organization window) before going to the setting and when the user click on the back button then he will move to tab whose value is being stored in the variable.

GIF

Working

You have tested this PR on:

  • Windows
  • Linux/Ubuntu
  • macOS

@abhigyank abhigyank requested a review from punchagan March 12, 2020 16:08
@abhigyank
Copy link
Collaborator

@adityamudgil2505 Does this add working for back cases with in the settings tab?

@adityamudgil2505
Copy link
Collaborator Author

@abhigyank No, it will get back to organisation.

@abhigyank
Copy link
Collaborator

abhigyank commented Mar 18, 2020

I am not sure switching back to the organization after navigating a bit in the settings is a behavior that the user would expected. If the back is enabled in settings I suppose the user would then expect it to work with the settings window? I am not sure how the UX would be based on this change. Maybe someone else suggest if this'd be a good behavior. cc @rishig

@adityamudgil2505
Copy link
Collaborator Author

@abhigyank As both the ways have their own advantage and the way to solve this issue using both methods is similar.

@adityamudgil2505 adityamudgil2505 changed the title Fix #650 Broken back button [WIP] Fix #650 Broken back button Mar 19, 2020
@zulipbot zulipbot added size: M and removed size: S labels Mar 23, 2020
@adityamudgil2505
Copy link
Collaborator Author

adityamudgil2505 commented Mar 23, 2020

@andersk @abhigyank I updated the code. Now when the user opens the application after installing, then the back button will be disabled. After adding one or more organizations, it will work as what back button has to do. We can also use the back button in settings. The code is very messy, as there are many corner cases.
Back
Idea:
I created new data member in webview which will keep track of previous webview's index because we can move back inside webview without any problem but we can't move to different webview.
Now we have two types of movement

  1. When going in the forward direction, when we just update the previous index of the new webview with the previous one.
  2. When going in a backward direction, first we need to update the previous webview index of the current webview tab to -1 to avoid infinite loop and then move.

To handle this, I modify activateTab to add one more parameter which will tell whether we are going in forward direction or backward direction.
This will handle almost every case but it doesn't handle when we install the application for the first time, so I disable the back button there. We can enable the functionality of the back button in this case also but it requires some hard code.

Copy link
Collaborator

@manavmehta manavmehta left a comment

Choose a reason for hiding this comment

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

@adityamudgil2505 can you please fix the typo and reconsider the name of the getCurrentSettingWindowName() function as it can be used elsewhere too, afaik.
Otherwise LGTM

@manavmehta
Copy link
Collaborator

manavmehta commented Jul 1, 2020

@abhigyank Is this g2g or some work is left?
cc @adityamudgil2505

@zulipbot
Copy link
Member

Heads up @adityamudgil2505, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

Base automatically changed from master to main January 22, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants