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

Visual bug with UI with StyledPlayerView #7898

Closed
uberchilly opened this issue Sep 13, 2020 · 8 comments
Closed

Visual bug with UI with StyledPlayerView #7898

uberchilly opened this issue Sep 13, 2020 · 8 comments
Assignees
Labels

Comments

@uberchilly
Copy link

[REQUIRED] Issue description

Visual bug with rewind and fast forward buttons, description text not in the center, and button icons not white.

[REQUIRED] Reproduction steps

Added StyledPlayerView to the layout like this
image

[REQUIRED] Link to test content

Not applicable

[REQUIRED] A full bug report captured from the device

image

[REQUIRED] Version of ExoPlayer being used

2.12.0

[REQUIRED] Device(s) and version(s) of Android being used

Oneplus 6t, Android 10

@1nsun
Copy link
Contributor

1nsun commented Sep 16, 2020

Do you have theme for TextViews in your app? If so, can you share the theme definition? Thanks!

@uberchilly
Copy link
Author

uberchilly commented Sep 16, 2020

The only style that has this green color (which is called nature_interaction in the codebase)
is here

	<style name="MainButtonColoredStyle" parent="@style/Widget.AppCompat.Button.Colored">
		<item name="android:fontFamily">@font/calibri_bold</item>
		<item name="android:textColor">@color/white</item>
		<item name="android:textSize">@dimen/standard_text_size</item>
	</style>

	<style name="ButtonNature" parent="@style/MainButtonColoredStyle">
		<item name="backgroundTint">@color/nature_interactions</item>
	</style>

But these styles are not configured into the global theme they need to be set manually in XML like this

<Button
        style="@style/ButtonNature"

Global theme

	<style name="AppTheme" parent="Theme.MaterialComponents.Light.NoActionBar">
		<!-- Customize your theme here. -->
		<item name="colorPrimary">@color/nature_interactions</item>
		<item name="colorPrimaryDark">@color/nature_interactions_dark</item>
		<item name="colorAccent">@color/warm_highlights</item>

		<item name="navigationDrawerLogo">@drawable/aliunid_new_logo_png</item>
		<item name="windowActionModeOverlay">true</item>

		<item name="actionModeStyle">@style/ActionModeStyle</item>
		<item name="actionModeCloseDrawable">@drawable/ic_close_24dp</item>
	</style>

That green color is also the color primary for the global theme.
If that theme is picked from there, what about the centering issue?

@Seil0
Copy link

Seil0 commented Oct 30, 2020

I have the same problem. I can override the primary color in the player theme which fixes the wrong color issue, but i can't work around the not centered text.

Version of ExoPlayer being used: 2.12.1
Device(s) and version(s) of Android being used: Pixel 2, Android 11

@ojw28
Copy link
Contributor

ojw28 commented Oct 30, 2020

@1nsun - I think we need to make sure that the icons are not picking up the primary color. Is it possible they're also picking up some default padding or something, which is affecting the text positioning?

@petitJAM
Copy link

petitJAM commented Nov 5, 2020

I believe that this issue is happening because of the Material Design Components library. I am seeing the same issue with the numbers being up higher than they should on the app I'm currently working on that uses MDC. I just tried making a small test app without MDC and I do not see the issue with misaligned text.

MDC does something where it intercepts XML inflation. It takes <Button /> tags and actually inflates them as <com.google.android.material.button.MaterialButton />s instead. In a Theme.MaterialComponents.* theme, there's a viewInflaterClass attribute set to com.google.android.material.theme.MaterialComponentsViewInflater, this is what does the inflation. Looking at this classes source code though, it doesn't actually do much, so I think the problem is somewhere in MaterialButton then. It might have some default paddings.

@fuadreza
Copy link

fuadreza commented Nov 18, 2020

I have the same problem, just like @petitJAM mentioned it because we use material theme that causing style on the button change.

Workaround for me is that I create a custom theme with Theme.AppCompat.Light.NoActionBar as a parent. and then apply that custom theme to the activity in AndroidManifest.xml. Here what it look like:
In style.xml :

<style name="CustomTheme" parent="Theme.AppCompat.Light.NoActionBar">
        <item name="android:textViewStyle">@style/NunitoTextStyle</item>
</style>

OR

As the button use the MaterialButton theme, create a custom theme that inherit from Widget.MaterialComponents.Button to override the android:insetBottom. After that create another style and use materialButtonStyle in the item and reference that to custom button theme as we created before. It will look like this :

    <style name="ExoCustomTheme">
        <item name="materialButtonStyle">@style/ExoButtonStyle</item>
    </style>

    <style name="ExoButtonStyle" parent="Widget.MaterialComponents.Button">
        <item name="android:insetBottom">0dp</item>
        <item name="backgroundTint">@color/colorWhite</item>
    </style>

After that apply that theme to your ExoPlayerView theme,

<com.google.android.exoplayer2.ui.StyledPlayerView
                            android:id="@+id/video_view"
                            android:layout_width="match_parent"
                            android:theme="@style/ExoCustomTheme"
                            android:layout_height="wrap_content"/>

Using the second option, you don't need to change the theme of the Activity in AndroidManifest.xml.
I suggest you use the second option as if there are many thing that possibly you have to set if you create a parent theme for the activity.

@ojw28
Copy link
Contributor

ojw28 commented Nov 19, 2020

Thanks @uberchilly @petitJAM @fuadreza for your help diagnosing this. I think we can work around this in the ExoPlayer library by:

  1. Setting android:insetBottom to 0 on our style for these two buttons.
  2. Defining our own backgroundTint attr, setting it to white in our style for these two buttons, and relying on the fact there will be a namespace collision with the Material library's backgroundTint attr for our defined value to be picked up.

The second one is awkward because the ExoPlayer UI module doesn't have a dependency on the Material library, which means we can't just use Material's backgroundTint attr directly. Using android:backgroundTint doesn't work.

@ojw28 ojw28 assigned ojw28 and unassigned 1nsun Nov 19, 2020
ojw28 added a commit that referenced this issue Nov 19, 2020
#minor-release
Issue: #7898
PiperOrigin-RevId: 343251455
@ojw28
Copy link
Contributor

ojw28 commented Nov 19, 2020

This is hopefully fixed by the change ref'd above. We'll include this in 2.12.2, which is due in the next couple of weeks.

@ojw28 ojw28 closed this as completed Nov 19, 2020
icbaker pushed a commit that referenced this issue Nov 30, 2020
@google google locked and limited conversation to collaborators Jan 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants