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

Adding edge to edge support #1607

Merged
merged 7 commits into from
Sep 2, 2024
Merged

Adding edge to edge support #1607

merged 7 commits into from
Sep 2, 2024

Conversation

dessalines
Copy link
Member

This was pretty easy to add, and mostly works well.

Some quirks mainly have to do with us not using proper material3 color scheming in a few places, but I'll start on that soon.

@dessalines dessalines requested a review from MV-GH as a code owner July 31, 2024 15:55
@MV-GH
Copy link
Collaborator

MV-GH commented Jul 31, 2024

I have attempted this before, and it didn't go well. Did you check this on every Android version, especially the old ones. Had very weird behaviour. see #1443

@dessalines
Copy link
Member Author

I just tested on my device (android 14). I have a few other apps that are just using it in the recommended way, and ppl don't seem to be having issues with it.

I suppose I could try to test a few others out.

@MV-GH
Copy link
Collaborator

MV-GH commented Jul 31, 2024

I'll check it this weekend on various verions.

@MV-GH
Copy link
Collaborator

MV-GH commented Aug 5, 2024

On android 10 and lower you have these issues

Extra bottom padding ??
image

image

The bottom app bar color doesn't match
image
image

Top bar icons stay dark
image

So yeah still plenty of issues

@dessalines
Copy link
Member Author

Dang. I'll try to investigate why the bottomappbar is doing that.

@dessalines
Copy link
Member Author

Okay I installed an android 10 emulator, and fixed the insets.

I couldn't replicate the icon color issue you're getting tho.

Screenshot_20240805_183512_AVNC

The bottomappbar color I think is unavoidable for lower android version, but its still usable.

@dessalines
Copy link
Member Author

Anything else I need to do with this one @MV-GH ?

@MV-GH
Copy link
Collaborator

MV-GH commented Aug 9, 2024

I'll test it more this weekend

@dessalines
Copy link
Member Author

bump on this one also @MV-GH

@MV-GH
Copy link
Collaborator

MV-GH commented Sep 2, 2024

I'll test it rn

@MV-GH
Copy link
Collaborator

MV-GH commented Sep 2, 2024

Can't find anything wrong. But users will definitely remark the jumping images

@dessalines
Copy link
Member Author

I'm fairly sure this has to do with the coil version bumps, not this. I'll test downgrading those.

@MV-GH
Copy link
Collaborator

MV-GH commented Sep 2, 2024

On Android 11

light theme looks like it has the wrong colors for the nagivation bar + statusbar

image

@dessalines dessalines merged commit 4720509 into main Sep 2, 2024
2 checks passed
@dessalines
Copy link
Member Author

The only thing I can think of is to put version checks on enableEdgeToEdge(). But since we're using this in the officially-documented way, we should also probably open up an issue on the google issue tracker.

@MV-GH
Copy link
Collaborator

MV-GH commented Sep 3, 2024

The only thing I can think of is to put version checks on enableEdgeToEdge().

I would only do this as a last resort.

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.

2 participants