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

feat: convert iOS translations to Android format #585

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

boringcactus
Copy link
Member

Summary

Ticket: 🤖 | Determine localization approach

I couldn't find any obviously useful automated tools for converting between iOS Localizable.xcstrings and Android string resources, but it turns out it's fairly straightforward to staple something basic together myself.

Tradeoffs:

  • I'm using the iOS format as the source of truth here, because there's nothing in Android that can explicitly represent the "Needs Review" state that our workflow relies on.
  • Instead of generating Android IDs for new iOS strings eagerly, this will only copy translations for strings which are already defined on Android in English. This is both easier and more useful, since it lets us pick manually what ID makes the most sense.
  • The iOS string templating system and the Android string templating system aren't all that far apart, so it's not difficult to translate the easy cases over automatically.
    • I suspect but am not confident that multi-argument templates will also Just Work™.
  • There are several options for inline text formatting on Android, some of which might even be easy to handle here, but we are currently not doing any of them and doing string manipulation after the fact instead, so this needs to be a separate ticket.
  • Android plurals are a separate kind of resource entirely from Android strings; conveniently, we aren't using that subsystem at all yet, so I'm keeping things simple and ignoring that entirely for now.

Notes:

  • I've set things up so we can manually define localized strings separate from the automatically converted translations; if we define a key in the strings.xml it will be removed from strings_ios_converted.xml to avoid confusion.
  • I've flipped the switch to allow Android app-specific language preferences at the OS level, but if we want that functionality to be usable on Android 9-12 and not just 13+ we need our own UI for it, and I'm not going to preemptively do anything for that.
  • In the distant future we may benefit from linting on strings that are present in iOS but absent in Android, but right now we still have a ton of functionality that is simply not present in Android, so that would be incorrect to build now.
    • I've re-enabled the default lint for strings which have not been translated into all supported languages, though, which winds up being a lint on strings that are present in Android but absent in iOS.

Testing

Manually validated that the imported translations work for the languages I can check. (Neither my emulator nor my physical Android phone support Haitian Creole at all.)

@boringcactus boringcactus requested a review from a team as a code owner December 12, 2024 23:27
text = stringResource(R.string.approaching_abbr),
text = stringResource(R.string.minutes_abbr, 1),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we handle the Approaching case on iOS - there's no separate string for "1 min" - so it seems reasonable to do the same thing here.

Comment on lines -13 to +12
<string name="icon_description_external_link">External Link</string>
<string name="minutes_abbr">%1$s min</string>
<string name="icon_description_external_link" tools:ignore="MissingTranslation">External Link</string>
<string name="minutes_abbr" tools:ignore="MissingTranslation">%1$s min</string>
Copy link
Member Author

@boringcactus boringcactus Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no "External Link" on iOS (that icon has no VoiceOver text, and VoiceOver already reads them as "<link text>, button, link"), and the iOS English string is **%ld** min but as mentioned in the PR body inline formatting is going to take some work of its own.

Comment on lines -25 to +24
<string name="no_stops_nearby">Your current location is outside of our search area.</string>
<string name="no_stops_nearby_title">No nearby MBTA stops</string>
<string name="no_stops_nearby">You’re outside the MBTA service area.</string>
<string name="no_stops_nearby_title">No nearby stops</string>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text changed in iOS, so to get it to match in Android (and thereby not fail the build due to MissingTranslation) I've gone ahead and updated it here.

Copy link
Contributor

@JackVCurtis JackVCurtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is very straightforward. iOS as the source of truth makes sense for a bunch of reasons IMO. Same goes for inline formatting as a separate ticket. Really appreciate the thorough PR description!

Copy link
Collaborator

@KaylaBrady KaylaBrady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Love your approach of scoping down this ticket.

thought(non-blocking): I think it would be worth eagerly creating android strings with default ids - something like the english text stripped of punctuation & underscore separated.

That way as we build out the UI, it will be easy to autocomplete for the correct existing string resource . We could then update those IDs as they become used, but I also wonder if it would be worth just standardizing to using that format for simplicity. (Of course, if the string changes, then the ID changes, which would be inconvenient, but maybe not terribly).

@boringcactus
Copy link
Member Author

There are definitely some strings for which light post-processing of the English text would make a decent ID, but I don't think R.strings.in_1d_hr_2d_min or R.strings.mbta_go_is_in_the_early_stages_we_want_your_feedback_as_we_continue_making_improvements_and_adding_new_features or R.strings.made_with_by_the_t would be all that great, and I'd rather let people come up with good IDs as they add new functionality than have them put up with merely tolerable IDs generated in a one-off eager migration.

@boringcactus boringcactus merged commit 98c275c into main Dec 13, 2024
5 checks passed
@boringcactus boringcactus deleted the mth-android-localization branch December 13, 2024 17:09
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.

3 participants