Add resources module with themes/styles by ricknout · Pull Request #27 · ataulm/muvi

ricknout · 2019-08-11T17:45:00Z

This adds a new module - :resources - which is now an :app dependency, allowing it to also be used by dynamic feature modules.

The :resources module contains:

App themes that inherit from MDC-Android themes
Values for color, type and shape theming
A downloadable font: https://fonts.google.com/specimen/Poppins
A ThemePlaygroundActivity (which can now be launched from MainActivity)
Dark theme compatibility via DayNight and the -night resource qualifier

Note: Both the color palette and custom font were chosen by inspecting values on https://letterboxd.com. These can be fairly easily changed if necessary.

resolves #26

ataulm

Thanks so much man! I literally thought that adding a theme would be like 1-2 files.

The playground is above and beyond ✊ I'll leave for @hitherejoe to take a look tomorrow and then maybe we can merge, and I'll raise my PR that integrates with this new theme for you to peek at?

ataulm · 2019-08-11T18:52:50Z

resources/src/main/java/com/muvi/resources/ThemePlaygroundActivity.kt

+
+import androidx.appcompat.app.AppCompatActivity
+
+class ThemePlaygroundActivity : AppCompatActivity(R.layout.activity_theme_playground)


ataulm · 2019-08-11T18:53:51Z

resources/build.gradle

+    implementation libraries.androidxAppCompat
+    implementation libraries.androidxConstraintLayout
+    implementation libraries.kotlinStdLib
+    implementation libraries.materialComponents


ataulm · 2019-08-11T18:56:28Z

app/src/main/java/com/muvi/MainActivity.kt

+import com.muvi.navigation.themePlaygroundIntent

-class MainActivity : AppCompatActivity() {
+class MainActivity : AppCompatActivity(R.layout.activity_main) {


ataulm · 2019-08-11T18:57:57Z

app/build.gradle

    implementation project(':core')
    implementation project(':feed')
+    implementation project(':navigation')
+    implementation project(':resources')


👍 we pulled out the feed stuff into a library module because we didn't want logic in app.

I think pulling in navigation and resources are fine to support the "meta" ThemePlaygroundActivity

dependencies.gradle

ataulm · 2019-08-11T19:01:31Z

resources/src/main/AndroidManifest.xml

+
+    <application>
+
+        <activity android:name=".ThemePlaygroundActivity" />


In a real life app, I guess this Manifest would exist in both the main and debug sourcesets so we only had ThemePlaygroundActivity in debug, but the meta-data block would be alright here since the theme needs it?

yea I think we could get away with it in debug, theoretically, unless I'm missing something!

ataulm · 2019-08-11T19:02:28Z

resources/src/main/res/drawable/ic_android_24dp.xml

+        android:height="24dp"
+        android:viewportWidth="24.0"
+        android:viewportHeight="24.0"
+        android:tint="?attr/colorOnPrimarySurface">


ataulm · 2019-08-11T19:05:18Z

resources/src/main/res/layout/activity_theme_playground.xml

+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:paddingBottom="16dp"
+            android:clipToPadding="false">


is this needed for something?

Do you pull copy these activity theme playground from somewhere?

Is it something that could exist as a library that could be added to any project? Where it would take the correct theme from the app?

This is the layout for ThemePlaygroundActivity.

Yep, I copied it from my own playground layout in a MDC-Android sample code repo:
https://github.com/ricknout/android-mdc-theming/blob/master/app/src/main/res/layout/activity_theming.xml

It could definitely form part of a library in future 👍 agree it does bloat the PR a bit for now though

is this needed for something?

I meant the clip to padding false 😅

Oh lol, so the components on the playground screen can scroll past the bottom padding without being clipped

ataulm · 2019-08-11T19:06:28Z

resources/src/main/res/values-night/colors.xml

+<?xml version="1.0" encoding="utf-8"?>
+<resources>
+
+    <!-- Theme colors -->


I guess I need to upgrade to Q now to test this out 🆗 👌

ataulm · 2019-08-11T19:08:22Z

resources/src/main/res/values/font_certs.xml

+            MIIEqDCCA5CgAwIBAgIJANWFuGx90071MA0GCSqGSIb3DQEBBAUAMIGUMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTAeFw0wODA0MTUyMzM2NTZaFw0zNTA5MDEyMzM2NTZaMIGUMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTCCASAwDQYJKoZIhvcNAQEBBQADggENADCCAQgCggEBANbOLggKv+IxTdGNs8/TGFy0PTP6DHThvbbR24kT9ixcOd9W+EaBPWW+wPPKQmsHxajtWjmQwWfna8mZuSeJS48LIgAZlKkpFeVyxW0qMBujb8X8ETrWy550NaFtI6t9+u7hZeTfHwqNvacKhp1RbE6dBRGWynwMVX8XW8N1+UjFaq6GCJukT4qmpN2afb8sCjUigq0GuMwYXrFVee74bQgLHWGJwPmvmLHC69EH6kWr22ijx4OKXlSIx2xT1AsSHee70w5iDBiK4aph27yH3TxkXy9V89TDdexAcKk/cVHYNnDBapcavl7y0RiQ4biu8ymM8Ga/nmzhRKya6G0cGw8CAQOjgfwwgfkwHQYDVR0OBBYEFI0cxb6VTEM8YYY6FbBMvAPyT+CyMIHJBgNVHSMEgcEwgb6AFI0cxb6VTEM8YYY6FbBMvAPyT+CyoYGapIGXMIGUMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbYIJANWFuGx90071MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEEBQADggEBABnTDPEF+3iSP0wNfdIjIz1AlnrPzgAIHVvXxunW7SBrDhEglQZBbKJEk5kT0mtKoOD1JMrSu1xuTKEBahWRbqHsXclaXjoBADb0kkjVEJu/Lh5hgYZnOjvlba8Ld7HCKePCVePoTJBdI4fvugnL8TsgK05aIskyY0hKI9L8KfqfGTl1lzOv2KoWD0KWwtAWPoGChZxmQ+nBli+gwYMzM1vAkP+aayLe0a1EQimlOalO762r0GXO0ks+UeXde2Z4e+8S/pf7pITEI/tP+MxJTALw9QUWEv9lKTk+jkbqxbsh8nfBUapfKqYn0eidpwq2AzVp3juYl7//fKnaPhJD9gs=
+        </item>
+    </string-array>
+    <string-array name="com_google_android_gms_fonts_certs_prod">


Where are these generated from? Is there any issue with these being in the open? (I guess since they'll need to be in the APK, they're not secret secret).

These are auto-generated when going through the Android Studio downloadable font wizard. I think it's fine, I've done something similar in some of my own repos. Alternatively we could package the font files and use local XML fonts, but that would have an impact on APK size and maybe the complexity of this PR. Thoughts?

works for me, as long as it's not like tied to your account and cause you problems or anything

ataulm · 2019-08-11T23:00:18Z

:resources - which is now an :app dependency, allowing it to also be used by dynamic feature modules.

🤔 resources is an implementation dependency of app so my expectation was that resources would not be available to anyone except app.

In film_detail (DFM), it can't resolve the theme in the IDE:

but it works at runtime, and compiles fine.

If we change it to api, then the IDE stops complaining:

I think it ought to be api. The good thing about keeping resources as a separate module (instead of consuming it into app is that a design_library module can depend on it too, without depending on app.

ataulm · 2019-08-11T23:18:40Z

(I merged this into my branch, if this is good, please merge commit, don't squash 😅 I really don't want to deal with merge conflicts/rebasing 😂 )

hitherejoe · 2019-08-12T18:46:08Z

Wow, thanks so much for putting this together @ricknout ! Looks awesome, we'll be glad to no longer see our plain UI when working on this 😆 Looks good from my end, so feel free to merge this @ataulm !

ataulm · 2019-08-12T20:45:11Z

🤔 I've only seen it on scrolling containers, I didn't think it'd work on the constraint layout.

…

On Mon, 12 Aug 2019, 21:20 Nick Rout, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In resources/src/main/res/layout/activity_theme_playground.xml <#27 (comment)>: > @@ -0,0 +1,243 @@ +<?xml version="1.0" encoding="utf-8"?> +<ScrollView + xmlns:android="http://schemas.android.com/apk/res/android" + xmlns:tools="http://schemas.android.com/tools" + xmlns:app="http://schemas.android.com/apk/res-auto" + android:layout_width="match_parent" + android:layout_height="match_parent" + tools:context=".ThemePlaygroundActivity"> + + <androidx.constraintlayout.widget.ConstraintLayout + android:layout_width="match_parent" + android:layout_height="wrap_content" + android:paddingBottom="16dp" + android:clipToPadding="false"> Oh lol, so the components on the playground screen can scroll past the padding without being clipped — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27?email_source=notifications&email_token=AAUN6G276HAYTAE4F72SDMLQEHAYLA5CNFSM4IK4KPY2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBJ42RQ#discussion_r313110331>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUN6GYNJE45ACSC3OYGH43QEHAYLANCNFSM4IK4KPYQ> .

ricknout · 2019-08-12T20:48:33Z

🤔 I've only seen it on scrolling containers, I didn't think it'd work on the constraint layout.
…
On Mon, 12 Aug 2019, 21:20 Nick Rout, @.> wrote: @.* commented on this pull request. ------------------------------ In resources/src/main/res/layout/activity_theme_playground.xml <#27 (comment)>: > @@ -0,0 +1,243 @@ + +<ScrollView + xmlns:android="http://schemas.android.com/apk/res/android" + xmlns:tools="http://schemas.android.com/tools" + xmlns:app="http://schemas.android.com/apk/res-auto" + android:layout_width="match_parent" + android:layout_height="match_parent" + tools:context=".ThemePlaygroundActivity"> + + <androidx.constraintlayout.widget.ConstraintLayout + android:layout_width="match_parent" + android:layout_height="wrap_content" + android:paddingBottom="16dp" + android:clipToPadding="false"> Oh lol, so the components on the playground screen can scroll past the padding without being clipped — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27?email_source=notifications&email_token=AAUN6G276HAYTAE4F72SDMLQEHAYLA5CNFSM4IK4KPY2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBJ42RQ#discussion_r313110331>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUN6GYNJE45ACSC3OYGH43QEHAYLANCNFSM4IK4KPYQ .

Oooh wait sorry, I think this was to prevent clipping of component elevation shadows

ataulm · 2019-08-12T20:50:06Z

Ah nice that sounds legit 👌👌👌

…

On Mon, 12 Aug 2019, 21:48 Nick Rout, ***@***.***> wrote: 🤔 I've only seen it on scrolling containers, I didn't think it'd work on the constraint layout. … <#m_-7178712554514617139_> On Mon, 12 Aug 2019, 21:20 Nick Rout, *@*.*> wrote: @.** commented on this pull request. ------------------------------ In resources/src/main/res/layout/activity_theme_playground.xml <#27 (comment) <#27 (comment)>>: > @@ -0,0 +1,243 @@ + +<ScrollView + xmlns:android=" http://schemas.android.com/apk/res/android" + xmlns:tools=" http://schemas.android.com/tools" + xmlns:app=" http://schemas.android.com/apk/res-auto" + android:layout_width="match_parent" + android:layout_height="match_parent" + tools:context=".ThemePlaygroundActivity"> + + <androidx.constraintlayout.widget.ConstraintLayout + android:layout_width="match_parent" + android:layout_height="wrap_content" + android:paddingBottom="16dp" + android:clipToPadding="false"> Oh lol, so the components on the playground screen can scroll past the padding without being clipped — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27 <#27>?email_source=notifications&email_token=AAUN6G276HAYTAE4F72SDMLQEHAYLA5CNFSM4IK4KPY2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBJ42RQ#discussion_r313110331>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUN6GYNJE45ACSC3OYGH43QEHAYLANCNFSM4IK4KPYQ . Oooh wait sorry, I think this was to prevent clipping of component elevation shadows — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#27?email_source=notifications&email_token=AAUN6G6FZKW7J5KMQ2GSP43QEHECDA5CNFSM4IK4KPY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4DZAUQ#issuecomment-520589394>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUN6G4EJFW2BRU6CVO4XU3QEHECDANCNFSM4IK4KPYQ> .

ricknout added 7 commits August 11, 2019 14:33

Add basic resources module with MDC-Android dependency

229fcf7

Add initial MuVi app themes to resources and use in app / film-detail

3a2bf9d

Add theme color attributes

adbbf6f

Add theme type attributes with downloadable font

e1a6d6d

Add theme shape attributes

81556d6

Add theme playground

59d8036

Make app themes inherit from DayNight for dark theme compatibility

f64a3bb

ricknout added the enhancement New feature or request label Aug 11, 2019

ricknout requested review from ataulm and hitherejoe August 11, 2019 17:45

ricknout self-assigned this Aug 11, 2019

ataulm approved these changes Aug 11, 2019

View reviewed changes

ricknout added 2 commits August 12, 2019 19:51

Make :resources and :app api dependency

0657d95

Revert extracted MDC dependency version

c017fec

ataulm merged commit 0fb6344 into master Aug 12, 2019

ataulm deleted the feature/themes-styles branch August 12, 2019 20:21


		import androidx.appcompat.app.AppCompatActivity

		class ThemePlaygroundActivity : AppCompatActivity(R.layout.activity_theme_playground)


		<application>

		<activity android:name=".ThemePlaygroundActivity" />

Conversation

ricknout commented Aug 11, 2019

Uh oh!

ataulm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ricknout Aug 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ataulm commented Aug 11, 2019

Uh oh!

ataulm commented Aug 11, 2019

Uh oh!

hitherejoe commented Aug 12, 2019

Uh oh!

ataulm commented Aug 12, 2019 via email

Uh oh!

ricknout commented Aug 12, 2019

Uh oh!

ataulm commented Aug 12, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ricknout Aug 12, 2019 •

edited

Loading