search

ataulm / muvi

Apache License 2.0
39 stars 3 forks source link

Add resources module with themes/styles #27

Closed ricknout closed 5 years ago

ricknout commented 5 years ago

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:

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.

muvi

resolves #26

ataulm commented 5 years ago

: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:

image

but it works at runtime, and compiles fine.

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

image

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 commented 5 years ago

(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 commented 5 years ago

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 commented 5 years ago

🤔 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, notifications@github.com wrote:

@ricknout commented on this pull request.

In resources/src/main/res/layout/activity_theme_playground.xml https://github.com/ataulm/muvi/pull/27#discussion_r313110331:

@@ -0,0 +1,243 @@ +<?xml version="1.0" encoding="utf-8"?> +<ScrollView

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 https://github.com/ataulm/muvi/pull/27?email_source=notifications&email_token=AAUN6G276HAYTAE4F72SDMLQEHAYLA5CNFSM4IK4KPY2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBJ42RQ#discussion_r313110331, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUN6GYNJE45ACSC3OYGH43QEHAYLANCNFSM4IK4KPYQ .

ricknout commented 5 years ago

🤔 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 .

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

ataulm commented 5 years ago

Ah nice that sounds legit 👌👌👌

On Mon, 12 Aug 2019, 21:48 Nick Rout, notifications@github.com 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) https://github.com/ataulm/muvi/pull/27#discussion_r313110331>: > @@ -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 https://github.com/ataulm/muvi/pull/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 https://github.com/ataulm/muvi/pull/27?email_source=notifications&email_token=AAUN6G6FZKW7J5KMQ2GSP43QEHECDA5CNFSM4IK4KPY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4DZAUQ#issuecomment-520589394, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUN6G4EJFW2BRU6CVO4XU3QEHECDANCNFSM4IK4KPYQ .