Add home screen long-press shortcuts with localized labels#5950
Add home screen long-press shortcuts with localized labels#5950
Conversation
8ba3ec2 to
404fbc0
Compare
samholmes
left a comment
There was a problem hiding this comment.
Review Summary
2 Critical Issues:
- Android shortcuts won't open https URLs in browser (inconsistent with iOS implementation)
- Unescaped
&in Android strings.xml
4 Warnings: Inconsistent copy between platforms, double emoji on Android, different shortcut order, misleading "data loss" wording in changelog
5 Suggestions: UIApplicationShortcutItemType convention, Android icons, completion handler, code comments, localization support
See inline comments for details.
854b747 to
01725a8
Compare
2665230 to
a1f8cef
Compare
adb67a5 to
40001b3
Compare
j0ntz
left a comment
There was a problem hiding this comment.
Two new findings — the Android browser-intent dispatch lacks exception handling, and the iOS shortcut launch logic can be simplified. Existing reviewer feedback looks addressed.
| if (DEEP_LINK_HOSTS.contains(host)) return false | ||
|
|
||
| val browserIntent = Intent(Intent.ACTION_VIEW, data) | ||
| startActivity(browserIntent) |
There was a problem hiding this comment.
Warning: startActivity(browserIntent) can throw ActivityNotFoundException if no activity is registered to handle the ACTION_VIEW + https intent (e.g., device with no default browser, stripped-down enterprise builds, browsers disabled). The exception would propagate and crash the app when the user taps a shortcut.
Wrap in try-catch and fall through to super.onNewIntent / RN handling, or at minimum log and return false so the normal path runs:
return try {
startActivity(browserIntent)
true
} catch (e: ActivityNotFoundException) {
false
}| @@ -49,6 +52,11 @@ class AppDelegate: UIResponder, UIApplicationDelegate { | |||
| _ application: UIApplication, | |||
| didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? = nil | |||
| ) -> Bool { | |||
| let launchedFromShortcut = launchOptions?[.shortcutItem] is UIApplicationShortcutItem | |||
| if let shortcutItem = launchOptions?[.shortcutItem] as? UIApplicationShortcutItem { | |||
| pendingShortcutItem = shortcutItem | |||
| } | |||
|
|
|||
| // Initialize SDK's: | |||
| initializeSentry() | |||
| FirebaseApp.configure() | |||
| @@ -72,7 +80,37 @@ class AppDelegate: UIResponder, UIApplicationDelegate { | |||
| launchOptions: launchOptions | |||
| ) | |||
|
|
|||
| return true | |||
| if let shortcutItem = pendingShortcutItem { | |||
| _ = handleShortcutItem(shortcutItem) | |||
| pendingShortcutItem = nil | |||
| } | |||
|
|
|||
| return !launchedFromShortcut | |||
There was a problem hiding this comment.
Suggestion: pendingShortcutItem is stored as an instance property but is set and consumed synchronously within didFinishLaunchingWithOptions — nothing is actually deferred across an async boundary, so the "Deferred until React Native is fully initialized" comment is misleading. launchedFromShortcut also re-derives the same launchOptions check.
Can be collapsed to a local, e.g.:
let shortcutItem = launchOptions?[.shortcutItem] as? UIApplicationShortcutItem
// ... init Sentry/Firebase/RN ...
if let shortcutItem = shortcutItem {
_ = handleShortcutItem(shortcutItem)
}
return shortcutItem == nilRemoves the ivar, the bool, and the stale comment.
Add two static quick actions visible on long-press of the app icon: 1. "Save 2FA First!" - warns users to preserve credentials before uninstalling, links to the support article on getting a new phone. 2. "Contact Support" - opens the support chat page. On iOS, shortcuts are defined in Info.plist with localized titles via InfoPlist.strings, and handled in AppDelegate with the canonical return-false pattern to prevent double-delivery on cold start. On Android, shortcuts are declared in res/xml/shortcuts.xml and referenced from AndroidManifest.xml. URLs that aren't registered deep link hosts are routed to the default browser via maybeOpenInBrowser. Made-with: Cursor
Add translated shortcut labels for de, es, es-MX, fr, it, ja, ko, pt, ru, vi, and zh-Hans on both iOS (InfoPlist.strings) and Android (res/values-*/strings.xml). Register the locale .lproj bundles in the Xcode project. Made-with: Cursor
40001b3 to
e0655cb
Compare
| <resources> | ||
| <string name="app_name">Edge</string> | ||
| <string name="shortcut_contact_support_short">Contact Support</string> | ||
| <string name="shortcut_contact_support_long">Contact support for help from a live agent</string> |
There was a problem hiding this comment.
Warning: English Android long label still mismatches the iOS subtitle.
- Android (this file):
Contact support for help from a live agent - iOS (
en.lproj/InfoPlist.strings):Get help from our live support agents
Notably, all 11 non-English Android locales (values-de, values-es, values-fr, etc.) were translated against the iOS wording, so the English source string in this file is the only outlier in the localization tree.
Recommendation: Update to match the iOS subtitle:
<string name="shortcut_contact_support_long">Get help from our live support agents</string>| <resources> | ||
| <string name="shortcut_do_not_uninstall_short">⚠️ 2FA sichern!</string> | ||
| <string name="shortcut_do_not_uninstall_long">⚠️ Login erfordert 2FA & Zugangsdaten!</string> | ||
| <string name="shortcut_contact_support_short">Support kontakt.</string> |
There was a problem hiding this comment.
Optional / nit: Cross-platform short labels diverge for several locales (same class of inconsistency as samholmes' original review, but at the short-label level rather than the long-label level).
DE example here: Android Support kontakt. (note the trailing period — looks truncated/abbreviated) vs iOS Support anfragen in de.lproj/InfoPlist.strings.
Other divergences:
| Locale | Android short | iOS title |
|---|---|---|
de |
Support kontakt. |
Support anfragen |
es / es-MX |
Contactar soporte |
Contactar ayuda |
it |
Contatta supporto |
Contattaci |
ru |
Связаться (just "Contact") |
Поддержка |
Not blocking, but worth aligning so the same shortcut shows the same label across platforms within a given locale.
| @@ -0,0 +1,7 @@ | |||
| /* Home screen shortcut: uninstall warning */ | |||
There was a problem hiding this comment.
Optional / process question: This PR ships hand/AI-translated copy for 12 locales without going through Crowdin (which the repo normally uses for translations).
A few of the translations look like they would benefit from a real translator pass — e.g. Support kontakt. (DE, looks truncated), Связаться (RU, just "Contact" rather than "Contact Support"). It also creates a parallel translation pipeline that future shortcut additions will need to remember to update.
Suggestion (non-blocking): confirm with the team whether native shortcut strings should be added to Crowdin going forward, or formally accept this as a one-off carve-out and document it (e.g. in a brief note in the README or a comment in the .lproj files) so future contributors don't unknowingly let it drift.
| companion object { | ||
| private val DEEP_LINK_HOSTS = setOf( | ||
| "deep.edge.app", | ||
| "dl.edge.app", | ||
| "return.edge.app" | ||
| ) | ||
| } |
There was a problem hiding this comment.
Optional / nit: DEEP_LINK_HOSTS is duplicated from AndroidManifest.xml lines 60–62 (deep.edge.app, dl.edge.app, return.edge.app).
If a new deep-link host is added to the manifest, this set must be updated too — with no compile-time check to catch a mismatch. Could be brittle as the deep-link surface evolves.
Suggestion (non-blocking): read the registered hosts at runtime from the manifest (e.g. via PackageManager.getActivityInfo(componentName, GET_INTENT_FILTERS) or an Android <string-array> resource referenced by both files), or at minimum add a brief comment here pointing readers at the manifest as the source of truth.
| 81AB9BB82411601600AC10FF /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 81AB9BB72411601600AC10FF /* LaunchScreen.storyboard */; }; | ||
| BF115CD26A29F1C032E30289 /* Pods_edge.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E261C56FB78E202F218C2DCA /* Pods_edge.framework */; }; | ||
| E469AC702DC43791006A2530 /* AdServices.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = E469AC6F2DC43791006A2530 /* AdServices.framework */; settings = {ATTRIBUTES = (Weak, ); }; }; | ||
| F0A1B2C32E8A100000000001 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = F0A1B2C32E8A100000000003 /* InfoPlist.strings */; }; |
There was a problem hiding this comment.
Optional / nit: Hand-crafted Xcode UUIDs.
The 12 new file references and the variant group use sequential identifiers (F0A1B2C32E8A100000000001 … F0A1B2C32E8A10000000001A) rather than randomly generated 24-char hex from Xcode. They're valid and work today, but:
- They depart from the rest of the file's pattern, so subsequent edits via Xcode will mix two styles.
- There is a small (extremely small) chance of a future collision if Xcode happens to generate a similar low-entropy ID.
Suggestion (non-blocking): open the project in Xcode and let it regenerate proper random IDs (drag the InfoPlist.strings variant group out and back in, or re-add the localizations through the Project → Info → Localizations panel). Not necessary for correctness.
| if url.scheme == "https" || url.scheme == "http" { | ||
| UIApplication.shared.open(url, options: [:]) { success in | ||
| if !success { | ||
| print("Failed to open shortcut URL: \(urlString)") |
There was a problem hiding this comment.
Optional / nit: Failures land in print() only, which means production failures to open a shortcut URL won't be captured for diagnostics — they'll vanish from any user device that isn't attached to Xcode.
Suggestion (non-blocking): route through Sentry (SentrySDK.capture(message:) or a breadcrumb) so the failure is observable in production. Likely a low-frequency event, but the only way to know is to instrument it.
Alternatively, if Sentry instrumentation is intentionally skipped here, a brief comment explaining why would help future readers.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: