feat: Add split tunneling#205
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
gradle/libs.versions.toml (1)
23-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate navigation catalog entries pointing at the same modules with different versions.
navigation-fragment/navigation-ui(lines 41-42, version 2.9.0) and the newandroidx-navigation-fragment/androidx-navigation-ui(lines 47-48, version 2.9.8) declare the exact sameandroidx.navigationmodules. Sinceapp/build.gradle.ktspulls in both pairs, Gradle resolves to the highest (2.9.8), but the duplication is confusing and easy to drift. Consolidate to a single alias/version per module.♻️ Suggested consolidation
-navigationFragment = "2.9.0" -navigationUi = "2.9.0" +navigationFragment = "2.9.8" +navigationUi = "2.9.8" @@ -navigationFragmentVersion = "2.9.8" -navigationUiVersion = "2.9.8"-androidx-navigation-fragment = { group = "androidx.navigation", name = "navigation-fragment", version.ref = "navigationFragmentVersion" } -androidx-navigation-ui = { group = "androidx.navigation", name = "navigation-ui", version.ref = "navigationUiVersion" }Then keep only the existing
navigation-fragment/navigation-uialiases inbuild.gradle.kts.Also applies to: 47-48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gradle/libs.versions.toml` around lines 23 - 24, The Gradle version catalog currently defines duplicate aliases for the same androidx.navigation modules with different versions, which should be consolidated to a single source of truth. Update the entries in libs.versions.toml so only one alias pair remains for navigation-fragment and navigation-ui, keeping the version that app/build.gradle.kts should use, and remove the redundant androidx-navigation-fragment/androidx-navigation-ui aliases to prevent drift and confusion. Use the existing navigationFragmentVersion, navigationUiVersion, and the navigation aliases as the reference points when cleaning this up.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/build.gradle.kts`:
- Around line 74-84: The dependency block in the Gradle build has duplicate
Navigation entries and hardcoded Fragment/RecyclerView versions. In the
app/build.gradle.kts dependencies section, keep only one Navigation pair by
choosing either libs.androidx.navigation.fragment/ui or
libs.navigation.fragment/ui, then replace the direct androidx.fragment:fragment
and androidx.recyclerview:recyclerview declarations with the catalog aliases
libs.androidx.fragment and libs.androidx.recyclerview for consistency.
In
`@app/src/main/java/io/netbird/client/ui/splittunneling/SplitTunnelingFragment.java`:
- Line 44: `SplitTunnelingFragment.loadApps()` is doing expensive package
enumeration and label/icon lookups on the UI thread from `onCreateView`, which
can cause ANR. Move the work in `loadApps()` to a background executor or similar
worker, then post the resulting app list back to the adapter on the main thread.
Keep `onCreateView` lightweight and use
`loadApps()`/`getInstalledApplications()` only off the main thread, with the UI
update happening after the background load completes.
In `@app/src/main/res/drawable/ic_chevron_right.xml`:
- Around line 6-8: The chevron drawable uses a hardcoded black fillColor, which
won’t adapt to dark theme styling. Update ic_chevron_right.xml to use a
theme-aware color reference or tint source instead of the fixed `#FF000000`, and
align it with the existing icon styling used in this feature so the drawable
renders correctly in both light and dark modes.
In `@app/src/main/res/layout/fragment_split_tunneling.xml`:
- Around line 11-16: The ConstraintLayout in fragment_split_tunneling currently
uses match_parent width, which prevents maxWidth and layout_gravity
center_horizontal from taking effect. Update the root layout in
fragment_split_tunneling.xml so the container width is wrap_content (while
keeping maxWidth and center_horizontal) to ensure the fragment is capped at
fragment_max_width and properly centered on wide screens.
In `@tool/src/main/java/io/netbird/client/tool/IFace.java`:
- Around line 153-164: The INCLUDE split-tunneling branch in IFace is treating
an empty apps list as a warning, but it actually leaves the VPN unrestricted
because no addAllowedApplication calls run. Update the logic in the mode ==
Preferences.SplitTunnelingMode.INCLUDE path to explicitly block setup when
apps.isEmpty() is true, either by returning an error or failing tunnel
configuration before iterating. Keep the behavior localized around the
builder.addAllowedApplication flow and the surrounding INCLUDE-mode handling so
the tunnel cannot be started with an empty allowlist.
---
Nitpick comments:
In `@gradle/libs.versions.toml`:
- Around line 23-24: The Gradle version catalog currently defines duplicate
aliases for the same androidx.navigation modules with different versions, which
should be consolidated to a single source of truth. Update the entries in
libs.versions.toml so only one alias pair remains for navigation-fragment and
navigation-ui, keeping the version that app/build.gradle.kts should use, and
remove the redundant androidx-navigation-fragment/androidx-navigation-ui aliases
to prevent drift and confusion. Use the existing navigationFragmentVersion,
navigationUiVersion, and the navigation aliases as the reference points when
cleaning this up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5740c52-9271-4903-ac6a-9800793ad867
📒 Files selected for processing (15)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/io/netbird/client/ui/advanced/AdvancedFragment.javaapp/src/main/java/io/netbird/client/ui/splittunneling/AppInfo.javaapp/src/main/java/io/netbird/client/ui/splittunneling/SplitTunnelingAdapter.javaapp/src/main/java/io/netbird/client/ui/splittunneling/SplitTunnelingFragment.javaapp/src/main/res/drawable/ic_chevron_right.xmlapp/src/main/res/layout/fragment_advanced.xmlapp/src/main/res/layout/fragment_split_tunneling.xmlapp/src/main/res/layout/list_item_app.xmlapp/src/main/res/navigation/mobile_navigation.xmlapp/src/main/res/values/strings.xmlgradle/libs.versions.tomltool/src/main/java/io/netbird/client/tool/IFace.javatool/src/main/java/io/netbird/client/tool/Preferences.java
| <androidx.constraintlayout.widget.ConstraintLayout | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:maxWidth="@dimen/fragment_max_width" | ||
| android:layout_gravity="center_horizontal" | ||
| android:padding="16dp"> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
maxWidth + layout_gravity centering is ineffective with match_parent width.
With android:layout_width="match_parent", the android:maxWidth constraint is ignored and layout_gravity="center_horizontal" has nothing to center, so on wide screens (tablets/landscape) this view will stretch full-width instead of being capped at fragment_max_width. Use wrap_content width so maxWidth and centering take effect.
♻️ Proposed fix
<androidx.constraintlayout.widget.ConstraintLayout
- android:layout_width="match_parent"
+ android:layout_width="wrap_content"
android:layout_height="match_parent"
android:maxWidth="`@dimen/fragment_max_width`"
android:layout_gravity="center_horizontal"
android:padding="16dp">Based on learnings: avoid centering a view with width set to match_parent using layout_gravity; use layout_width="wrap_content" with layout_gravity="center_horizontal" to center a maxWidth view.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <androidx.constraintlayout.widget.ConstraintLayout | |
| android:layout_width="match_parent" | |
| android:layout_height="match_parent" | |
| android:maxWidth="@dimen/fragment_max_width" | |
| android:layout_gravity="center_horizontal" | |
| android:padding="16dp"> | |
| <androidx.constraintlayout.widget.ConstraintLayout | |
| android:layout_width="wrap_content" | |
| android:layout_height="match_parent" | |
| android:maxWidth="`@dimen/fragment_max_width`" | |
| android:layout_gravity="center_horizontal" | |
| android:padding="16dp"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/res/layout/fragment_split_tunneling.xml` around lines 11 - 16,
The ConstraintLayout in fragment_split_tunneling currently uses match_parent
width, which prevents maxWidth and layout_gravity center_horizontal from taking
effect. Update the root layout in fragment_split_tunneling.xml so the container
width is wrap_content (while keeping maxWidth and center_horizontal) to ensure
the fragment is capped at fragment_max_width and properly centered on wide
screens.
Source: Learnings
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
I noticed when reading your codebase that your Wireguard implementation already supports split tunneling.
With this PR I introduce a new preference and screen for managing split tunneling only on certain specific apps.
I'm not an android developer, rather a full-stack one. I used AI to help me write the pull request.
I have compiled and tested it on my phone and it works as expected.
Motivation
I'm implementing this functionality because Netbird has significantly slowed down all the traffic on my phone, so scoping it only to the apps I need would be an easy win.
Screenshots
Related Issues
#72
Summary by CodeRabbit