-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Close #9960: Launch URI in Fenix if failed to launch in custom tab #9964
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9964 +/- ##
============================================
- Coverage 19.37% 19.17% -0.21%
+ Complexity 538 521 -17
============================================
Files 339 336 -3
Lines 13801 13732 -69
Branches 1847 1844 -3
============================================
- Hits 2674 2633 -41
+ Misses 10888 10860 -28
Partials 239 239
Continue to review full report at Codecov.
|
|
||
val deepLinkScheme: String | ||
get() = when (this) { | ||
FenixNightly -> "fenix-nightly" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the time, when we added the deepLinkScheme, the only use case for it was in the manifest.
I don't think we can avoid the duplication to set the values in the manifest an in the Kotlin layer, but what do you think about using a buildConfigField
so we set the values in the same build.gradle
place? We wouldn't need the Config.deepLinkScheme
parsing and it would be less likely for errors if we decide to change our variants tomorrow (it's quite likely actually with LeanPlum things 😄 ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll look into creating a buildConfigField
. Thanks,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨
@@ -33,9 +33,11 @@ android { | |||
resValue "bool", "IS_DEBUG", "false" | |||
buildConfigField "boolean", "USE_RELEASE_VERSIONING", "false" | |||
buildConfigField "String", "AMO_COLLECTION", "\"7e8d6dc651b54ab385fb8791bf9dac\"" | |||
def deepLinkSchemeValue = "fenix-dev" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice! I didn't think this would work for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too. Luckily found this: https://developer.android.com/studio/build/gradle-tips#share-properties-with-the-manifest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
Pull Request checklist
After merge
To download an APK when reviewing a PR: