Conversation
Add a static home screen shortcut that warns users about data loss when uninstalling the app. Includes shortcut handling infrastructure in AppDelegate.swift.
Add a static home screen shortcut that warns users about data loss when uninstalling the app.
Add a shortcut to quickly open the support chat.
Add a shortcut to quickly open the support chat.
Co-authored-by: Cursor <cursoragent@cursor.com>
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.
| android:shortcutLongLabel="@string/shortcut_contact_support_long"> | ||
| <intent | ||
| android:action="android.intent.action.VIEW" | ||
| android:data="https://support.edge.app/hc/en-us?chat=open" /> |
There was a problem hiding this comment.
Critical: Android shortcuts will not work — https URLs are not opened in the browser.
The iOS implementation in AppDelegate.swift intercepts https/http URLs and opens them directly in Safari:
if url.scheme == "https" || url.scheme == "http" {
UIApplication.shared.open(url)
return true
}However, Android has no equivalent native handling. The intent sends the URL to MainActivity, which passes it through React Native's Linking → parseDeepLink → launchDeepLink. The 'other' case tries to parse it as a wallet URI, fails, and shows "No wallet for this URI" toast.
Recommended fix (consistent with iOS): Add native Android handling in MainActivity to open https/http shortcut URLs in the browser before they reach React Native. This matches the iOS pattern where https URLs bypass parseDeepLink.
Alternative design consideration: If the preference is to have parseDeepLink be the central handler for all intent URIs, the fix would instead be:
- Update the
'other'case inDeepLinkingActions.tsxto open unrecognized https URLs in the browser as a fallback (similar tolinkReferralWithCurrencieswhich already doesif (parsed.type === 'other') await Linking.openURL(uri)) - Update iOS
AppDelegate.swiftto pass https URLs throughRCTLinkingManagerinstead of opening them directly
This would centralize URL handling logic in one place (the deep link infrastructure) rather than having platform-specific native handling.
| <string name="shortcut_contact_support_short">Contact Support</string> | ||
| <string name="shortcut_contact_support_long">Contact support for help from a live agent</string> | ||
| <string name="shortcut_do_not_uninstall_short">⚠️ Save 2FA First!</string> | ||
| <string name="shortcut_do_not_uninstall_long">⚠️ Login requires 2FA & credentials!</string> |
There was a problem hiding this comment.
Critical: Unescaped ampersand in XML.
In XML, & must be escaped as &. An unescaped & can cause parse errors or unexpected behavior.
Fix:
<string name="shortcut_do_not_uninstall_long">⚠️ Login requires 2FA & credentials!</string>| <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: Inconsistent copy between platforms.
| Platform | Contact Support (long) |
|---|---|
| Android | "Contact support for help from a live agent" |
| iOS | "Get help from our live support agents" |
Same feature with different wording can be confusing for localization and UX.
Recommendation: Pick one and use it on both platforms.
| <string name="shortcut_do_not_uninstall_short">⚠️ Save 2FA First!</string> | ||
| <string name="shortcut_do_not_uninstall_long">⚠️ Login requires 2FA & credentials!</string> |
There was a problem hiding this comment.
Warning: Double emoji on Android vs single on iOS.
- Android short: "
⚠️ Save 2FA First!" - Android long: "
⚠️ Login requires 2FA & credentials!" - iOS title: "
⚠️ Save 2FA First!" - iOS subtitle: "Login requires 2FA & credentials!" (no emoji)
Android uses
Recommendation: Remove
| @@ -0,0 +1,23 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <shortcuts xmlns:android="http://schemas.android.com/apk/res/android"> | |||
| <shortcut | |||
There was a problem hiding this comment.
Warning: Shortcut order differs between platforms.
- iOS: 2FA warning first, Contact Support second
- Android: Contact Support first, 2FA warning second
If the order is intentional for platform UX, add a comment explaining the rationale. Otherwise, align for consistency.
| <string>Login requires 2FA & credentials!</string> | ||
| <key>UIApplicationShortcutItemIconSymbolName</key> | ||
| <string>nosign</string> | ||
| <key>UIApplicationShortcutItemType</key> |
There was a problem hiding this comment.
Suggestion: UIApplicationShortcutItemType is typically an identifier (e.g. com.edge.app.contact_support), not a full URL.
Storing URLs there works, but it mixes identity with data. Consider using a type like contact_support and passing the URL via userInfo for clearer separation. The handleShortcutItem function would then switch on the type and look up the URL.
| <shortcut | ||
| android:shortcutId="contact_support" | ||
| android:enabled="true" | ||
| android:icon="@mipmap/ic_launcher" |
There was a problem hiding this comment.
Suggestion: Both Android shortcuts use the same app launcher icon (@mipmap/ic_launcher), making them visually identical.
iOS uses distinct SF Symbols (nosign and message.fill) for each shortcut. Consider using different icons for Android too to improve shortcut recognition and UX.
|
|
||
| private func handleShortcutItem(_ shortcutItem: UIApplicationShortcutItem) -> Bool { | ||
| guard let url = URL(string: shortcutItem.type) else { return false } | ||
|
|
There was a problem hiding this comment.
Suggestion: UIApplication.shared.open has a completion handler that could help with debugging or analytics.
UIApplication.shared.open(url, options: [:]) { success in
if !success {
// Log or handle failure
}
}Not critical, but could be useful for future troubleshooting.
| class AppDelegate: UIResponder, UIApplicationDelegate { | ||
| var window: UIWindow? | ||
| var securityView: UIView? | ||
| private var pendingShortcutItem: UIApplicationShortcutItem? |
There was a problem hiding this comment.
Suggestion: Consider adding a brief comment explaining why shortcut handling is deferred.
The pattern of storing pendingShortcutItem and handling it after React Native setup is useful but not immediately obvious. A comment like:
// Shortcut actions are deferred until React Native is fully initialized
private var pendingShortcutItem: UIApplicationShortcutItem?would clarify the intent for future readers.
| </array> | ||
| <key>CFBundleVersion</key> | ||
| <string>99999999</string> | ||
| <key>UIApplicationShortcutItems</key> |
There was a problem hiding this comment.
Localization: These shortcut strings are hardcoded in English only.
The app supports 13 locales via React Native (src/locales/strings/*.json), but these native shortcuts won't be translated.
Recommendation: Use InfoPlist.strings for localization:
- Create
ios/edge/en.lproj/InfoPlist.strings:
SHORTCUT_UNINSTALL_TITLE = "⚠️ Save 2FA First!";
SHORTCUT_SUPPORT_TITLE = "Contact Support";
-
Create locale-specific
.lprojfolders (e.g.,es.lproj,de.lproj) with translated strings. -
Update
Info.plistto use the keys instead of hardcoded strings.
Similarly for Android, add values-es/strings.xml, values-de/strings.xml, etc.
This could be deferred to a follow-up PR, but should at least be documented as a known limitation.
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: