-
-
Notifications
You must be signed in to change notification settings - Fork 4
Handle mailto links properly #1983
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
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughImplements platform-specific handling for mailto: links. On Android, adds a BlazorWebView UrlLoading handler that intercepts mailto: and launches an external email intent while canceling in-webview navigation. Wires the handler in MainPage and exposes StartPath setter. In the frontend, marks the FeedbackDialog mailto button as external. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(no out-of-scope functional changes identified) Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
frontend/viewer/src/lib/about/FeedbackDialog.svelte (1)
38-39
: Small UX/accessibility touch-up (optional)Consider clarifying that this opens the email app (e.g., aria-label or visually hidden note) to set user expectation.
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs (1)
39-48
: Consider covering other external schemes (optional)If users tap tel:, sms:, or geo: links, you likely want similar behavior (launch external app and cancel WebView navigation). Easy extension alongside mailto.
backend/FwLite/FwLiteMaui/MainPage.xaml.cs (1)
15-15
: Event subscription is fine; avoid duplicates if this ctor can run more than onceIf there’s any chance this page gets re-initialized, pre-unsubscribe to prevent multiple handlers.
- blazorWebView.UrlLoading += BlazorWebViewOnUrlLoading; + blazorWebView.UrlLoading -= BlazorWebViewOnUrlLoading; // idempotency + blazorWebView.UrlLoading += BlazorWebViewOnUrlLoading;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs
(2 hunks)backend/FwLite/FwLiteMaui/MainPage.xaml.cs
(2 hunks)frontend/viewer/src/lib/about/FeedbackDialog.svelte
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-07T08:35:47.803Z
Learnt from: hahn-kev
PR: sillsdev/languageforge-lexbox#1885
File: backend/FwLite/FwLiteMaui/MainPage.xaml.Windows.cs:30-41
Timestamp: 2025-08-07T08:35:47.803Z
Learning: In backend/FwLite/FwLiteMaui/MainPage.xaml.Windows.cs, the SetPermissionStateAsync calls in BlazorWebViewInitialized method are intentionally used in a fire-and-forget pattern with pragma warning suppression for VSTHRD110, as awaiting them is not necessary and async void should be avoided.
Applied to files:
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs
backend/FwLite/FwLiteMaui/MainPage.xaml.cs
🧬 Code graph analysis (2)
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs (1)
backend/FwLite/FwLiteMaui/MainPage.xaml.cs (2)
BlazorWebViewOnUrlLoading
(40-40)BlazorWebViewOnUrlLoading
(43-45)
backend/FwLite/FwLiteMaui/MainPage.xaml.cs (1)
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs (1)
BlazorWebViewOnUrlLoading
(39-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build API / publish-api
- GitHub Check: Build UI / publish-ui
- GitHub Check: frontend-component-unit-tests
- GitHub Check: check-and-lint
- GitHub Check: frontend
- GitHub Check: Build FW Lite and run tests
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
frontend/viewer/src/lib/about/FeedbackDialog.svelte (1)
38-39
: External link flag for mailto looks correctUsing the Button’s external prop avoids target="_blank" so UrlLoading can intercept on Android. LGTM.
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs (1)
3-3
: Android intent namespace import is appropriateImporting Android.Content is needed for Intent usage. LGTM.
backend/FwLite/FwLiteMaui/MainPage.xaml.cs (1)
20-24
: StartPath setter addition looks goodThis keeps the API symmetrical and enables programmatic navigation.
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.
Worked for me on Maui and Android 👍
closes #1975
Description
mailto
links to ensure proper functionality.