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

Incorrect directionality in RTL OSD #4884

Open
ShlomoCode opened this issue Apr 19, 2024 · 10 comments · May be fixed by #4886
Open

Incorrect directionality in RTL OSD #4884

ShlomoCode opened this issue Apr 19, 2024 · 10 comments · May be fixed by #4886

Comments

@ShlomoCode
Copy link

ShlomoCode commented Apr 19, 2024

System and IINA version:

macOS 14.4.1
IINA 1.3.4 + PR #4922

Expected behavior:
The audio progress indicator in the OSD should always be from left to right (see #4878 (comment)).
The OSD itself should be on the left side of the screen in RTL mode.

Actual behavior:

  • The OSD (on screen display) has a playback progress slider while changing the position, and it should be like the slider in the OSC (from left to right, like a tape).
  • The numbers in the OSD are also not in the right direction, meaning it should be
    {current_time} / {remaining_time} (as in the OSC slider)
    and not {remaining_time} / {current_time}
  • In addition, the OSD should be on the left side of the window and not on the right side

Screenshort:

Video
CleanShot.2024-04-19.at.01.46.36.mp4

Steps to reproduce:

  • Open the IINA project in Xcode
  • Under the Product menu open the Scheme menu and select Edit Scheme…
  • Click on the Options tab
  • Scroll down to the App Language setting
  • In the pull down select Right-to-Left Pseudolanguage near the end of the list
  • Start IINA running under Xcode
  • Start playing a video
  • Change playback position (for example by tapping on a position in the progress slider)

Or you can build with PR #4922 and set your language to Hebrew.

[x] MPV does not have this problem.

mpv does not support localization.

Originally posted by @ShlomoCode in #4878 (comment)

@low-batt
Copy link
Contributor

This is the part I don't understand:

In addition, the OSD should be on the left side of the window and not on the right side

Why would the OSD not flip location like other components?

The OSD is "interesting" since it needs to not flip when showing playback progress, but needs to flip when showing other information such as volume, yes?

@ShlomoCode
Copy link
Author

ShlomoCode commented Apr 20, 2024

Why would the OSD not flip location like other components?

Mmm... I forgot that in the LTR version it's already on the left side of the screen, I'm used to notifications (toasts) appearing at the "end" of the screen, that is, in English it's usually on the right side. It's like that in macos notifications, in English it's on the right and in Hebrew it's on the left.
But iina is the opposite of normal in English... you know why?

The OSD is "interesting" since it needs to not flip when showing playback progress, but needs to flip when showing other information such as volume, yes?

Yes

@low-batt
Copy link
Contributor

Caught me working on this. There are a number of OSD messages that need to be corrected. Juggling multiple tasks. I may not finish this one today.

On this:

But iina is the opposite of normal in English... you know why?

I'm a newbie (relative to others) junior IINA developer. I wasn't around when IINA was initially developed and still don't have any knowledge about such design decisions. My guess is that IINA matched mpv whose OSD is also on the left side. As you can disable IINA's OSD and switch to the mpv OSD it makes sense that they are both in the same part of the window. I believe the attraction of the mpv OSD is that it can be customized.

low-batt added a commit that referenced this issue Apr 20, 2024
This commit will:
- Add withPosition and withLeftToRightText message types to OSDMessage
- Change the seek message type in OSDMessage to use withPosition instead
  of withProgress
- Change the abLoop, abLoopUpdate, chapter, pause and  resume message
  types in OSDMessage to use withLeftToRightText instead of withText
- Add support for withPosition to MainWindowController.displayOSD that
  uses a left to right layout for the text and slider
- Add support for withLeftToRightText to MainWindowController.displayOSD
  that uses a left to right layout for the text

This adjusts IINA's behavior to match Apple's guidance that timeline
indicators should not flip in a right-to-left language. The updated OSD
messages all reference positions within the video.
@low-batt low-batt linked a pull request Apr 20, 2024 that will close this issue
2 tasks
@low-batt low-batt linked a pull request Apr 20, 2024 that will close this issue
2 tasks
@low-batt
Copy link
Contributor

Proposed fix posted in PR #4886.

@ShlomoCode
Copy link
Author

ShlomoCode commented Apr 20, 2024

i pulled the pr, build and checked the abLoop, pause, resume, volume and progress messages
Looks good!

@low-batt
Copy link
Contributor

Great! Next up is the remaining problems in issue #4776.

In IINA's instance of Crowdin Hebrew is 99% translated. There are still a few missing translations. Are you able to bring it to 100%?

@ShlomoCode
Copy link
Author

@low-batt There were 5 strings left that I couldn't understand the context/meaning of. If you comment on them it will help!

p.s. still the location of the OSD toast on the right side of the screen seems illogical to me

@low-batt
Copy link
Contributor

As I don't normally work on localization I don't have a Crowdin account yet. I'm trying to create one now. Having some trouble. I will help with the translations once I gain access.

For something like the OSD location it is best to comment on the PR so when I get a senior developer to review the proposed changes they may be able to comment on why things are the way they are, or suggest a change.

@low-batt
Copy link
Contributor

I am now on Crowdin. I answered the questions I found. Crowdin wouldn't let me use my normal email, so I will be very slow to notice messages posted there.

@ShlomoCode
Copy link
Author

Thank you, now Hebrew is 100% complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants