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: Inherit from Material 3 themes and round corners on box art #10532

Merged
merged 1 commit into from Mar 27, 2022

Conversation

t895
Copy link
Contributor

@t895 t895 commented Mar 24, 2022

In order to retain the app's original look, I had to import the back button clip art and change the title bar text color in the settings theme because there isn't a Theme.Material3.DayNight.DarkActionBar.

I also made the box art for games fit with their associated cardviews so they appear rounded.

@Simonx22
Copy link
Member

Please provide screenshots for UI changes.

@t895
Copy link
Contributor Author

t895 commented Mar 24, 2022

Screenshot_20220324-014923
Screenshot_20220324-014918
Screenshot_20220324-014913
Edit -
Screenshot_20220324-015156
Screenshot_20220324-015314

@MayImilae
Copy link
Contributor

Small nitpick, but the covers are too close to the console selection tabs at the top. It makes the white line tab indicator look almost like the selection for the games and just looks kind of broken. Moving the covers a bit downward away from the tabs will address that.

@t895
Copy link
Contributor Author

t895 commented Mar 24, 2022

@MayImilae The app has always had the games close to the tabs like that but I could add additional padding if necessary

Example in the current build -
IMG_0194

@t895
Copy link
Contributor Author

t895 commented Mar 24, 2022

On a second look though, it would be a good idea to tighten the cards a bit more because there is still a small space on box art with a base color other than white. If I really wanted to round the box art properly I could import another library like Glide to do that for me but that could just be too extra.

@MayImilae
Copy link
Contributor

The app has always had the games close to the tabs like that but I could add additional padding if necessary

Ah, I see why. The shadowing under the blue top bar in the old style helps give it separation that doesn't exist in the Material 3 version. A little extra padding should fix that.

@t895
Copy link
Contributor Author

t895 commented Mar 24, 2022

With new commits -
signal-2022-03-24-123941_001

@JosJuice
Copy link
Member

Not entirely sure how I feel about the rounded upper corners on the cover art... But maybe that's just because of how used I am to seeing the original physical GC/Wii game boxes.

Code changes look OK. I'd like to let May have the final say on the design (if she does want to have the final say on Android design).

@t895
Copy link
Contributor Author

t895 commented Mar 24, 2022

@MayImilae Just let me know if you want more changes to the existing look, but I've been wondering. The styles.xml way of handling themes currently implemented really prevents some cool color customization work for a full on dynamic theme (>API30). If I made a pull request for changing to a system surrounding themes.xml files, would it be up for consideration?

The reason I ask is because I heard somewhere that larger commits aren't likely to be accepted.

@JosJuice
Copy link
Member

Larger commits aren't forbidden per se. If it's a good change and there's no reasonable way to break it into smaller commits (which seems to be the case here on a first impression), it's okay, but it may take longer to get reviewed.

But either way, if you want to do this, it should be in a separate pull request.

(FWIW, right now I know almost nothing about what dynamic themes are and should probably read up on them :))

@t895
Copy link
Contributor Author

t895 commented Mar 24, 2022

This is a pretty good guide on how the colors work in Material 3 and when done right you can maintain the Dolphin-Blue color while introducing wallpaper-generated ones.
https://m3.material.io/styles/color/dynamic-color/overview

And thanks a lot for the information! @JosJuice

@JosJuice
Copy link
Member

Ah, right, the new color system in Android 12. I don't have any personal opinion on whether we should use it, but I have nothing against doing a larger refactoring of how we handle themes, if it's necessary for dynamic themes or even just if it makes the code more maintainable.

(Just to clarify, since I think you're new to contributing to Dolphin: I'm the main person responsible for the Android code, and May is the main person responsible for design, but she doesn't specialize in Android.)

@t895
Copy link
Contributor Author

t895 commented Mar 24, 2022

I am new, and that's really helpful to know. Happy to meet you both and I hope to contribute more in the near future.

@t895
Copy link
Contributor Author

t895 commented Mar 26, 2022

I meant I fixed this gap
159851052-edce2647-cb74-4dcd-a104-acfd13c36cf4

Everything about this current screenshot
image

is fine

Use large card view rounded corner guidelines

Fix action bar theming

Needed to import android back button clip art to fix material 3 theming issue. The DolphinSettingsBase style used to inherit from the Theme.MaterialComponents.DayNight.DarkActionBar theme which would provide the light text and icons but this is no longer available with Material 3.

Fit box art more snugly in CardView

Change card height to match cover art

Add padding to top of games list recyclerview
@t895
Copy link
Contributor Author

t895 commented Mar 26, 2022

@JosJuice Just squashed and ready to commit

@JosJuice JosJuice merged commit f12681b into dolphin-emu:master Mar 27, 2022
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants