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

Better implementation of Material You #1594

Closed
wants to merge 3 commits into from
Closed

Conversation

gcantoni
Copy link

@gcantoni gcantoni commented Feb 8, 2025

Hello,

these commits include several changes to the core and non-core layouts of SD Maid. At present the app does not implement a good part of the colors defined in colors within its layouts. For example, the main backgrounds and card backgrounds are not used correctly.

Example - layout/dashboard_fragment.xml:

<?xml version="1.0" encoding="utf-8"?> <androidx.coordinatorlayout.widget.CoordinatorLayout xmlns:android="http://schemas.android.com/apk/res/android" xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:tools="http://schemas.android.com/tools" android:background="?attr/colorSurfaceContainer" android:layout_width="match_parent" android:layout_height="match_parent">

Attribute android:background="?attr/colorSurfaceContainer" was missing.

Another example - setup_shizuku_item.xml:

<?xml version="1.0" encoding="utf-8"?> <com.google.android.material.card.MaterialCardView xmlns:android="http://schemas.android.com/apk/res/android" xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:tools="http://schemas.android.com/tools" style="@style/DashboardCardItem" app:cardCornerRadius="@dimen/cardRadius" app:cardBackgroundColor="?attr/colorSurface" android:layout_width="match_parent" android:layout_height="wrap_content">

Both attributes cardCornerRadius and cardBackgroundColor were missing.

At the same time, some changes have been made to the styles and themes of the app to ensure a better implementation of basic theme and Material You.

The implementation for the basic theme (NO Material You) is much better but somehow some colors related to the toolbars are hardcoded on the code side. (in particular some menu icons - see attached screeshots)

Material You's implementation follows similar with the exception of the status bar and navigation bar. It would be better to avoid defining hardcoded elements (such as the color of the status bar here https://github.com/d4rken-org/sdmaid-se/blob/main/app/src/main/java/eu/darken/sdmse/common/theming/Theming.kt#L125)

Some screenshots

  • Basic theme

Screenshot_20250208-142731
Screenshot_20250208-142747
Screenshot_20250208-142752
Screenshot_20250208-142803
Screenshot_20250208-142809
Screenshot_20250208-142841
Screenshot_20250208-142915
Screenshot_20250208-142924
Screenshot_20250208-142942
Screenshot_20250208-142945

@gcantoni
Copy link
Author

gcantoni commented Feb 8, 2025

Before
Screenshot_20250208-150525.png

Now
Screenshot_20250208-142841.png

Before
Screenshot_20250208-150538.png

Now
Screenshot_20250208-142915.png

Please note how all the Material Design 3 colours are implemented. The difference is huge.

Copy link
Member

@d4rken d4rken left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the PR!

Could you please remove all the white-space only changes?
That would make reviewing a lot easier.
There is also a new top-level values folder, can you tell me more about that?

@d4rken
Copy link
Member

d4rken commented Feb 8, 2025

Another example - setup_shizuku_item.xml:
Both attributes cardCornerRadius and cardBackgroundColor were missing.

Isn't background color and cardCornerRadius (or rather shape), already defined through the style which inherits from the styles provided by the Material3 dependency?

<style name="DashboardCardItem" parent="Widget.Material3.CardView.Elevated">

The implementation for the basic theme (NO Material You) is much better but somehow some colors related to the toolbars are hardcoded on the code side. (in particular some menu icons - see attached screeshots)

I think I did that because the icon colors did not display correctly on the background. How Would you fix that?

Material You's implementation follows similar with the exception of the status bar and navigation bar. It would be better to avoid defining hardcoded elements (such as the color of the status bar here https://github.com/d4rken-org/sdmaid-se/blob/main/app/src/main/java/eu/darken/sdmse/common/theming/Theming.kt#L125)

Yeah that's not ideal, I think I also fixed that in #1469

@gcantoni gcantoni closed this by deleting the head repository Feb 9, 2025
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