Skip to content

Conversation

@myieye
Copy link
Collaborator

@myieye myieye commented Oct 2, 2025

@coderabbitai
Copy link

coderabbitai bot commented Oct 2, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Introduces Mono.Unix as a centrally managed package and references it in FwLiteMaui with assets excluded. Cleans up unused using directives in several files. Adjusts Android WebChromeClient initialization in MainPage.xaml.Android.cs to use a version-aware base client and removes SetGeolocationDatabasePath.

Changes

Cohort / File(s) Summary of changes
Mono.Unix dependency introduction
backend/Directory.Packages.props, backend/FwLite/FwLiteMaui/FwLiteMaui.csproj
Added central version for Mono.Unix and a project-level PackageReference (ExcludeAssets="all"); minor formatting in props file.
Unused usings cleanup
backend/FwLite/FwDataMiniLcmBridge/Api/RichTextMapping.cs, backend/Testing/Services/*
Removed unused using directives (including Mono.Unix.Native, Polly, FluentAssertions, etc.); no logic changes.
Android WebChromeClient setup
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs
Reworked WebChromeClient initialization to be Android-version-aware; removed SetGeolocationDatabasePath; passes base client into PermissionManagingBlazorWebChromeClient.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10–15 minutes

Suggested labels

💻 FW Lite

Suggested reviewers

  • hahn-kev

Poem

I thump my paws, dependencies align,
A Unix moon on Maui’s vine. 🌙🐇
Chrome clients hop by API light,
Trimmed usings make the codepath tight.
With tidy trails and version cheer,
I nibble bugs—see you next release, my dear!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the main intent of the changeset by referencing the 16 KB page-size warning fix and additional Android warnings, which aligns with the changes to the .csproj, code files, and package references aimed at resolving those specific build issues. It is concise, clear, and focused on the primary purpose of the pull request.
Description Check ✅ Passed The description links directly to the official guidance on 16 KB page-size support, which is the central reference for the changes implemented in this pull request, making it relevant and on-topic.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added 💻 FW Lite issues related to the fw lite application, not miniLcm or crdt related 📦 Lexbox issues related to any server side code, fw-headless included labels Oct 2, 2025
@argos-ci
Copy link

argos-ci bot commented Oct 2, 2025

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No changes detected - Oct 3, 2025, 3:15 PM

@github-actions
Copy link

github-actions bot commented Oct 2, 2025

UI unit Tests

  1 files  ±0   45 suites  ±0   28s ⏱️ ±0s
111 tests ±0  111 ✅ ±0  0 💤 ±0  0 ❌ ±0 
160 runs  ±0  160 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit e8ea855. ± Comparison against base commit 4c03705.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 2, 2025

C# Unit Tests

130 tests   130 ✅  20s ⏱️
 20 suites    0 💤
  1 files      0 ❌

Results for commit e8ea855.

♻️ This comment has been updated with latest results.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ec05cf and aad14f9.

📒 Files selected for processing (6)
  • backend/Directory.Packages.props (2 hunks)
  • backend/FwLite/FwDataMiniLcmBridge/Api/RichTextMapping.cs (0 hunks)
  • backend/FwLite/FwLiteMaui/FwLiteMaui.csproj (1 hunks)
  • backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs (1 hunks)
  • backend/Testing/Services/JwtHelper.cs (0 hunks)
  • backend/Testing/Services/ModifyProjectHelper.cs (0 hunks)
💤 Files with no reviewable changes (3)
  • backend/Testing/Services/JwtHelper.cs
  • backend/FwLite/FwDataMiniLcmBridge/Api/RichTextMapping.cs
  • backend/Testing/Services/ModifyProjectHelper.cs
🧰 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
🧬 Code graph analysis (1)
backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs (1)
backend/FwLite/FwLiteMaui/Platforms/Android/PermissionManagingBlazorWebChromeClient.cs (2)
  • PermissionManagingBlazorWebChromeClient (21-231)
  • PermissionManagingBlazorWebChromeClient (57-62)
⏰ 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). (6)
  • GitHub Check: Build API / publish-api
  • GitHub Check: Build FwHeadless / publish-fw-headless
  • GitHub Check: Analyze (csharp)
  • GitHub Check: Build FW Lite and run tests
  • GitHub Check: frontend
  • GitHub Check: frontend-component-unit-tests
🔇 Additional comments (2)
backend/FwLite/FwLiteMaui/FwLiteMaui.csproj (1)

88-96: Verify runtime after Mono.Unix exclusion
Static search found no Mono.Unix references in any .cs files. Confirm at runtime that FwLiteMaui features work, SIL.WritingSystems/SIL.LCMModel have no missing APIs, and the 16 KB page-size warnings are resolved.

backend/FwLite/FwLiteMaui/MainPage.xaml.Android.cs (1)

35-38: Clarify Android version rationale and geolocation API deprecation

  • WebSettings.setGeolocationDatabasePath was deprecated in API 24; modern WebView manages storage internally—use WebSettings.setGeolocationEnabled plus WebChromeClient.onGeolocationPermissionsShowPrompt with runtime location permissions.
  • Explain why on Android 26+ you reuse e.WebView.WebChromeClient if set versus always creating a new instance on older OS versions.

@myieye myieye force-pushed the fix-android-build-warnings branch from aad14f9 to e8ea855 Compare October 3, 2025 15:12
@myieye myieye merged commit 3dc65b5 into develop Oct 3, 2025
25 checks passed
@myieye myieye deleted the fix-android-build-warnings branch October 3, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💻 FW Lite issues related to the fw lite application, not miniLcm or crdt related 📦 Lexbox issues related to any server side code, fw-headless included

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants