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

Android: Do not fit system windows #11079

Merged
merged 1 commit into from Oct 23, 2022
Merged

Conversation

t895
Copy link
Contributor

@t895 t895 commented Sep 22, 2022

In order to have more control over each layout and how they appear in a given window, I have set each activity to not fit system windows. I have listeners set to apply the correct insets based on the size of system bars given notches, cameras, etc. There are only a small number of visual changes, but this should help to create more immersive layouts in the future.

Notable visual changes

List items now appear behind a semi-transparent navigation bar. The scrim around the navigation bar is the surface color used in the app. This prevents a previous issue where on API 29 or higher, content protection could activate and show a scrim with a system-provided color and not the app-provided one.
image

Cheats Activity

While adjusting each activity, I noticed that the Cheats Activity was lacking some visual separation between the app bar and the cheats list. Additionally the action buttons got a divider in the editor.

Before -
image
image

After -
image
image

@mbc07
Copy link
Contributor

mbc07 commented Sep 23, 2022

Items from long lists going under a semi-transparent navigation bar seems really odd. I've switched to gesture navigation long ago, but I momentarily reverted back to the 3 button navigation and couldn't find a single app from the ~80 I have on my phone that had this semi-transparent behavior on long lists. Other than that, this LGTM...

@K0bin
Copy link
Contributor

K0bin commented Sep 23, 2022

I've switched to gesture navigation long ago, but I momentarily reverted back to the 3 button navigation and couldn't find a single app from the ~80 I have on my phone that had this semi-transparent behavior on long lists.

There's basically no difference between going under the small margin that has the gesture bar or the 3 button navigation bar. It's the same thing from Androids POV. Unfortunately most apps don't even bother doing this correctly.

@t895
Copy link
Contributor Author

t895 commented Sep 23, 2022

There are very few apps that have this behavior. Most of the time they will opt to fit within system windows or use the system content protection to manage it for them. This PR keeps things consistent across versions and if we ever wanted to change that behavior, we now have more control. (One great app that uses this technique is Tachiyomi)

@MayImilae
Copy link
Contributor

MayImilae commented Sep 24, 2022

I'm quite surprised people still use the 3 button navigation bar, it seems quite clunky compared to gestures but, eh.

As for this change... I have some concerns. The visuals are ok, like, I'd prefer the nav bar to be a bit higher opacity for clarity, but it's still legible. However, this change creates a bit of weirdness as to when a checkbox is checkable. Like, despite the visuals, the nav bar is going to have a defined hitbox, and any Dolphin GUI bits under that hitbox are just going to be inaccessible and require the user to scroll to reach them, right? But where does the nav bar hitbox begin and end? ┐(´-`)┌ There's no way to tell, and that is a functionality downgrade. Not a big one mind you, but it is one.

EDIT: (≖ ‸ ≖) After squinting a bit, there does appear to be a defined line when the navigation ends and Dolphin's GUI (controllably anyway) begins. But it is SO hard to see in a still like this. If you could show me a video of it scrolling slowly, that would let me see the division clearly, and I'll better be able to access the functionality implications of this change.

@t895
Copy link
Contributor Author

t895 commented Sep 24, 2022

Here's a video -
https://user-images.githubusercontent.com/14132249/192098427-51d00a05-6695-4242-ace8-e6e0779a3ec9.mp4

I don't use the 2 or 3 button navigation myself but I like to keep all options supported. Regardless you CAN press buttons behind the navigation bar and now that you mention it, I don't really like that either. Visually I'm happy with it but if I could disable touch this would be perfect imo.

@mbc07
Copy link
Contributor

mbc07 commented Sep 24, 2022

Couldn't you somehow keep the original behavior (solid navigation bar) without fitting system windows? At first I thought it was just a visual change, but being able to press checkboxes behind the navigation bar makes it even worse...

@t895
Copy link
Contributor Author

t895 commented Sep 24, 2022

I did some work looking into the issue and it turns out that interaction with UI under the navigation bar is a bug in the latest version of Android (which is <1% of users). I filed a bug report here and hopefully this gets fixed or we get more options to adjust this behavior. With that said, I still think this looks good design-wise and it's still worth merging without the Android system fix.

@mbc07
Copy link
Contributor

mbc07 commented Sep 25, 2022

[...] and it's still worth merging without the Android system fix.

Um, I don't think that's OK. Android 13 might represent just a fraction of the userbase right now, but it will eventually grow a lot once new devices shipping with it from factory and system updates for existing devices starts rolling out, and as of now there's no reasonable time frame of when this issue will get fixed (or even if it will get fixed)...

@JosJuice
Copy link
Member

JosJuice commented Sep 25, 2022

For comparison, there's a bug in Android 11 that was fixed in Android 12 that in some cases makes Dolphin's input handling not work. We've gotten tons of support requests about this, most of them after Android 12 was released, because phones simply do not get timely updates. Though, I do suppose that the issue in this pull request is less severe.

@K0bin
Copy link
Contributor

K0bin commented Sep 25, 2022

Um, I don't think that's OK. Android 13 might represent just a fraction of the userbase right now, but it will eventually grow a lot once new devices shipping with it from factory and system updates for existing devices starts rolling out, and as of now there's no reasonable time frame of when this issue will get fixed (or even if it will get fixed)...

And the issue will affect hundreds of apps. I think it's a very minor issue and I honestly don't see why Dolphin in particular has to work around it.

@t895
Copy link
Contributor Author

t895 commented Sep 25, 2022

For comparison, there's a bug in Android 11 that was fixed in Android 12 that in some cases makes Dolphin's input handling not work. We've gotten tons of support requests about this, most of them after Android 12 was released, because phones simply do not get timely updates. Though, I do suppose that the issue in this pull request is less severe.

Fair point, I shouldn't be naïve about how poorly Android updates are distributed. I'll get started on a workaround. I'm sure I could create a view that blocks touches when the navigation bar appears in portrait mode.

@t895
Copy link
Contributor Author

t895 commented Sep 25, 2022

Just finished a workaround for the Android 13 bug by placing a view on top of the navigation bar that blocks inputs. Everything should be good to go now.

@t895 t895 force-pushed the system-windows branch 2 times, most recently from 4d2b61d to b6ed009 Compare September 26, 2022 02:19
@t895
Copy link
Contributor Author

t895 commented Sep 26, 2022

I should have done this earlier, but when using gesture navigation, the scrim around the navigation bar is removed. The gesture bar will change color based on the background automatically.

example

@t895
Copy link
Contributor Author

t895 commented Oct 11, 2022

After talking to @MayImilae I've increased the opacity of the navigation bar. The new looks are now reflected in the OP.
image

@MayImilae
Copy link
Contributor

All of my issues have been addressed!

Seeing the images now, my only complaint was the colour difference between the white in nav bar and the white of the page but t895 tells me that's a temporary bug that already has a solution. So with that, LGTM.

@t895
Copy link
Contributor Author

t895 commented Oct 12, 2022

Just to clarify, there is a bug in master where the navigation bar color would be controlled by the system or reset to the base theme value in certain situations. This is fixed as of this PR by disabling content protection and applying the correct navigation bar color in more places.

@JosJuice
Copy link
Member

Not a big fan of how much code this adds for what seems like relatively little benefit, but... Eh, I guess we can always revert it if the maintenance burden becomes too much

@JosJuice JosJuice merged commit d442f3f into dolphin-emu:master Oct 23, 2022
11 checks passed
@t895 t895 deleted the system-windows branch January 1, 2023 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants