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

PlayerControlView incorrectly mirrors next/prev and ffwd/rewind icons in RTL #227

Closed
1 task
maknon opened this issue Dec 17, 2022 · 15 comments
Closed
1 task
Assignees
Labels

Comments

@maknon
Copy link

maknon commented Dec 17, 2022

Media3 Version

1.0.0-beta03

Devices that reproduce the issue

Samsung S22+ Android 13

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

My App is RTL and no control playback is shown (snapshot below) when layout_height=wrap_content

<androidx.media3.ui.PlayerControlView
        android:id="@+id/playerView"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:layout_gravity="bottom" />

if I increase the height manually it will render normally

another issue is the next back icones are reversed

Expected result

icons should be reversed and all playback controls should be shown

Actual result

as attached

Media

2

1

Bug Report

  • You will email the zip file produced by adb bugreport to [email protected] after filing this issue.
@icbaker
Copy link
Collaborator

icbaker commented Dec 19, 2022

if I increase the height manually it will render normally

Based on the XML snippet it looks like you might be using PlayerControlView without PlayerView? I'm afraid this is not supported (partly because, as you've seen, the controls expect to the the full height of the viewport).


another issue is the next back icones are reversed

I agree the icons being reversed in RTL is a bug - I will keep this open to track resolving that.

Reference for time-related playback icons not being mirrored in RTL: https://m2.material.io/design/usability/bidirectionality.html#mirroring-elements

@icbaker icbaker changed the title PlayerControlView rendering issues in RTL PlayerControlView incorrectly mirrors next/prev and ffwd/rewind icons in RTL Dec 19, 2022
@maknon
Copy link
Author

maknon commented Dec 19, 2022

Based on the XML snippet it looks like you might be using PlayerControlView without PlayerView? I'm afraid this is not supported (partly because, as you've seen, the controls expect to the the full height of the viewport).

thank you, indeed im using it without PlayerView. but im not able to achieve the required behaviour here. I just need PlayerControlView (audio only) and I need it to show/hide at the bottom of the activity.
Replacing PlayerControlView with PlayerView seems to fill the whole screen and is not overlaying with the below activity like PlayerControlView. Using surface_type="none" does not improve the case.

Is there any issue continuing with PlayerControlView without PlayerView. It seems it works fine for now. Layout might not be optimum using a fixed hight but at least works

@icbaker
Copy link
Collaborator

icbaker commented Dec 19, 2022

Is there any issue continuing with PlayerControlView without PlayerView. It seems it works fine for now. Layout might not be optimum using a fixed hight but at least works

You're welcome to use it like this, but by "not supported" I mean that we won't be able to help with issues you encounter due to this choice.

@icbaker
Copy link
Collaborator

icbaker commented Dec 19, 2022

I'm not able to reproduce your RTL problem in the demo app. When I change the language to an RTL one I see the controls look the same as when I view it in english.

Please can you see if you're able to reproduce it there? If not then I suspect it's another side-effect of using PlayerControlView outside of PlayerView.

@maknon
Copy link
Author

maknon commented Dec 19, 2022

i just tried it with PlayerView instead of PlayerControlView. it seems it is a RTL issue
1

i will try to check the demo app

@maknon
Copy link
Author

maknon commented Dec 20, 2022

it seems the same case with demo app. you just need to enable RTL and can see it in the design view

image

@icbaker
Copy link
Collaborator

icbaker commented Dec 20, 2022

What happens if you actually run the demo app on a real device in an RTL language? That's what I tried (Pixel 5 running Android 13) and didn't observe a problem. I tried the following ways to introduce an RTL layout:

  • 'force RTL' in developer settings
  • select the 'fake' BiDi language in system-wide language settings
  • select arabic in system-wide language settings

All of these affected other UI elements like back arrows as expected - but none had the effect on the playback controls shown in your screenshots.

@maknon
Copy link
Author

maknon commented Dec 20, 2022

Please find the output on real device:
Screenshot_20221220_143848_ExoPlayer

im using this to activate the RTL:
on every activity:

@Override
  protected void attachBaseContext(Context base)
  {
    super.attachBaseContext(ContextWrapper.wrap(base, new Locale("ar")));
  }
public class ContextWrapper extends android.content.ContextWrapper
{
    public ContextWrapper(final Context base)
    {
        super(base);
    }

    public static ContextWrapper wrap(Context context, final Locale newLocale)
    {
        final Resources res = context.getResources();
        final Configuration configuration = res.getConfiguration();
        final Configuration configuration2 = new Configuration(configuration);

        if (Build.VERSION.SDK_INT >= 24)
        {
            final LocaleList localeList = new LocaleList(newLocale);
            LocaleList.setDefault(localeList);
            configuration2.setLocales(localeList);
        }
        else
        {
            Locale.setDefault(newLocale);
            configuration2.locale = newLocale;
        }

        return new ContextWrapper(context.createConfigurationContext(configuration2));
    }
}

@icbaker
Copy link
Collaborator

icbaker commented Dec 20, 2022

What do you observe if you change the system-wide language using the settings UI rather than overriding it in code?

@maknon
Copy link
Author

maknon commented Dec 20, 2022

here it is
Screenshot_٢٠٢٢١٢٢٠_١٤٤٧١٠_ExoPlayer

you can see the top status bar is inverted because the it is system wide language (ar). i have removed that code.

now i remember samsung android is doing a lot of things related to RTL language to make most of the app works out of the box. i noticed this behavior when testing on Huawei old phone at one point of time. not sure if this is what you are facing here.

@icbaker
Copy link
Collaborator

icbaker commented Dec 20, 2022

Do you have other devices you could try on? Or on an emulator in Android Studio? I'd like to understand if the difference between what we're seeing is due to the specific Samsung device you're using.

@maknon
Copy link
Author

maknon commented Dec 21, 2022

Huawei android 9
Screenshot_20221221_081105_com google android exoplayer2 demo

@icbaker
Copy link
Collaborator

icbaker commented Dec 21, 2022

I'm afraid without being able to reproduce the problem it's hard to investigate further. Can you try with the demo app in the emulator? That might be as similar as we can get to each other. I used a Pixel 5 API 33 emulator with the demo app built from 1.0.0-beta03 (no code changes) and I enabled 'force RTL' in developer options. I see this:

Screenshot 2022-12-21 at 16 01 01

@maknon
Copy link
Author

maknon commented Dec 21, 2022

ok i just recognised that i added additional RTL support in the manifest

android:supportsRtl="true"

yes without this line RTL is not activated fully and you will not see this issue. please try it

@icbaker
Copy link
Collaborator

icbaker commented Dec 21, 2022

Thanks - I can now reproduce the problem.

marcbaechinger pushed a commit to google/ExoPlayer that referenced this issue Jan 4, 2023
Issue: androidx/media#227

#minor-release

PiperOrigin-RevId: 497159283
marcbaechinger pushed a commit to google/ExoPlayer that referenced this issue Jan 4, 2023
We might as well keep this enabled by default, rather than having to
manually toggle it on to investigate RTL issues like Issue: androidx/media#227.

PiperOrigin-RevId: 497159744
marcbaechinger pushed a commit that referenced this issue Jan 5, 2023
Issue: #227

#minor-release

PiperOrigin-RevId: 497159283
marcbaechinger pushed a commit that referenced this issue Jan 5, 2023
We might as well keep this enabled by default, rather than having to
manually toggle it on to investigate RTL issues like Issue: #227.

PiperOrigin-RevId: 497159744
@icbaker icbaker closed this as completed Jan 5, 2023
christosts pushed a commit that referenced this issue Jan 31, 2023
Issue: #227

#minor-release

PiperOrigin-RevId: 497159283
(cherry picked from commit 8060342)
christosts pushed a commit that referenced this issue Jan 31, 2023
We might as well keep this enabled by default, rather than having to
manually toggle it on to investigate RTL issues like Issue: #227.

PiperOrigin-RevId: 497159744
(cherry picked from commit 69583d0)
christosts pushed a commit to google/ExoPlayer that referenced this issue Feb 16, 2023
Issue: androidx/media#227

#minor-release

PiperOrigin-RevId: 497159283
(cherry picked from commit 8313af1)
christosts pushed a commit to google/ExoPlayer that referenced this issue Feb 16, 2023
We might as well keep this enabled by default, rather than having to
manually toggle it on to investigate RTL issues like Issue: androidx/media#227.

PiperOrigin-RevId: 497159744
(cherry picked from commit 010c6b9)
@androidx androidx locked and limited conversation to collaborators Mar 7, 2023
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

2 participants