search

MFlisar / GDPRDialog

GDPR fragment dialog implementation
Apache License 2.0
211 stars 53 forks source link

Use MaterialComponents theme #83

Closed yshrsmz closed 5 years ago

yshrsmz commented 5 years ago

close #81

I've switched all theme/style inside the library to MaterialComponents.

Here's the screenshot of the demo app. screenshot_1551524565

However, if AppTheme is extended from AppCompat theme, it looks like this

screenshot_1551524839

So how should we handle this?

My idea is below two:

MFlisar commented 5 years ago

I think following will work in all cases:

Top button like this

<com.google.android.material.button.MaterialButton
        android:id="@+id/btButton"
        style="@style/Widget.MaterialComponents.Button"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"/>

And bottom button like this

<com.google.android.material.button.MaterialButton
        android:id="@+id/btButton"
        style="@style/Widget.MaterialComponents.Button.OutlinedButton"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"/>

Using MaterialButton + explicit styles will work with the bridge and the normal material theme I think, you could try them, I will have time for it on the weekend otherwise. And forcing a material theme is fine, it's an android x library anyways

yshrsmz commented 5 years ago

Thanks for the feedback, I've updated PR with suggested changes.

Btw using MaterialButton explicitly with AppCompat theme will fail with following error, so we should make sure to include a migration note regarding the change.

android.view.InflateException: Binary XML file line #90: Binary XML file line #4: Error inflating class com.google.android.material.button.MaterialButton
    Caused by: android.view.InflateException: Binary XML file line #4: Error inflating class com.google.android.material.button.MaterialButton
    Caused by: java.lang.reflect.InvocationTargetException
        at java.lang.reflect.Constructor.newInstance0(Native Method)
        at java.lang.reflect.Constructor.newInstance(Constructor.java:343)
        at android.view.LayoutInflater.createView(LayoutInflater.java:647)
        at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:790)
        at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:730)
        at android.view.LayoutInflater.rInflate(LayoutInflater.java:863)
        at android.view.LayoutInflater.parseInclude(LayoutInflater.java:963)
        at android.view.LayoutInflater.rInflate(LayoutInflater.java:859)
        at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:824)
        at android.view.LayoutInflater.rInflate(LayoutInflater.java:866)
        at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:824)
        at android.view.LayoutInflater.rInflate(LayoutInflater.java:866)
        at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:824)
        at android.view.LayoutInflater.inflate(LayoutInflater.java:515)
        at android.view.LayoutInflater.inflate(LayoutInflater.java:423)
        at com.michaelflisar.gdprdialog.GDPRDialog.initView(GDPRDialog.java:146)
        at com.michaelflisar.gdprdialog.GDPRDialog.onCreateView(GDPRDialog.java:73)
        at androidx.fragment.app.Fragment.performCreateView(Fragment.java:2439)
        at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManager.java:1460)
        at androidx.fragment.app.FragmentManagerImpl.moveFragmentToExpectedState(FragmentManager.java:1784)
        at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManager.java:1852)
        at androidx.fragment.app.BackStackRecord.executeOps(BackStackRecord.java:802)
        at androidx.fragment.app.FragmentManagerImpl.executeOps(FragmentManager.java:2625)
        at androidx.fragment.app.FragmentManagerImpl.executeOpsTogether(FragmentManager.java:2411)
        at androidx.fragment.app.FragmentManagerImpl.removeRedundantOperationsAndExecute(FragmentManager.java:2366)
        at androidx.fragment.app.FragmentManagerImpl.execPendingActions(FragmentManager.java:2273)
        at androidx.fragment.app.FragmentManagerImpl.dispatchStateChange(FragmentManager.java:3273)
        at androidx.fragment.app.FragmentManagerImpl.dispatchActivityCreated(FragmentManager.java:3229)
        at androidx.fragment.app.FragmentController.dispatchActivityCreated(FragmentController.java:201)
        at androidx.fragment.app.FragmentActivity.onStart(FragmentActivity.java:620)
        at androidx.appcompat.app.AppCompatActivity.onStart(AppCompatActivity.java:178)
        at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1391)
        at android.app.Activity.performStart(Activity.java:7157)
        at android.app.ActivityThread.handleStartActivity(ActivityThread.java:2937)
        at android.app.servertransaction.TransactionExecutor.performLifecycleSequence(TransactionExecutor.java:180)
        at android.app.servertransaction.TransactionExecutor.cycleToPath(TransactionExecutor.java:165)
        at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:142)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:70)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1808)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
2019-03-07 14:12:12.819 6903-6903/com.michaelflisar.gdprdialog.demo E/AndroidRuntime: Caused by: java.lang.IllegalArgumentException: This component requires that you specify a valid TextAppearance attribute. Update your app theme to inherit from Theme.MaterialComponents (or a descendant).
        at com.google.android.material.internal.ThemeEnforcement.checkTextAppearance(ThemeEnforcement.java:170)
        at com.google.android.material.internal.ThemeEnforcement.obtainStyledAttributes(ThemeEnforcement.java:75)
        at com.google.android.material.button.MaterialButton.<init>(MaterialButton.java:140)
        at com.google.android.material.button.MaterialButton.<init>(MaterialButton.java:133)
            ... 45 more
yshrsmz commented 5 years ago

Hi, do you plan to merge my PR? Feel free to close this if you have another plan to go.

MFlisar commented 5 years ago

If we force to use a material theme, the buttons can stay buttons, can't they? Using an app compat theme was anyway a mistake, with android x you should use the material theme anyways. And for people you still use app compat there even exist a material bridge theme for an easier switch

yshrsmz commented 5 years ago

Yes, if we force a material theme, everything is fine.