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: Add a possibility to set app locale #580

Merged
merged 5 commits into from May 24, 2020

Conversation

mykola-mokhnach
Copy link
Contributor

Currently it is only possible to set locale via Appium Settings helper, which does it in a really hacky manner. Also there is no guarantee this hack going to work in newer Android versions.

Although, Espresso anyway uses the target context concept, which we could change locale for without private method calls. I've added the locale capability that allows to set locale for an initial app startup as well as mobile: setLocale one, which does it dynamically.

I would really appreciate it if you could help me with testing on newer Android versions @KazuCocoa

@mykola-mokhnach
Copy link
Contributor Author

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

Let me check RTL, too. Will update our demo app for it to check.

lib/desired-caps.js Outdated Show resolved Hide resolved
}
val res = context.resources
val config = res.configuration
config.locales = LocaleList(locale)
Copy link
Member

Choose a reason for hiding this comment

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

config.setLayoutDirection(locale) is necessary to apply RTL style properly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docstring on setLocales method:

    /**
     * Set the locale list. This is the preferred way for setting up the locales (instead of using
     * the direct accessor or {@link #setLocale(Locale)}). This will also set the layout direction
     * according to the first locale in the list.
     *
     * Note that the layout direction will always come from the first locale in the locale list,
     * even if the locale is not supported by the resources (the resources may only support
     * another locale further down the list which has a different direction).
     *
     * @param locales The locale list. If null, an empty LocaleList will be assigned.
     */

Copy link
Member

Choose a reason for hiding this comment

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

ah, okay. I see.
I also understand this is application-wide change (via the target context), so other areas such as status bar and other applications do not change.
Will check with an app which has multiple locale/layout later

Copy link
Member

@KazuCocoa KazuCocoa May 23, 2020

Choose a reason for hiding this comment

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

LocaleList is only for API level 24. (below is for lower versions)

config.locale = locale
config.setLayoutDirection(locale)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, will change

@@ -41,7 +41,7 @@ let espressoCapConstraints = {
activityOptions: {
isObject: true
},
locale: {
appLocale: {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@mykola-mokhnach mykola-mokhnach changed the title feat: Add a possibility to set locale feat: Add a possibility to set app locale May 22, 2020
@KazuCocoa
Copy link
Member

How did you check this feature?
I downloaded Firefox app, and set it as the app under test. The device under test was Android 10 Nokia device.

But the locale (displayed language) did not change by this via mobile command even only application-wide.

@mykola-mokhnach
Copy link
Contributor Author

mykola-mokhnach commented May 22, 2020

I tried Android 8. I suppose you have to restart the activity or start a new one/restart the current to have the changes applied to the UI.

@KazuCocoa
Copy link
Member

KazuCocoa commented May 22, 2020

Thanks.
Yeah, it worked in application-wide after killing running activity, and launch it again.

Even we provide appLocale capability, the initial activity does not have the appLocale one.
I mean when the device is en_US and I set appLocale as appLocale: {language: 'ja', country: 'JP'},, the initial activity has the en_US's one. Once I close the activity and launch a new or the same one, the appLocale has the jp's one.

We should apply the locale change before starting the app under test's process to apply this from the beginning. In Espresso case, I imagine applying this change in ActivityScenario before launching the activity under test.

@mykola-mokhnach
Copy link
Contributor Author

Even we provide appLocale capability, the initial activity does not have the appLocale one.

Yes, this is something I was getting as well. And this is strange, because I actually set the stuff before starting the activity

@mykola-mokhnach
Copy link
Contributor Author

mykola-mokhnach commented May 22, 2020

Added some methods to restart the activity after locale change. Perhaps, it could make it working

@KazuCocoa
Copy link
Member

KazuCocoa commented May 23, 2020

I have Android 6 and Android 10 devices.
#580 (comment) worked for Android 6 with the current logic with capabilities. When I sent mobile:setAppLocal as {language: 'fa', country: 'AF'}, the preference was applied the locale partially (it depends on Firefox app's implementation, I believe) after backing the app to background and moving it forward with launch the app by history. Once I killed the app process and launched it again, the {language: 'fa', country: 'AF'} was applied fully. So, capabilities style works expectedly but we should note for mobile command the locale will be applied activities which launches after the command. (Or I also think only capabilities is fine to reduce user's confusion)

But no luck for Android 10 even I did the same steps with Android 6. I mean both via capabilities and mobile command. I needed to kill the activity once and launch them again by manual.

@mykola-mokhnach
Copy link
Contributor Author

thanks for your help
Let me focus better on Android 10 then.

@mykola-mokhnach
Copy link
Contributor Author

Let me know if you have any ideas regarding the code modification

@mykola-mokhnach
Copy link
Contributor Author

mykola-mokhnach commented May 23, 2020

You were right @KazuCocoa
We never know how to restart an activity properly, since it depends on the particular app implementation. Thus I've removed the mobile: endpoint and only keep the capability for setting the locale on app startup. Tested on Android 10 and Android 5 (Firefox binary): both worked as expected

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

👍
I checked this method worked on my phone, too.

I think this way, leaving only the capability, is the best enough way for us (as espresso-driver)
The Espresso approach depends on the app under test's implementation.
So, generally, users should decide if follow this application-wide method or system-wide (by settings app, but hacky one) in their tests

}
return null
}
for (method in listOf(method1, method2)) {
Copy link
Member

Choose a reason for hiding this comment

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

👍

Locale.setDefault(Locale.Category.DISPLAY, locale)
}

if (SDK_INT >= Build.VERSION_CODES.N) {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@mykola-mokhnach mykola-mokhnach merged commit 74910bf into appium:master May 24, 2020
@mykola-mokhnach mykola-mokhnach deleted the locale branch May 24, 2020 05:38
@KazuCocoa KazuCocoa mentioned this pull request May 29, 2020
10 tasks
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.

None yet

2 participants