beeware / toga

A Python native, OS native GUI toolkit.
https://toga.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
4.22k stars 655 forks source link

The `ROW` direction in RTL Android Phones is a mess! #2692

Open vzool opened 2 weeks ago

vzool commented 2 weeks ago

Describe the bug

Toga.Box() on the ROW direction in RTL Android Phones is a mess, while it is working just fine in LTR.

Steps to reproduce

  1. Clone the repo git@github.com:vzool/zakat-tracker.git
  2. Run on Android Phone with RTL locale like (Arabic or Hebrew)
  3. Browse the UI
  4. See the error

Expected behavior

The UI should behave the same on all RTL or LTR locale phones.

Screenshots

On the left is the issue on RTL Android phone languages like Arabic, on the right is the same UI on LTR Android phone languages like English:

‏لقطة الشاشة 1446-01-01 في 6 02 07 ص ‏لقطة الشاشة 1446-01-01 في 6 02 55 ص

is

Environment

Logs

Additional context

No response

freakboy3742 commented 2 weeks ago

Thanks for the report. I don't doubt there are issues with RTL handling; sadly, none of the currently active core team members are fluent in an RTL language.

It would be especially helpful if you're able to provide examples of layouts that you know to be problematic, with code samples. There are parts of RTL handling that are implemented as cross-platform layout code, and some that leverage platform RTL handling. Having specific examples of problematic layouts provides specific issues that can be resolved, rather than a nebulous "it's a mess" descriptor which, while it may be true, doesn't provide a very good mechanism for us to determine when enough fixes have been applied that it is no longer "a mess".

vzool commented 2 weeks ago

Great, here is a snippet code illustrating the issue in a single file app.py:

"""
My first application
"""

import toga
from toga.style import Pack
from toga.style.pack import COLUMN, ROW
from datetime import datetime

class HelloWorld(toga.App):
    def startup(self):
        """Construct and show the Toga application.

        Usually, you would add your application to a main content box.
        We then create a main window (with a name matching the app), and
        show the main window.
        """
        main_box = toga.Box()

        main_box.add(self.datetime_widget())

        self.main_window = toga.MainWindow(title=self.formal_name)
        self.main_window.content = main_box
        self.main_window.show()

    def datetime_widget(self):
        now = datetime.now()
        datetime_box = toga.Box(style=Pack(direction=COLUMN, padding=5))
        datetime_label = toga.Label('DateTime', style=Pack(padding=(0, 5), text_align="center", font_weight='bold'))

        year_label = toga.Label('Year', style=Pack(padding=(0, 5)))
        self.year_input = toga.NumberInput(min=1000, value=now.year, style=Pack(flex=1, width=66))
        month_label = toga.Label('Month', style=Pack(padding=(0, 5)))
        self.month_input = toga.NumberInput(min=1, max=12, value=now.month, style=Pack(flex=1, width=45))
        day_label = toga.Label('Day', style=Pack(padding=(0, 5)))
        self.day_input = toga.NumberInput(min=1, max=31, value=now.day, style=Pack(flex=1, width=45))

        date_box = toga.Box(style=Pack(direction=ROW, padding=5, flex=1))
        date_box.add(year_label)
        date_box.add(self.year_input)
        date_box.add(month_label)
        date_box.add(self.month_input)
        date_box.add(day_label)
        date_box.add(self.day_input)

        hour_label = toga.Label('Hour', style=Pack(padding=(0, 5)))
        self.hour_input = toga.NumberInput(min=0, max=23, value=now.hour, style=Pack(flex=1, width=45))
        minute_label = toga.Label('Minute', style=Pack(padding=(0, 5)))
        self.minute_input = toga.NumberInput(min=0, max=59, value=now.minute, style=Pack(flex=1, width=45))
        second_label = toga.Label('Second', style=Pack(padding=(0, 5)))
        self.second_input = toga.NumberInput(min=0, max=59, value=now.second, style=Pack(flex=1, width=45))

        time_box = toga.Box(style=Pack(direction=ROW, padding=5, flex=1))
        time_box.add(hour_label)
        time_box.add(self.hour_input)
        time_box.add(minute_label)
        time_box.add(self.minute_input)
        time_box.add(second_label)
        time_box.add(self.second_input)

        datetime_box.add(datetime_label)
        datetime_box.add(toga.Divider())
        datetime_box.add(date_box)
        datetime_box.add(time_box)
        return datetime_box

def main():
    return HelloWorld()

Normal behavior on Android LTR locale device:

Issue caught again on Android RTL locale device:

Thanks

mhsmith commented 2 weeks ago

I see the same on API level 34. It can be worked around in your app by editing build/.../android/gradle/app/src/main/AndroidManifest.xml to change android:supportsRtl to "true" "false".

We probably shouldn't make this change in the Briefcase template, because it would be difficult to remove in the future given that there's not a direct relationship between Toga and Briefcase versions. Instead, we should try and fix Toga so it's at least usable in RTL locales until we support them fully.

vzool commented 2 weeks ago

Thanks for the suggestion. But, I searched the BeeWare repositories and I saw that android:supportsRtl="true" at AndroidManifest.xml is already set by the briefcase-android-gradle-template on this line, as I understand the UI is RTL but Toga content can't adapt. I think Android RTL/LTR locales will force its direction, no matter what was set for android:supportsRtl="true". Android: If the user wants RTL/LTR, your wish is my command. ^_^

mhsmith commented 2 weeks ago

I think Android RTL/LTR locales will force its direction, no matter what was set for android:supportsRtl="true"

Sorry, I got that the wrong way round. Try setting it to "false".

That fixed your example code for me, on a device set to Arabic. But expect it will still have some issues in the presence of actual Arabic text, so please try it and let us know what you think.

vzool commented 1 week ago

Thank you very much, it is way better now. BTW, will you accept PR if I make this change on briefcase-android-gradle-template?

mhsmith commented 1 week ago

No, I changed my mind about that – see this comment.