Skip to content

Arabic localization enhancement#65

Open
elkhoudiry wants to merge 6 commits into
darkokoa:mainfrom
elkhoudiry:update/ar-settings
Open

Arabic localization enhancement#65
elkhoudiry wants to merge 6 commits into
darkokoa:mainfrom
elkhoudiry:update/ar-settings

Conversation

@elkhoudiry

Copy link
Copy Markdown

First, thanks so much for the effort the library is great.

Second there is a visual bug when the locale is arabic but i suspect the same issue will exist for some other locales as well, the problem is:

  • Currently the WheelDateTimePicker view support month names to be in short format as the month cell width will be hardcoded to a small value.
  • This works well in english and other locales that has shorter form of the month name.
  • Unfortunately this is not the case in arabic as actually there is no short form for month names ( at least common one ) so the view crops the month name as it shown in the image below.
  • if the WheelDatePicker is used it shows the month name correctly "أغسطس" but in the WheelDateTimePicker it gets cropped to "أغسطـ"
    image

Also timeFormatter() function is hardcoding the time format to HOUR_24 in languages other than english but in arabic the AM_PM format works well and is desirable so modified the function parameters to take the time format from the caller and defaulting to null to keep compatibility with the current API.

The result:
ar_enhancement

@darkokoa

Copy link
Copy Markdown
Owner

Hey @elkhoudiry , thanks for your PR!

Really appreciate you taking the time to improve localization

I'll check this out soon and get back to you

@darkokoa

Copy link
Copy Markdown
Owner

Hi @elkhoudiry ,

I've carefully considered your proposal. While it does address some issues, I have a couple of different ideas regarding these two main changes that I'd like to discuss with you:

1. Regarding the month name truncation issue:

I noticed this primarily affects the WheelDateTimePicker. My thinking is that we could adjust the width allocation for the day, month, and year pickers within the WheelDateTimePicker component itself. Specifically, we could consider allocating relatively wider space for the year and month pickers, and narrower space for the day picker. This would help avoid truncation, rather than uniformly using Modifier.widthIn(). This approach would allow for more flexible control over the proportions of each part and enable optimization based on the actual content.

2. Regarding the new timeFormat: TimeFormat? = null parameter in timeFormatter():

I'm inclined to avoid using a nullable timeFormat, as it might confuse users (e.g., when should they pass null?). I suggest we add a new overloaded function that explicitly accepts a non-nullable TimeFormat, like this:

@Composable
fun timeFormatter(
  locale: Locale,
  timeFormat: TimeFormat
): TimeFormatter {
  val lyricist = rememberStrings(currentLanguageTag = locale.language)

  return remember(lyricist.strings, locale, timeFormat) {
    timeFormatter(
      strings = lyricist.strings,
      timeFormat = timeFormat
    )
  }
}

This would maintain function clarity. If users need to choose different TimeFormats based on certain conditions, or not format the time at all (although the purpose of timeFormatter here is indeed formatting), they can implement the logic at the call site and then pass a specific TimeFormat or call another timeFormatter overload that doesn't take a TimeFormat parameter (if such an overload exists and makes sense).


I'd appreciate any input you have on these ideas.

@elkhoudiry

Copy link
Copy Markdown
Author

Hello @darkokoa

Regarding the points you presented:

1- Ok this solution would work temporarily, i will move forward with it for now, but i have the concern of it may not be enough as we can't control the length of month names in all localizations, and the settings of the typography settings of different applications, and the available screen width where the component will be displayed.

2- Yes i agree, I myself also thought it would be a better approach after i open this PR, so i will do it and get back to you

@darkokoa

Copy link
Copy Markdown
Owner

I understand your concern, and honestly I don't have a perfect solution either.

I've just been trying to figure out how to make the text overflow on both sides within the WheelPicker - what do you think about this approach? If we can't find a particularly good solution in the short term, I think using widthIn(min=size.width) would be acceptable as well.

@elkhoudiry

Copy link
Copy Markdown
Author

I pushed the time format override.

About the month name situation, Well the only thing that may have a problem is month names as they are unexpected, a widthIn solution will work.
or give the month cell a weight(1f) and leave the rest of the fields without a weight which will make rest of the cells take their required width and then the month cell will fill the rest of the width.
but this requires more setup, so a widthIn will be sufficient for now

@darkokoa

darkokoa commented Jun 2, 2025

Copy link
Copy Markdown
Owner

Long story short, using widthIn() is still not ideal - it causes other items to be clipped or makes the scrolling experience feel choppy. I'd like to fix it too, but I'm quite torn on the approach.

My current thinking: keep the status quo. When this situation occurs, users should either increase the Picker's width or reduce the text size.
Screenshot_20250602-162349

@elkhoudiry

Copy link
Copy Markdown
Author

Ok restored the widthIn changes for now and using this work around since the problem is in the DateTimePicker and the regular DatePicker is fine

AppRow(
        modifier,
        arrangement = Arrangement.spacedBy(Dimensions.Paddings.small.dp, Alignment.CenterHorizontally),
        alignment = Alignment.CenterVertically
    ) {
        WheelDatePicker(
            modifier = Modifier.weight(1f),
            dateFormatter = dateFormatter(locale = Locale("ar"), monthDisplayStyle = MonthDisplayStyle.FULL),
            startDate = currentDate,
            minDate = remember { LocalDate(nowDate.date.year - 1, 1, 1) },
            maxDate = remember { LocalDate(nowDate.date.year, 12, 31) },
            onSnappedDate = { currentDate = it }
        )

        WheelTimePicker(
            startTime = currentTime,
            timeFormatter = timeFormatter(locale = Locale("ar"), TimeFormat.AM_PM),
            onSnappedTime = { currentTime = it }
        )
    }

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants