[local_auth] Add Android theme compatibility documentation #6875

Abel1027 · 2022-12-22T14:51:55Z

Add documentation to README.md file from local_auth package:

Compatibility of Android theme for Android 8 and below.

On Android 8 and below the app crashes if the AppCompat theme is not used. For example, I had this default theme in my Flutter project (AndroidManifest.xml) when I created the project and it makes the app crashes when authenticating with biometric:

<activity
            ...
            android:theme="@style/LaunchTheme"
            ...
>

In order to fix it, I had to replace the style with this one:

<activity
            ...
            android:theme="@style/Theme.AppCompat.Light"
            ...
>

This is the crash:

java.lang.IllegalStateException: You need to use a Theme.AppCompat theme (or descendant) with this activity.
at

androidx.appcompat.app.AppCompatDelegateImpl.createSubDecor(AppCompatDelegateImpl.java:852)

at

androidx.appcompat.app.AppCompatDelegateImpl.ensureSubDecor(AppCompatDelegateImpl.java:815)

at

androidx.appcompat.app.AppCompatDelegateImpl.setContentView(AppCompatDelegateImpl.java:703)

at

androidx.appcompat.app.AppCompatDialog.setContentView(AppCompatDialog.java:95)

at

androidx.appcompat.app.AlertController.installContent(AlertController.java:232)

at

androidx.appcompat.app.AlertDialog.onCreate(AlertDialog.java:279)

at

android.app.Dialog.dispatchOnCreate(Dialog.java:516)

at

android.app.Dialog.show(Dialog.java:382)

at

androidx.fragment.app.DialogFragment.onStart(DialogFragment.java:728)

at

androidx.fragment.app.Fragment.performStart(Fragment.java:3162)

at

androidx.fragment.app.FragmentStateManager.start(FragmentStateManager.java:588)

at

androidx.fragment.app.FragmentStateManager.moveToExpectedState(FragmentStateManager.java:279)

at

androidx.fragment.app.FragmentManager.executeOpsTogether(FragmentManager.java:1899)

at

androidx.fragment.app.FragmentManager.removeRedundantOperationsAndExecute(FragmentManager.java:1823)

at

androidx.fragment.app.FragmentManager.execPendingActions(FragmentManager.java:1760)

at

androidx.fragment.app.FragmentManager$5.run(FragmentManager.java:547)

at

android.os.Handler.handleCallback(Handler.java:809)

at

android.os.Handler.dispatchMessage(Handler.java:102)

at

android.os.Looper.loop(Looper.java:166)

at

android.app.ActivityThread.main(ActivityThread.java:7555)

at

java.lang.reflect.Method.invoke(Native Method:-2)

at

com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:469)

at

com.android.internal.os.ZygoteInit.main(ZygoteInit.java:963)

google-cla · 2022-12-22T14:51:58Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

stuartmorgan · 2022-12-23T16:26:22Z

Thanks for the submission!

In the future, please do not delete the checklist that is in the PR template; it is there for a reason.

stuartmorgan · 2022-12-23T16:27:40Z

Adding @camsim99 for review since this is Android-specific; I'm not sure what the impact of switching away from LaunchTheme is, but it seems likely that just unconditionally changing this without changing anything else is correct. (Also, I would imagine that hard-coding .Light is not the only option here.)

camsim99

This seems reasonable, I just have one suggestion.

@Abel1027 can you also clarify how you know this is specifically for compatibility with Android 8 and below?

packages/local_auth/local_auth/README.md

Abel1027 · 2023-01-06T13:19:45Z

@camsim99 I have updated the doc with the change you suggested me. I encourage the developer to change the LaunchTheme in the styles.xml file in case it has one; otherwise it could be changed from the AndroidManifest.xml directly.

I noticed that this is specifically for Android 8 and below because me and my team developed a Flutter app that we released to production three weeks ago and we noticed that there were a lot of Android 7 and 8 users with app crashes in the Dynatrace console. We went to the console and we checked out that the crash was related with AppTheme.

Then, I tested the fingerprint functionality from Android 6 (the first one with biometric support) to Android 8 and in all of them the app crashes when I tried to use biometrics. However, for Android 9, 11 or 13 the issue was not reproduced.

After that, I searched for solutions and I found the one that I suggest in the PR. Then, I tested again all of those Android versions and everything worked well for all of them.

Now, with your suggestion @camsim99, I made the same tests and everything is OK too.

I have also attached the screenshot were you can see the users affected by the crash in the Dynatrace console.

packages/local_auth/local_auth/README.md

Co-authored-by: Camille Simon <43054281+camsim99@users.noreply.github.com>

Abel1027 · 2023-01-13T13:32:42Z

@camsim99 I agree with you about ensuring this is true for anyone so I already created the commit with the few changes you suggested. Thanks!

camsim99

LGTM! @stuartmorgan for second review

Abel1027 · 2023-01-27T22:32:04Z

Hi @stuartmorgan, have you had a chance to check the latest changes? Thank you in advance

stuartmorgan · 2023-01-30T16:25:50Z

packages/local_auth/local_auth/README.md

@@ -252,6 +252,37 @@ types (such as face scanning) and you want to support SDKs lower than Q,
 _do not_ call `getAvailableBiometrics`. Simply call `authenticate` with `biometricOnly: true`.
 This will return an error if there was no hardware available.

+#### Android theme
+
+On Android, ensure that the `LaunchTheme` of your app has a parent style with the `Theme.AppCompat.Light` theme; otherwise the app may crash. To do that go to `android > app > src > main > res > values > styles.xml` and look for the style with name `LaunchTheme` (Notice that `LaunchTheme` must be referenced in the `AndroidManifest.xml` file). Then, change the parent style with the new value and you should have set up something like this:


Nit: Please wrap all of the new text here to 80 characters, as with the rest of the README.

stuartmorgan · 2023-01-30T16:26:22Z

packages/local_auth/local_auth/README.md

@@ -252,6 +252,37 @@ types (such as face scanning) and you want to support SDKs lower than Q,
 _do not_ call `getAvailableBiometrics`. Simply call `authenticate` with `biometricOnly: true`.
 This will return an error if there was no hardware available.

+#### Android theme
+
+On Android, ensure that the `LaunchTheme` of your app has a parent style with the `Theme.AppCompat.Light` theme; otherwise the app may crash. To do that go to `android > app > src > main > res > values > styles.xml` and look for the style with name `LaunchTheme` (Notice that `LaunchTheme` must be referenced in the `AndroidManifest.xml` file). Then, change the parent style with the new value and you should have set up something like this:


You can remove "On Android", since this is in the Android section of the README.

stuartmorgan · 2023-01-30T16:27:47Z

packages/local_auth/local_auth/README.md

@@ -252,6 +252,37 @@ types (such as face scanning) and you want to support SDKs lower than Q,
 _do not_ call `getAvailableBiometrics`. Simply call `authenticate` with `biometricOnly: true`.
 This will return an error if there was no hardware available.

+#### Android theme
+
+On Android, ensure that the `LaunchTheme` of your app has a parent style with the `Theme.AppCompat.Light` theme; otherwise the app may crash. To do that go to `android > app > src > main > res > values > styles.xml` and look for the style with name `LaunchTheme` (Notice that `LaunchTheme` must be referenced in the `AndroidManifest.xml` file). Then, change the parent style with the new value and you should have set up something like this:


Why is .Light the only option here? Why not just Theme.AppCompat?

I made some research and test all variants of the Theme.AppCompat, and these were the results:

Using Theme.AppCompat alone results in a dialog with weird (no contrast) theme:

Therefore, only two options left from the documentation:

Light

Dark

However, since API 29 there is an extra option: DayNight, which enables light/dark modes switching when the user changes the theme from the device's settings.

To check the later, I set up the Theme.AppCompat.DayNight theme and these are the results:

Choosing light mode from device's settings

Chossing dark mode from device's settings

The good thing is that even the Theme.AppCompat.DayNight theme has only sense from API 29, it maps the available mode of lower API versions. The following is an example of using DayNight on API 23 (Android 6):

As the options of AppCompat are related to a usecase, I updated the doc by suggesting to apply one of the AppCompat options. And I gave the DayNight example:

You need to update the LaunchTheme parent style with a valid Theme.AppCompat
theme to be compatible with Android version 8 and below, otherwise the app
crashes for those versions. For example, use Theme.AppCompat.DayNight to
enable light/dark modes for the biometric dialog.

stuartmorgan · 2023-01-30T16:28:20Z

packages/local_auth/local_auth/README.md

@@ -252,6 +252,37 @@ types (such as face scanning) and you want to support SDKs lower than Q,
 _do not_ call `getAvailableBiometrics`. Simply call `authenticate` with `biometricOnly: true`.
 This will return an error if there was no hardware available.

+#### Android theme
+
+On Android, ensure that the `LaunchTheme` of your app has a parent style with the `Theme.AppCompat.Light` theme; otherwise the app may crash. To do that go to `android > app > src > main > res > values > styles.xml` and look for the style with name `LaunchTheme` (Notice that `LaunchTheme` must be referenced in the `AndroidManifest.xml` file). Then, change the parent style with the new value and you should have set up something like this:


Nit: replace the ; with a , since the final clause is not a full sentence.

stuartmorgan · 2023-01-30T16:29:06Z

packages/local_auth/local_auth/README.md

@@ -252,6 +252,37 @@ types (such as face scanning) and you want to support SDKs lower than Q,
 _do not_ call `getAvailableBiometrics`. Simply call `authenticate` with `biometricOnly: true`.
 This will return an error if there was no hardware available.

+#### Android theme
+
+On Android, ensure that the `LaunchTheme` of your app has a parent style with the `Theme.AppCompat.Light` theme; otherwise the app may crash. To do that go to `android > app > src > main > res > values > styles.xml` and look for the style with name `LaunchTheme` (Notice that `LaunchTheme` must be referenced in the `AndroidManifest.xml` file). Then, change the parent style with the new value and you should have set up something like this:


I don't understand the parenthetical; why does someone need to "notice" something in order to make this change, particularly something in another file?

In AndroidManifest.xml you specify the name of the style you have defined in the styles.xml, therefore if you don't notice that you have referenced the correct name the theme in styles.xml won't apply.

I updated this part telling the reader that this reference makes possible the application of the defined theme in styles.xml:

(Notice that LaunchTheme must be referenced in the AndroidManifest.xml file to apply the changes made in styles.xml)

if you don't notice that you have referenced the correct name the theme in styles.xml won't apply

Whether or not someone notices that its' there is irrelevant to whether it works, only whether it is there. So this is telling someone to do something that doesn't help ("notice" something) if it is there, and also doesn't tell them what to do if it's not. The instructions here for styles.xml seem to assume that the reader isn't familiar with any of this and thus needs extremely detailed instructions; the instructions for AndroidManifest.xml should equally clear and detailed, assuming the same level of reader knowledge.

I understand. Well, due to Flutter creates the styles.xml always with a LaunchTheme style name and it is associated to the AndroidManifest.xml by default, I will remove the phrase between parentheses (...).

stuartmorgan · 2023-01-30T16:31:09Z

packages/local_auth/local_auth/README.md

@@ -252,6 +252,37 @@ types (such as face scanning) and you want to support SDKs lower than Q,
 _do not_ call `getAvailableBiometrics`. Simply call `authenticate` with `biometricOnly: true`.
 This will return an error if there was no hardware available.

+#### Android theme
+
+On Android, ensure that the `LaunchTheme` of your app has a parent style with the `Theme.AppCompat.Light` theme; otherwise the app may crash. To do that go to `android > app > src > main > res > values > styles.xml` and look for the style with name `LaunchTheme` (Notice that `LaunchTheme` must be referenced in the `AndroidManifest.xml` file). Then, change the parent style with the new value and you should have set up something like this:


Nit: "and you should have set up something like this" isn't really adding anything to the instructions, it's just making it longer. "Then change the parent for that style as follows:" conveys the same information, but is much shorter.

stuartmorgan · 2023-01-30T16:31:44Z

packages/local_auth/local_auth/README.md

+...
+```
+
+In case you don't have a `styles.xml` file for your Android project you can set up the Android theme directly on your `android > app > src > main > AndroidManifest.xml` file:


Nit: If you don't have

stuartmorgan · 2023-01-30T16:32:01Z

packages/local_auth/local_auth/README.md

+...
+```
+
+In case you don't have a `styles.xml` file for your Android project you can set up the Android theme directly on your `android > app > src > main > AndroidManifest.xml` file:


Nit: directly in

stuartmorgan · 2023-01-30T16:33:01Z

packages/local_auth/local_auth/README.md

+...
+```
+
+In case you don't have a `styles.xml` file for your Android project you can set up the Android theme directly on your `android > app > src > main > AndroidManifest.xml` file:


Nit: remove "your" and "file" at the end; the fact that it's a file in their project is clear from context.

stuartmorgan · 2023-01-30T16:33:56Z

packages/local_auth/local_auth/README.md

@@ -252,6 +252,37 @@ types (such as face scanning) and you want to support SDKs lower than Q,
 _do not_ call `getAvailableBiometrics`. Simply call `authenticate` with `biometricOnly: true`.
 This will return an error if there was no hardware available.

+#### Android theme
+
+On Android, ensure that the `LaunchTheme` of your app has a parent style with the `Theme.AppCompat.Light` theme; otherwise the app may crash. To do that go to `android > app > src > main > res > values > styles.xml` and look for the style with name `LaunchTheme` (Notice that `LaunchTheme` must be referenced in the `AndroidManifest.xml` file). Then, change the parent style with the new value and you should have set up something like this:


Why are the paths here and below written like menu navigation instructions (with >) rather than as paths (with /)?

Abel1027 · 2023-02-07T16:34:54Z

@stuartmorgan I have made the requested changes and suggestions.

stuartmorgan

LGTM

…lutter/plugins#6875)

…lutter/plugins#6875) (#121144)

…entation (flutter/plugins#6875) (#121144) Commit: 23df770860dfb2277488b5a0223bf7eafae8fae5

…ntation (flutter#6875) (flutter/flutter#121144)

…ation (flutter/plugins#6875) (flutter/flutter#121144)

[DOC][Android] - Add Android theme compatibility documentation

eb0e98b

Abel1027 requested a review from stuartmorgan as a code owner December 22, 2022 14:51

github-actions bot added the p: local_auth label Dec 22, 2022

[VERSION] - Upgrade version to 2.1.4

a39808b

stuartmorgan changed the title ~~[DOC][Android] - Add Android theme compatibility documentation~~ [local_auth] Add Android theme compatibility documentation Dec 23, 2022

stuartmorgan requested a review from camsim99 December 23, 2022 16:26

camsim99 suggested changes Jan 5, 2023

View reviewed changes

packages/local_auth/local_auth/README.md Show resolved Hide resolved

[local_auth] Suggest set up Android LaunchTheme theme in styles.xml

144d4b1

camsim99 self-requested a review January 10, 2023 00:40

camsim99 reviewed Jan 12, 2023

View reviewed changes

packages/local_auth/local_auth/README.md Outdated Show resolved Hide resolved

[DOC][local_auth] Update Android compatibility suggestion

043b928

Co-authored-by: Camille Simon <43054281+camsim99@users.noreply.github.com>

camsim99 approved these changes Jan 13, 2023

View reviewed changes

stuartmorgan suggested changes Jan 30, 2023

View reviewed changes

Abel1027 added 5 commits February 4, 2023 12:39

[DOC][local_auth] Update android theme suggestion with DayNight option

1b1ab97

[local_auth] Merge origin changes

f444f14

[DOC][local_auth] Remove typo

890e7c7

Merge origin changes

51bfbca

[local_auth] Add Android theme description to changelog

0d70eb5

Abel1027 requested review from cbracken, cyanglaz, hellohuanlin, jmagman, LouiseHsu and vashworth as code owners February 4, 2023 12:40

Abel1027 added 2 commits February 7, 2023 17:06

[local_auth] Upgrade version to 2.1.4

3feee16

[DOC][local_auth] Amend android theme compatibility suggestion

443aa37

jmagman removed their request for review February 7, 2023 19:08

reidbaker removed the request for review from GaryQian February 9, 2023 19:28

stuartmorgan approved these changes Feb 21, 2023

View reviewed changes

stuartmorgan merged commit 34a6813 into flutter:main Feb 21, 2023

engine-flutter-autoroll mentioned this pull request Feb 21, 2023

Roll Plugins from 132d9c77da4d to 34a681353c78 (1 revision) flutter/flutter#121144

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 21, 2023

34a681353 [local_auth] Add Android theme compatibility documentation (f…

aa5a2f2

…lutter/plugins#6875)

auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 21, 2023

34a681353 [local_auth] Add Android theme compatibility documentation (f…

23df770

…lutter/plugins#6875) (#121144)

sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this pull request Feb 21, 2023

flutter: 34a681353 [local_auth] Add Android theme compatibility docum…

42dc983

…entation (flutter/plugins#6875) (#121144) Commit: 23df770860dfb2277488b5a0223bf7eafae8fae5

engine-flutter-autoroll mentioned this pull request Feb 21, 2023

Roll Flutter from 33e4d21f7c13 to 23df770860df (12 revisions) #7206

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 21, 2023

23df77086 34a6813 [local_auth] Add Android theme compatibility docume…

639c6e4

…ntation (flutter#6875) (flutter/flutter#121144)

engine-flutter-autoroll mentioned this pull request Feb 21, 2023

Roll Flutter from 33e4d21f7c13 to 6259b690f6e0 (15 revisions) #7207

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 21, 2023

23df77086 34a6813 [local_auth] Add Android theme compatibility docume…

40394b6

…ntation (flutter#6875) (flutter/flutter#121144)

engine-flutter-autoroll mentioned this pull request Feb 21, 2023

Roll Flutter from 33e4d21f7c13 to 09518ac72584 (17 revisions) #7208

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 21, 2023

23df77086 34a6813 [local_auth] Add Android theme compatibility docume…

6772bc5

…ntation (flutter#6875) (flutter/flutter#121144)

engine-flutter-autoroll mentioned this pull request Feb 21, 2023

Roll Flutter from 33e4d21f7c13 to bd5e8db3a331 (18 revisions) #7209

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 21, 2023

23df77086 34a6813 [local_auth] Add Android theme compatibility docume…

e518302

…ntation (flutter#6875) (flutter/flutter#121144)

This was referenced Feb 22, 2023

Roll Flutter from 7175de4fe686 to 4c25587b71aa (17 revisions) flutter/packages#3255

Closed

Roll Flutter from 33e4d21f7c13 to 4c25587b71aa (27 revisions) #7212

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 22, 2023

23df77086 34a6813 [local_auth] Add Android theme compatibility docume…

ad0eb3b

…ntation (flutter#6875) (flutter/flutter#121144)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2023

23df770 34a6813 [local_auth] Add Android theme compatibility document…

f8bb4c9

…ation (flutter/plugins#6875) (flutter/flutter#121144)

engine-flutter-autoroll mentioned this pull request Feb 22, 2023

Roll Flutter from 7175de4fe686 to d816e72f07a0 (18 revisions) flutter/packages#3262

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2023

23df770 34a6813 [local_auth] Add Android theme compatibility document…

d84425f

…ation (flutter/plugins#6875) (flutter/flutter#121144)

engine-flutter-autoroll mentioned this pull request Feb 22, 2023

Roll Flutter from 7175de4fe686 to 7d85a585da75 (19 revisions) flutter/packages#3264

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2023

23df770 34a6813 [local_auth] Add Android theme compatibility document…

c9844f6

…ation (flutter/plugins#6875) (flutter/flutter#121144)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[local_auth] Add Android theme compatibility documentation #6875

[local_auth] Add Android theme compatibility documentation #6875

Abel1027 commented Dec 22, 2022

google-cla bot commented Dec 22, 2022

stuartmorgan commented Dec 23, 2022

stuartmorgan commented Dec 23, 2022

camsim99 left a comment

Abel1027 commented Jan 6, 2023

Abel1027 commented Jan 13, 2023

camsim99 left a comment

Abel1027 commented Jan 27, 2023

stuartmorgan Jan 30, 2023

stuartmorgan Jan 30, 2023

stuartmorgan Jan 30, 2023

Abel1027 Feb 4, 2023 •

edited

Loading

stuartmorgan Jan 30, 2023

stuartmorgan Jan 30, 2023

Abel1027 Feb 4, 2023

stuartmorgan Feb 7, 2023

Abel1027 Feb 7, 2023

stuartmorgan Jan 30, 2023

stuartmorgan Jan 30, 2023

stuartmorgan Jan 30, 2023

stuartmorgan Jan 30, 2023

stuartmorgan Jan 30, 2023

Abel1027 commented Feb 7, 2023

stuartmorgan left a comment

[local_auth] Add Android theme compatibility documentation #6875

[local_auth] Add Android theme compatibility documentation #6875

Conversation

Abel1027 commented Dec 22, 2022

google-cla bot commented Dec 22, 2022

stuartmorgan commented Dec 23, 2022

stuartmorgan commented Dec 23, 2022

camsim99 left a comment

Choose a reason for hiding this comment

Abel1027 commented Jan 6, 2023

Abel1027 commented Jan 13, 2023

camsim99 left a comment

Choose a reason for hiding this comment

Abel1027 commented Jan 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abel1027 Feb 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Abel1027 commented Feb 7, 2023

stuartmorgan left a comment

Choose a reason for hiding this comment

Abel1027 Feb 4, 2023 •

edited

Loading