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

Back Button on Mobile Devices should close current Modal #4264

Open
Nutomic opened this issue Jul 20, 2017 · 13 comments
Open

Back Button on Mobile Devices should close current Modal #4264

Nutomic opened this issue Jul 20, 2017 · 13 comments
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)

Comments

@Nutomic
Copy link
Contributor

Nutomic commented Jul 20, 2017

When using Syncthing from a mobile browser, pressing the back button should close the current modal. At the moment, pressing the back button will always navigate to the previous page (away from Syncthing). To me, that isn't the expected behaviour. Tested with latest Firefox and Chrome on Android.

Version Information

Syncthing Version: v0.14.32
OS Version: Android 6.0
Browser Version: Firefox 54, Chrome 59

Nutomic added a commit to syncthing/syncthing-android that referenced this issue Jul 20, 2017
@calmh
Copy link
Member

calmh commented Jul 20, 2017

I don't think that's the expected behavior at all. For example, here in GitHub, if you click the gear besides "Assignees", "Labels", etc to the right to bring up a modal (albeit a smaller one than ours) and then hit back, you will leave the page. None of the web apps I have around me at the moment that have this kind of in-page modal have the back button behavior you describe...

@AudriusButkevicius
Copy link
Member

Well the in page modals block the rest of the page (not just a pop out) so it somewhat makes sense.

@AudriusButkevicius
Copy link
Member

AudriusButkevicius commented Jul 20, 2017

Perhaps the right workaround is on android, asking people to confirm exiting web view with a second back press (while the warning toast is up). There is no reason to back in the web view as it's an SPA, and I've seem some apps implement this.

@calmh
Copy link
Member

calmh commented Jul 20, 2017

When presenting the UI as an "app", I don't think there should be a back button at all? Where does it lead anyway?

The github modals I point to on the right also block the rest of the page exactly like ours do, they just don't fade out the background.

@AudriusButkevicius
Copy link
Member

Or rather how do you get out of the web ui if back is highjacked by the page?

@calmh
Copy link
Member

calmh commented Jul 20, 2017

I haven't seen the Android UI so can't recommend. When presenting a web UI as an app on desktop you typically close the window when done with it. If the Syncthing web UI is presented as a modal or view in the Android app I would assume there is a "close", "done", or similar button that closes the web view and goes back to wherever you came from. I would not expect there to be a "back" button that does something useful within the web UI.

@Ferroin
Copy link

Ferroin commented Jul 20, 2017

In the Android app, you have both the system UI mandated back button (which based on other experience I would expect to close the modals, but unfortunately that's probably not possible), and then a separate back button in the header. Both of them take you back to the regular Syncthing Android UI.

When just connected through a mobile browser, you only have the system UI back button, which needs to behave consistently with other web pages, and therefore needs to go to the previous page.

@AudriusButkevicius
Copy link
Member

I am sure that doing something what is asked in this ticket is possible, I am arguing that it might not be the best behaviour for the back button. We could conditionally enable these on mobiles, but I think double back to confirm is a lower hanging fruit.

@calmh
Copy link
Member

calmh commented Jul 20, 2017

I have no strong opinion either way. Mapping "back" to "close dialog" in all cases might be unexpected for some (me) but hardly a big deal either way, it's not like there's any harm done.

@Ferroin
Copy link

Ferroin commented Jul 20, 2017

I think the issue is really when dealing with small screens (like mobile devices) where the dialogs take up almost all of the screen, and it's not always easy to quickly back out by tapping the background. On this count, I would tend to agree with @AudriusButkevicius on this one, at a minimum if this is implemented, it makes the most sense to restrict it to mobile devices, but just requiring the user to hit the back button twice to actually go back would be sufficient too, and probably much quicker to implement.

@Nutomic
Copy link
Contributor Author

Nutomic commented Jul 21, 2017

To explain this a bit more, I think the back button should only close a dialog if one is open. If no dialog is opened, the back button would have the normal behaviour (going to the previous page). So even if a dialog is open, you could always go to the previous page by pressing the back button twice.

@calmh calmh added the enhancement New features or improvements of some kind, as opposed to a problem (bug) label Aug 8, 2017
@calmh calmh added this to the Unplanned (Contributions Welcome) milestone Aug 8, 2017
@calmh
Copy link
Member

calmh commented Aug 8, 2017

Y'all are welcome to try something smart here

@carstenhag
Copy link

In apps with browsers in side them, like Relay for Reddit, a press on the back button just returns you to the main view.

Many apps require you to confirm that you fully want to leave an app when pressing the back button. I think this should be implemented inside the webview/browser of syncthing-android, not syncthing-gui.

Quick edit to show what I mean:

@calmh calmh removed this from the Unplanned (Contributions Welcome) milestone Feb 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)
Projects
None yet
Development

No branches or pull requests

5 participants