Align the Android client with the iOS and desktop clients - #179
Open
pappz wants to merge 100 commits into
Open
Conversation
Replace the side drawer + collapsible bottom-sheet pattern with a bottom navigation bar (4 tabs: Home, Peers, Resources, Settings) to mirror the iOS client's TabView. Sub-screens (Advanced, Profiles, Change Server, Troubleshoot, About) are reached from the Settings tab and use an iOS-style sectioned list layout. - New SettingsFragment (sectioned list mirroring iOSSettingsView) - Promote PeersFragment / NetworksFragment to top-level destinations; drop the modal BottomDialogFragment + PagerAdapter - Profile chip on Home opens a ProfilePickerSheet (one-tap switch + Manage profiles link), echoing the iOS ProfileBadge - Restyle AdvancedFragment + TroubleshootFragment as sectioned lists; Theme Mode now opens a bottom-sheet picker - Refresh menu icons to thinner outlined Material Symbols - NavigationRailView via layout-w960dp for large screens / TV - Toolbar hidden on top-level destinations; visible on sub-screens - Profile cards adopt PR #137 dark-mode contrast fix - Treat the empty-profile-state JSON read as a normal first-launch case in ProfileManagerWrapper instead of logging at error level
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe app switches from drawer navigation to bottom navigation, adds profile and theme bottom sheets, rebuilds home and settings screens around row-based layouts, and removes legacy dialog and Lottie-based UI pieces. ChangesNavigation, profile, and settings shell
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
app/src/main/res/layout/list_item_profile_picker.xml (1)
21-29: ⚡ Quick winConstrain profile name to a single line for stable row layout.
At Line 21-29, long names can wrap and push row height unexpectedly. Add
maxLines="1"andellipsize="end"to keep picker rows consistent.🤖 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/list_item_profile_picker.xml` around lines 21 - 29, The TextView with id profile_picker_name can wrap long names and change row height; update the element (profile_picker_name) to constrain it to a single line by adding maxLines="1" and ellipsize="end" so overflowing text is truncated with an ellipsis and picker rows remain a stable height.app/src/main/res/drawable/ic_nav_settings.xml (1)
7-8: ⚡ Quick winAvoid hardcoded white fill for navigation icons.
Line 7 uses
#FFFFFFFF, which makes this asset less theme-adaptive. Prefer a theme color (or rely on menu/icon tint) so the icon works across light/dark and future palette changes.🤖 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/drawable/ic_nav_settings.xml` around lines 7 - 8, The vector drawable currently hardcodes android:fillColor="#FFFFFFFF" which prevents theming; update the path element in ic_nav_settings.xml to use a theme attribute instead (e.g. replace android:fillColor="#FFFFFFFF" with android:fillColor="?attr/colorControlNormal" or another appropriate theme attr like ?attr/colorOnSurface), or remove the fillColor so the menu/icon tint can apply; ensure the path element with android:pathData remains unchanged.tool/src/main/java/io/netbird/client/tool/ProfileManagerWrapper.java (1)
56-64: ⚡ Quick winConsider improving error detection robustness for first-launch profile state handling.
The code currently detects empty profile state files by matching the error message
"unexpected end of JSON input". While this message originates from Go's standardencoding/jsonpackage (making it inherently stable), relying on message text remains fragile as a detection pattern. For improved maintainability, consider checking for the underlying exception type or implementing a more explicit state-check approach (e.g., attempting to detect empty/missing state files before callinggetActiveProfile(), or requesting gomobile expose a dedicated error code or exception type for this scenario).🤖 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 `@tool/src/main/java/io/netbird/client/tool/ProfileManagerWrapper.java` around lines 56 - 64, The code in ProfileManagerWrapper is brittle because it detects the first-launch empty profile by matching the error message text from getActiveProfile; instead, detect the empty/missing state more robustly before parsing (e.g., check the profile state file exists and its length/content is empty) or catch a specific exception type if gomobile exposes one; update getActiveProfile call-site to first inspect the profile state file (or wrap the parsing call and inspect the underlying cause) and only treat the empty-file case as a benign fallback (log via TAG) while letting other exceptions be logged as errors.app/src/main/res/layout/list_item_setting_section.xml (1)
4-12: ⚡ Quick winUse the shared
SettingsSectionHeaderstyle here to avoid style drift.This layout duplicates the same header attributes now defined in
@style/SettingsSectionHeader. Reusing the style keeps section headers consistent across screens.Suggested diff
<TextView xmlns:android="http://schemas.android.com/apk/res/android" xmlns:tools="http://schemas.android.com/tools" - android:layout_width="match_parent" - android:layout_height="wrap_content" - android:paddingStart="20dp" - android:paddingEnd="20dp" - android:paddingTop="24dp" - android:paddingBottom="8dp" - android:textAllCaps="true" - android:textColor="@color/nb_txt_light" - android:textSize="13sp" + style="@style/SettingsSectionHeader" tools:text="Connection" />🤖 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/list_item_setting_section.xml` around lines 4 - 12, The TextView in this layout duplicates header attributes that are already captured by the shared style; replace the inline attributes by applying style="@style/SettingsSectionHeader" on the TextView (remove duplicated android:textAllCaps, android:textColor, android:textSize and any padding/text attributes that the style covers) so the view uses the single source of truth (SettingsSectionHeader) and avoid style drift; ensure any attributes not in the style that are specific to this layout remain, and verify the TextView ID/text content is unchanged.
🤖 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/src/main/java/io/netbird/client/MainActivity.java`:
- Around line 129-142: The first-install onboarding only hides bottomNav but
still falls through to compute isTopLevel and shows the toolbar; update the
navController.addOnDestinationChangedListener handler so that when
destination.getId() == R.id.firstInstallFragment you both set
bottomNav.setVisibility(View.GONE) and call setToolbarVisible(false) (and then
skip the top-level logic, e.g., return or an else) so the onboarding screen
bypasses both bottom navigation and the toolbar; reference the listener,
firstInstallFragment, bottomNav, setToolbarVisible, and topLevelDestinations
when making the change.
In `@app/src/main/java/io/netbird/client/ui/home/ProfilePickerSheet.java`:
- Around line 133-143: The validation uses sanitizeProfileName(profileName) but
the code still calls profileManager.addProfile(profileName), so invalid
characters get written; change the write to use the sanitized value instead. In
ProfilePickerSheet.replace the call to profileManager.addProfile(profileName)
should be profileManager.addProfile(sanitized) (and likewise use sanitized in
the success Toast/getString call) so the persisted profile and user feedback
match the validated name.
In `@app/src/main/java/io/netbird/client/ui/settings/SettingsFragment.java`:
- Around line 56-58: The click handler for binding.rowDocumentation creates an
ACTION_VIEW Intent with DOCS_URL and directly calls startActivity, which can
crash if no browser-capable handler exists; update the lambda that handles
binding.rowDocumentation.setOnClickListener to first query the PackageManager
(e.g., via requireContext().getPackageManager()) and use
intent.resolveActivity(packageManager) or packageManager.queryIntentActivities
to ensure there's at least one handler before calling startActivity, and if none
is found fail gracefully (e.g., show a toast or disable the action).
In `@app/src/main/res/drawable/ic_add.xml`:
- Line 7: The vector path in ic_add.xml hardcodes android:fillColor="#FFFFFFFF";
remove that literal and make the icon theme-tintable by replacing the hardcoded
value with a neutral theme attribute (e.g. ?attr/colorControlNormal or
?attr/colorOnBackground) or a named color resource, so the host view/menu can
apply tinting; update the android:fillColor attribute accordingly (and ensure
ImageView/MenuItem usage applies tint if needed).
In `@app/src/main/res/layout/activity_main.xml`:
- Around line 27-45: The root ConstraintLayout in fragment_home.xml is missing
the bottom navigation inset padding; open fragment_home.xml, locate the root
ConstraintLayout element and add the attribute
android:paddingBottom="@dimen/bottom_nav_inset" so it matches other top-level
fragments (e.g., fragment_peers, fragment_networks) and prevents content from
being hidden behind the BottomNavigationView; ensure you reference the existing
dimen resource bottom_nav_inset (no other changes needed).
In `@app/src/main/res/layout/sheet_theme_picker.xml`:
- Around line 21-116: The theme rows (theme_row_system, theme_row_light,
theme_row_dark) don't expose selection state to TalkBack; update each row to set
an accessible role and dynamic state by adding a contentDescription or
stateDescription that reflects whether its associated check ImageView
(theme_check_system, theme_check_light, theme_check_dark) is visible/selected,
and update that description whenever selection changes; additionally, after
changing selection call announceForAccessibility with a short message (e.g.
"Light theme selected") or attach a custom AccessibilityDelegate on the row
views to send TYPE_VIEW_SELECTED/STATE_CHANGED events so screen readers announce
the new selection.
In `@app/src/main/res/values/dimens.xml`:
- Around line 11-12: The bottom_nav_inset resource is set universally to 80dp
but large-screen rail layouts shouldn't have that bottom inset; add a
configuration-specific override by creating a w960dp-qualified values dimen
resource (e.g., values with qualifier w960dp) that defines bottom_nav_inset as
0dp (or another rail-appropriate value) while keeping the existing default 80dp
in the base dimens.xml so large screens won't get unnecessary bottom padding.
---
Nitpick comments:
In `@app/src/main/res/drawable/ic_nav_settings.xml`:
- Around line 7-8: The vector drawable currently hardcodes
android:fillColor="#FFFFFFFF" which prevents theming; update the path element in
ic_nav_settings.xml to use a theme attribute instead (e.g. replace
android:fillColor="#FFFFFFFF" with android:fillColor="?attr/colorControlNormal"
or another appropriate theme attr like ?attr/colorOnSurface), or remove the
fillColor so the menu/icon tint can apply; ensure the path element with
android:pathData remains unchanged.
In `@app/src/main/res/layout/list_item_profile_picker.xml`:
- Around line 21-29: The TextView with id profile_picker_name can wrap long
names and change row height; update the element (profile_picker_name) to
constrain it to a single line by adding maxLines="1" and ellipsize="end" so
overflowing text is truncated with an ellipsis and picker rows remain a stable
height.
In `@app/src/main/res/layout/list_item_setting_section.xml`:
- Around line 4-12: The TextView in this layout duplicates header attributes
that are already captured by the shared style; replace the inline attributes by
applying style="@style/SettingsSectionHeader" on the TextView (remove duplicated
android:textAllCaps, android:textColor, android:textSize and any padding/text
attributes that the style covers) so the view uses the single source of truth
(SettingsSectionHeader) and avoid style drift; ensure any attributes not in the
style that are specific to this layout remain, and verify the TextView ID/text
content is unchanged.
In `@tool/src/main/java/io/netbird/client/tool/ProfileManagerWrapper.java`:
- Around line 56-64: The code in ProfileManagerWrapper is brittle because it
detects the first-launch empty profile by matching the error message text from
getActiveProfile; instead, detect the empty/missing state more robustly before
parsing (e.g., check the profile state file exists and its length/content is
empty) or catch a specific exception type if gomobile exposes one; update
getActiveProfile call-site to first inspect the profile state file (or wrap the
parsing call and inspect the underlying cause) and only treat the empty-file
case as a benign fallback (log via TAG) while letting other exceptions be logged
as errors.
🪄 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: 411e484a-8e49-4f4f-b6b3-3b442e3b8742
📒 Files selected for processing (62)
app/src/main/java/io/netbird/client/MainActivity.javaapp/src/main/java/io/netbird/client/ui/advanced/AdvancedFragment.javaapp/src/main/java/io/netbird/client/ui/advanced/ThemePickerSheet.javaapp/src/main/java/io/netbird/client/ui/home/BottomDialogFragment.javaapp/src/main/java/io/netbird/client/ui/home/HomeFragment.javaapp/src/main/java/io/netbird/client/ui/home/PagerAdapter.javaapp/src/main/java/io/netbird/client/ui/home/ProfilePickerAdapter.javaapp/src/main/java/io/netbird/client/ui/home/ProfilePickerSheet.javaapp/src/main/java/io/netbird/client/ui/settings/SettingsFragment.javaapp/src/main/res/drawable/ic_add.xmlapp/src/main/res/drawable/ic_arrow_drop_down.xmlapp/src/main/res/drawable/ic_check.xmlapp/src/main/res/drawable/ic_chevron_right.xmlapp/src/main/res/drawable/ic_menu_about.xmlapp/src/main/res/drawable/ic_menu_advanced.xmlapp/src/main/res/drawable/ic_menu_change_server.xmlapp/src/main/res/drawable/ic_menu_docs.xmlapp/src/main/res/drawable/ic_menu_profile.xmlapp/src/main/res/drawable/ic_menu_troubleshoot.xmlapp/src/main/res/drawable/ic_nav_home.xmlapp/src/main/res/drawable/ic_nav_networks.xmlapp/src/main/res/drawable/ic_nav_peers.xmlapp/src/main/res/drawable/ic_nav_settings.xmlapp/src/main/res/drawable/ic_open_in_new.xmlapp/src/main/res/drawable/ic_profile_avatar.xmlapp/src/main/res/drawable/profile_chip_bg.xmlapp/src/main/res/drawable/settings_row_bg.xmlapp/src/main/res/drawable/sheet_row_bg.xmlapp/src/main/res/layout-w960dp/activity_main.xmlapp/src/main/res/layout/activity_main.xmlapp/src/main/res/layout/app_bar_main.xmlapp/src/main/res/layout/content_main.xmlapp/src/main/res/layout/fragment_about.xmlapp/src/main/res/layout/fragment_advanced.xmlapp/src/main/res/layout/fragment_bottom_dialog.xmlapp/src/main/res/layout/fragment_home.xmlapp/src/main/res/layout/fragment_networks.xmlapp/src/main/res/layout/fragment_peers.xmlapp/src/main/res/layout/fragment_profiles.xmlapp/src/main/res/layout/fragment_server.xmlapp/src/main/res/layout/fragment_settings.xmlapp/src/main/res/layout/fragment_troubleshoot.xmlapp/src/main/res/layout/list_item_profile.xmlapp/src/main/res/layout/list_item_profile_picker.xmlapp/src/main/res/layout/list_item_setting.xmlapp/src/main/res/layout/list_item_setting_divider.xmlapp/src/main/res/layout/list_item_setting_section.xmlapp/src/main/res/layout/list_item_setting_toggle.xmlapp/src/main/res/layout/nav_custom_bottom_item.xmlapp/src/main/res/layout/nav_header_main.xmlapp/src/main/res/layout/sheet_profile_picker.xmlapp/src/main/res/layout/sheet_theme_picker.xmlapp/src/main/res/menu/activity_main_drawer.xmlapp/src/main/res/menu/bottom_nav.xmlapp/src/main/res/navigation/mobile_navigation.xmlapp/src/main/res/values-night/colors.xmlapp/src/main/res/values-night/themes.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/dimens.xmlapp/src/main/res/values/strings.xmlapp/src/main/res/values/themes.xmltool/src/main/java/io/netbird/client/tool/ProfileManagerWrapper.java
💤 Files with no reviewable changes (8)
- app/src/main/res/layout/nav_custom_bottom_item.xml
- app/src/main/res/layout/fragment_bottom_dialog.xml
- app/src/main/res/layout/nav_header_main.xml
- app/src/main/java/io/netbird/client/ui/home/PagerAdapter.java
- app/src/main/res/layout/app_bar_main.xml
- app/src/main/res/menu/activity_main_drawer.xml
- app/src/main/res/layout/content_main.xml
- app/src/main/java/io/netbird/client/ui/home/BottomDialogFragment.java
The destination listener was hiding the bottom nav on the first-launch fragment but still falling through to the top-level toolbar logic, so the toolbar stayed visible. Treat firstInstallFragment as a full-screen take-over and bypass the rest of the listener.
ProfilePickerSheet validated the input via sanitizeProfileName() but still wrote the raw user string, so disallowed characters (anything outside [A-Za-z0-9_-]) made it into the engine. Pass the sanitized value to addProfile() and the success/duplicate toasts so what the user sees matches what was stored.
On a TV or stripped-down build there may not be an Activity that handles ACTION_VIEW for an https URL. Catch ActivityNotFoundException and surface a toast instead of crashing the app.
Hardcoded #FFFFFFFF prevents these icons from adapting to theme overlays (e.g. dark mode, focus inversions on TV). Switch to ?attr/colorControlNormal so each icon picks up the correct tint from its host theme. Affects ic_nav_home, ic_nav_peers, ic_nav_networks, ic_nav_settings, ic_add, ic_check, ic_chevron_right, ic_arrow_drop_down, ic_open_in_new — all custom icons introduced in this branch.
Theme picker rows now expose: - contentDescription with the theme label - stateDescription "Selected" on the active row (Android R+) - isSelected=true on the active row for accessibility services After picking, announce "<theme> theme selected" on the sheet root so TalkBack confirms the change before the sheet dismisses.
On large screens the bottom navigation moves to a side rail (layout-w960dp/activity_main.xml). The 80dp bottom padding fragments add to clear the bottom bar is therefore wasted vertical space — set the dimen to 0dp at the same qualifier so layouts pick the right value automatically without per-layout overrides.
A long profile name was wrapping and pushing the picker row taller than the others, breaking the row rhythm. maxLines=1 + ellipsize=end truncates the overflow with "…" so picker rows stay uniform.
list_item_setting_section.xml duplicated the same padding / text size / caps attributes already defined in @style/SettingsSectionHeader, which the section headers inside fragment_settings.xml etc. already apply via the style. Replace the inline attributes with a single style= reference so the section template is the same source of truth.
CodeRabbit suggested catching a typed exception or pre-checking the state file length. The gomobile binding flattens the Go error chain into go.Universe$proxyerror without surfacing the underlying type, and the state-file path is a Go-side constant not exposed to Java, so neither approach is available without a gomobile API change. Expand the comment to make that trade-off explicit so the next reader doesn't try the same refactor.
Squash-merge the four commits from PR #189 (profile-id branch) onto the redesign branch, adapting the profile-id migration to the new bottom-nav UI instead of the old drawer/NavigationView layout: - getActiveProfile() now returns a Profile (with ID) instead of a String; update SettingsFragment and HomeFragment callers to use getName(). - Drop the PR's drawer-specific MainActivity changes (updateProfileMenuItem, drawer onKeyDown) — the redesign replaced the drawer with bottom nav. - Graft the new disable-IPv6 switch listener into AdvancedFragment and add the IPv6 settings row to fragment_advanced.xml in the redesign row style. - Bump netbird submodule to 62afff6 (adds Profile.ID to the gomobile binding).
Replace the Lottie connect button and background mask with a custom pill-shaped SwitchMaterial toggle (white thumb, orange track when connected), and restructure the home layout to match the iOS client: centered profile chip, NetBird logo, status text, hostname with a tappable IP/IPv6 detail section. - Remove Lottie dependency, ButtonAnimation and unused JSON assets - Add Inter and JetBrains Mono fonts; logo from the iOS client - Hostname shown emphasized, IPv4 in the muted summary (with chevron) - Info rows (IPv4 + IPv6) expand on tapping the summary; IPv6 row only shown when an IPv6 address is available; copy-to-clipboard buttons - Append ellipsis to Connecting/Disconnecting status - Make bottom nav unselected items white in night mode - Drop the 'profile created'/'switched to profile' success toasts
* Add profile id migration * Check if ID is set on Profile * Bump netbird * Update profile-id-name branch * Fix active profile errors, bump netbird * Bump netbird to v0.74.0
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/io/netbird/client/ui/profile/ProfilesFragment.java (1)
121-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject names that sanitize to empty
ServiceManager.AddProfile()already strips invalid characters, so this flow still needs the post-sanitize empty-name check fromProfilePickerSheet(or the backend should enforce it). Inputs like!!!become""and get written as.json, creating a blank profile entry.🤖 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/java/io/netbird/client/ui/profile/ProfilesFragment.java` around lines 121 - 135, The add-profile flow in showAddDialog currently only rejects null or raw empty input, but names like !!! can sanitize to an empty string and still be passed to addProfile, creating a blank .json entry. Update the validation in the showAddDialog callback, and mirror the ProfilePickerSheet post-sanitize check, so the sanitized profile name is verified to be non-empty before calling addProfile (or enforce the same rule in ServiceManager.AddProfile).app/src/main/java/io/netbird/client/ui/home/HomeFragment.java (1)
73-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winConnect click path doesn't disable the toggle, unlike disconnect.
The disconnect branch disables
buttonConnectimmediately before callingswitchConnection(false), but the connect branch does not disable it beforeswitchConnection(true). A rapid double-tap on "connect" can triggerswitchConnection(true)twice beforeonConnecting()arrives and disables the button.🐛 Proposed fix
} else { // We're currently disconnected, so connect + buttonConnect.setEnabled(false); setStatusText(R.string.main_status_connecting); serviceAccessor.switchConnection(true); }🤖 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/java/io/netbird/client/ui/home/HomeFragment.java` around lines 73 - 88, The connect click path in HomeFragment’s buttonConnect listener is missing the immediate disable that the disconnect path already uses, which allows repeated taps to call serviceAccessor.switchConnection(true) multiple times before onConnecting() disables the control. Update the else branch in the buttonConnect onClickListener to disable buttonConnect before setting the connecting status and invoking switchConnection(true), matching the disconnect flow and preventing double-triggered connection attempts.
🧹 Nitpick comments (1)
tool/src/main/java/io/netbird/client/tool/EngineRunner.java (1)
86-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfirm
ProfileoverridestoString()for this log line.
activeProfileis now aProfileobject (previously aString), and it's logged directly via"Initializing engine with profile: " + activeProfile. IfProfiledoesn't overridetoString(), this will log an unhelpfulProfile@hashcodeinstead of the profile name/id, degrading this debug log's usefulness.♻️ Proposed fix
- Profile activeProfile = profileManager.getActiveProfile(); - Log.d(LOGTAG, "Initializing engine with profile: " + activeProfile); + Profile activeProfile = profileManager.getActiveProfile(); + Log.d(LOGTAG, "Initializing engine with profile: " + activeProfile.getName() + " (" + activeProfile.getId() + ")");🤖 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 `@tool/src/main/java/io/netbird/client/tool/EngineRunner.java` around lines 86 - 89, The EngineRunner log line now concatenates the activeProfile Profile object directly, so verify Profile overrides toString() to return a meaningful name or id. If it does not, update Profile’s toString() or change the log in EngineRunner to explicitly log the desired profile field via getActiveProfile() so the "Initializing engine with profile" message stays readable and useful.
🤖 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/src/main/java/io/netbird/client/ui/home/HomeFragment.java`:
- Around line 159-176: The status/button update paths in HomeFragment are
re-reading fragment view fields across the post() thread hop, which can become
null after the initial check. In setStatusText(), setToggle(), and the
onAddressChanged handling, capture textConnStatus, buttonConnect, and binding
into local variables first, null-check those locals, and use only the locals
inside the posted Runnable so onDestroyView() cannot null them between checks
and use.
In `@tool/src/main/java/io/netbird/client/tool/ProfileManagerWrapper.java`:
- Around line 77-86: Profile switching is still using the profile name in the
caller while ProfileManagerWrapper.switchProfile now expects an ID. Update the
Home profile chip flow that invokes switchProfile so it passes profile.getID()
instead of the display name, matching the wrapper’s ID-based routing and keeping
the behavior consistent with ProfilePickerSheet.
---
Outside diff comments:
In `@app/src/main/java/io/netbird/client/ui/home/HomeFragment.java`:
- Around line 73-88: The connect click path in HomeFragment’s buttonConnect
listener is missing the immediate disable that the disconnect path already uses,
which allows repeated taps to call serviceAccessor.switchConnection(true)
multiple times before onConnecting() disables the control. Update the else
branch in the buttonConnect onClickListener to disable buttonConnect before
setting the connecting status and invoking switchConnection(true), matching the
disconnect flow and preventing double-triggered connection attempts.
In `@app/src/main/java/io/netbird/client/ui/profile/ProfilesFragment.java`:
- Around line 121-135: The add-profile flow in showAddDialog currently only
rejects null or raw empty input, but names like !!! can sanitize to an empty
string and still be passed to addProfile, creating a blank .json entry. Update
the validation in the showAddDialog callback, and mirror the ProfilePickerSheet
post-sanitize check, so the sanitized profile name is verified to be non-empty
before calling addProfile (or enforce the same rule in
ServiceManager.AddProfile).
---
Nitpick comments:
In `@tool/src/main/java/io/netbird/client/tool/EngineRunner.java`:
- Around line 86-89: The EngineRunner log line now concatenates the
activeProfile Profile object directly, so verify Profile overrides toString() to
return a meaningful name or id. If it does not, update Profile’s toString() or
change the log in EngineRunner to explicitly log the desired profile field via
getActiveProfile() so the "Initializing engine with profile" message stays
readable and useful.
🪄 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: 3acf034a-011b-4f62-a07e-db1353f256a2
⛔ Files ignored due to path filters (6)
app/src/main/res/drawable-night-xxhdpi/bg_bottom.pngis excluded by!**/*.pngapp/src/main/res/drawable-night/nb_logo_full.pngis excluded by!**/*.pngapp/src/main/res/drawable-xxhdpi/bg_bottom.pngis excluded by!**/*.pngapp/src/main/res/drawable/nb_logo_full.pngis excluded by!**/*.pngapp/src/main/res/font/inter_variable.ttfis excluded by!**/*.ttfapp/src/main/res/font/jetbrains_mono_variable.ttfis excluded by!**/*.ttf
📒 Files selected for processing (52)
Readme.mdapp/build.gradle.ktsapp/src/main/assets/button_full.jsonapp/src/main/assets/button_full_dark.jsonapp/src/main/assets/loading.jsonapp/src/main/java/io/netbird/client/MainActivity.javaapp/src/main/java/io/netbird/client/ui/advanced/AdvancedFragment.javaapp/src/main/java/io/netbird/client/ui/advanced/ThemePickerSheet.javaapp/src/main/java/io/netbird/client/ui/home/ButtonAnimation.javaapp/src/main/java/io/netbird/client/ui/home/HomeFragment.javaapp/src/main/java/io/netbird/client/ui/home/Peer.javaapp/src/main/java/io/netbird/client/ui/home/PeersAdapter.javaapp/src/main/java/io/netbird/client/ui/home/PeersFragmentViewModel.javaapp/src/main/java/io/netbird/client/ui/home/ProfilePickerSheet.javaapp/src/main/java/io/netbird/client/ui/home/Resource.javaapp/src/main/java/io/netbird/client/ui/profile/ProfilesFragment.javaapp/src/main/java/io/netbird/client/ui/settings/SettingsFragment.javaapp/src/main/res/color/connect_thumb_tint.xmlapp/src/main/res/color/connect_track_tint.xmlapp/src/main/res/color/tab_icon_color.xmlapp/src/main/res/color/tab_text_color.xmlapp/src/main/res/drawable/connect_switch_thumb.xmlapp/src/main/res/drawable/connect_switch_track.xmlapp/src/main/res/drawable/ic_add.xmlapp/src/main/res/drawable/ic_arrow_drop_down.xmlapp/src/main/res/drawable/ic_check.xmlapp/src/main/res/drawable/ic_chevron_right.xmlapp/src/main/res/drawable/ic_content_copy.xmlapp/src/main/res/drawable/ic_nav_home.xmlapp/src/main/res/drawable/ic_nav_networks.xmlapp/src/main/res/drawable/ic_nav_peers.xmlapp/src/main/res/drawable/ic_nav_settings.xmlapp/src/main/res/drawable/ic_open_in_new.xmlapp/src/main/res/drawable/info_row_bg.xmlapp/src/main/res/layout/fragment_advanced.xmlapp/src/main/res/layout/fragment_home.xmlapp/src/main/res/layout/list_item_profile_picker.xmlapp/src/main/res/layout/list_item_setting_section.xmlapp/src/main/res/menu/peer_clipboard_menu.xmlapp/src/main/res/values-night/colors.xmlapp/src/main/res/values-w960dp/dimens.xmlapp/src/main/res/values/colors.xmlapp/src/main/res/values/strings.xmlgradle/libs.versions.tomlnetbirdtool/src/main/java/io/netbird/client/tool/EngineRunner.javatool/src/main/java/io/netbird/client/tool/IFace.javatool/src/main/java/io/netbird/client/tool/NetworkChangeNotifier.javatool/src/main/java/io/netbird/client/tool/Profile.javatool/src/main/java/io/netbird/client/tool/ProfileManagerWrapper.javatool/src/main/java/io/netbird/client/tool/TUNParameters.javatool/src/main/java/io/netbird/client/tool/VPNService.java
💤 Files with no reviewable changes (6)
- app/src/main/assets/loading.json
- app/src/main/java/io/netbird/client/ui/home/ButtonAnimation.java
- app/src/main/assets/button_full.json
- app/src/main/assets/button_full_dark.json
- app/build.gradle.kts
- gradle/libs.versions.toml
✅ Files skipped from review due to trivial changes (14)
- app/src/main/res/color/connect_thumb_tint.xml
- app/src/main/res/drawable/ic_chevron_right.xml
- app/src/main/res/values-w960dp/dimens.xml
- app/src/main/res/drawable/ic_check.xml
- app/src/main/res/drawable/ic_nav_settings.xml
- app/src/main/res/color/connect_track_tint.xml
- app/src/main/res/drawable/ic_nav_home.xml
- app/src/main/res/layout/list_item_setting_section.xml
- app/src/main/res/color/tab_text_color.xml
- app/src/main/res/drawable/ic_add.xml
- app/src/main/res/layout/list_item_profile_picker.xml
- Readme.md
- app/src/main/res/drawable/ic_nav_peers.xml
- app/src/main/res/values-night/colors.xml
🚧 Files skipped from review as they are similar to previous changes (9)
- app/src/main/res/drawable/ic_nav_networks.xml
- app/src/main/res/drawable/ic_arrow_drop_down.xml
- app/src/main/java/io/netbird/client/ui/home/ProfilePickerSheet.java
- app/src/main/java/io/netbird/client/ui/settings/SettingsFragment.java
- app/src/main/res/layout/fragment_advanced.xml
- app/src/main/java/io/netbird/client/ui/advanced/ThemePickerSheet.java
- app/src/main/java/io/netbird/client/ui/advanced/AdvancedFragment.java
- app/src/main/res/values/strings.xml
- app/src/main/java/io/netbird/client/MainActivity.java
Resolve conflicts in favor of the iOS-style bottom-nav redesign: - MainActivity: keep bottom-nav AppBarConfiguration; drop drawer setup, updateProfileMenuItem and drawer-based onKeyDown (drawer no longer exists) - AdvancedFragment: keep compact row-click listeners; drop redundant main block and a duplicate layoutDisableIpv6 listener - fragment_advanced.xml: keep settings_row_bg IPv6/firewall rows; drop main's ConstraintLayout variant that duplicated @+id/layout_force_relay_connection - ProfilesFragment: keep no success toast on profile switch - ProfileManagerWrapper: take main's getActiveProfile (getIsActive + throw); fresh-install empty state is now handled inside the engine - netbird submodule: advance to current main pointer (3aa6c02) Note: gomobile netbird.aar must be regenerated from the new submodule so the Profile.getIsActive() binding is available.
- Rework the connect toggle thumb as a centred layer so the white circle size is independent of the track height - Give the disconnected track a light grey fill with a thin darker border and disable Material auto-tint so the custom per-state colours render - Make the bottom nav background white and drive item icon/text colour off state_checked; use opaque icon fills so itemIconTint applies at full strength - Lighten the home background
- Give the connect toggle thumb a soft drop shadow (baked into the drawable; drop thumbTint so the shadow is not tinted white) - Merge the two info rows (IP / secondary value) into a single bordered box with a middle divider; showDividers hides the separator when the secondary row is gone
Revert to a plain white circle for the connect toggle thumb.
The recent white-surface tweaks hardcoded @color/white and #4A4A4A, which have no night variant, breaking dark mode (white bottom nav, white profile chip, invisible unselected icons). Introduce theme-aware semantic colours instead: - nb_bottom_nav_bg / nb_chip_bg: white in light, dark surface in night - tab_icon_unselected: #4A4A4A in light, white in night
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/res/drawable/connect_switch_track.xml (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting repeated dimensions.
88dp/44dp/100dpare duplicated across both selector items. Minor, purely stylistic.Also applies to: 19-20
🤖 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/drawable/connect_switch_track.xml` around lines 10 - 11, The switch track drawable repeats the same size and corner radius values in both selector items; extract the shared 88dp, 44dp, and 100dp measurements into reusable dimension resources and reference them from the shape definitions in the connect_switch_track drawable so both items stay consistent and easier to maintain.
🤖 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.
Nitpick comments:
In `@app/src/main/res/drawable/connect_switch_track.xml`:
- Around line 10-11: The switch track drawable repeats the same size and corner
radius values in both selector items; extract the shared 88dp, 44dp, and 100dp
measurements into reusable dimension resources and reference them from the shape
definitions in the connect_switch_track drawable so both items stay consistent
and easier to maintain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09e5f192-5e9b-4894-aa5d-ddc9fa50b5ac
📒 Files selected for processing (15)
app/src/main/res/color/connect_track_tint.xmlapp/src/main/res/color/tab_icon_color.xmlapp/src/main/res/color/tab_text_color.xmlapp/src/main/res/drawable/connect_switch_thumb.xmlapp/src/main/res/drawable/connect_switch_track.xmlapp/src/main/res/drawable/ic_nav_home.xmlapp/src/main/res/drawable/ic_nav_networks.xmlapp/src/main/res/drawable/ic_nav_peers.xmlapp/src/main/res/drawable/ic_nav_settings.xmlapp/src/main/res/drawable/info_row_divider.xmlapp/src/main/res/drawable/profile_chip_bg.xmlapp/src/main/res/layout/activity_main.xmlapp/src/main/res/layout/fragment_home.xmlapp/src/main/res/values-night/colors.xmlapp/src/main/res/values/colors.xml
✅ Files skipped from review due to trivial changes (7)
- app/src/main/res/drawable/connect_switch_thumb.xml
- app/src/main/res/color/tab_text_color.xml
- app/src/main/res/drawable/ic_nav_networks.xml
- app/src/main/res/drawable/profile_chip_bg.xml
- app/src/main/res/drawable/info_row_divider.xml
- app/src/main/res/color/connect_track_tint.xml
- app/src/main/res/values-night/colors.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/main/res/color/tab_icon_color.xml
- app/src/main/res/drawable/ic_nav_settings.xml
- app/src/main/res/layout/activity_main.xml
- app/src/main/res/layout/fragment_home.xml
- app/src/main/res/values/colors.xml
- Reduce row height (52->40dp), text (15->12sp) and copy buttons (40->34dp) - Middle-ellipsize the IPv6 row so a long address is truncated in the centre while the copy button still copies the full value
Use opaque fillColor so app:tint renders the profile picker + and check icons at full nb_orange intensity, matching the manage icon.
Replace the default circular ripple mask on bottom nav / navigation rail items with a rounded-rect (8dp) mask.
Navigating back to home re-inflates the fragment, so the hardcoded android:text default painted "Disconnected" before the real state arrived — and the state update was deferred by two nested post() calls even though registration replays it on the main thread. Move the layout default to tools:text, and apply view updates inline when already on the main thread (engine callbacks, which arrive off the main thread, still post). Snap the toggle thumb after setChecked so it doesn't animate into place on a fresh view.
The sheet listed every profile in a wrap_content RecyclerView, so past a handful of profiles the list grew past the screen and pushed the "Add profile" and "Manage profiles" rows out of reach. Cap the list height, show only the five most recently used profiles, and surface a "Show all profiles (N)" row into the manage screen when there are more. Expand the sheet on open so a long list no longer parks at peek height. Recency is tracked in SharedPreferences since the gomobile Profile has no last-used field. The store prunes entries for profiles that no longer exist on every read, so deleting a profile needs no bookkeeping from the caller and the store cannot outgrow the profile count. Also fix the sheet passing a profile name where switchProfile expects an ID, which made switching from the sheet fail whenever the two differed.
Collaborator
Author
Thank you! The first point has been fixed. On the second I need to figoure out what would be the best. The message is valid because it inidicate the Android background service is running but you are right it is not clear for the customer. |
The first-launch flag was cleared the moment the onboarding screen was shown, so killing the app before picking cloud or self-hosted silently left the user on the cloud default with no way back to the chooser. Clear the flag in FirstInstallFragment once the user commits to a server instead, and make back close the app rather than dismiss the screen, so there is no path that leaves the choice unmade but unaskable. Guard the navigation on a null savedInstanceState now that the flag stays set for the duration of the screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dual-stack exit nodes are now reported by the daemon as a single "0.0.0.0/0, ::/0" field instead of a bare prefix. isExitNodeAddress() only did exact string equality, so it missed these nodes: the exit node picker showed nothing for them, and the resource list failed to filter them out, leaking exit nodes into the plain resource list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* test: port the e2e suite to the redesigned UI The iOS-style redesign folded the old add-profile / switch / change-server flow into a single profile editor dialog, so LoginFlow now drives that: name + setup key in one submit, with the key field revealed via its row-owned switch. Enrolment success is verified positively — the profile must appear in the list — since the dialog also dismisses on paths that never registered the peer. Creating a profile does not activate it, so the flow switches to it explicitly before connecting. All navigation now goes through taps (bottom nav and settings rows) rather than NavController, keeping the app in the state a user would produce. The force-relay setup step was flaky because scrollForward() flings and a tap injected while the list is still settling is consumed as a scroll-stop. Scroll, settle, click and verify now retry as one unit. Also: EditText writes are read back (soft-keyboard layout shifts silently swallowed setText), autofilled setup-key fields are cleared first, screenshots moved to the app's external files dir (scoped storage made /sdcard/Pictures fail with EACCES), and the first-run screen is dismissed before the suite starts since it shares view ids with the profile editor. VpnTestHarness listens through StateListenerAdapter so the interface can grow without breaking the suite. * Bump netbird submodule: merge main into android/gui-integration Brings in #7022, which stops NewAuth from rebuilding the config from scratch on setup-key enrolment — previously that dropped every stored field, blanking the profile's display name and regenerating the WireGuard identity. The e2e suite's positive login check depends on it. * test: drive the VPN connect from the Home screen toggle connectAndAwait started the engine through MainActivity.switchConnection() and waited on a StateListener — a code path no user takes. Tap the Home screen's connect toggle instead and wait for the on-screen status text to read Connected, so both the trigger and the feedback are the exact UI a user sees. The suite runs on an English locale, so the literal status text is matched. * test: log the full ping output on every attempt A reachability failure only reported that the peer never answered; the evidence — did the name resolve, to what address, or did the reply just not come back — was discarded. Log the complete ping output (and the resolve probe's) so the CI logcat artifact shows exactly which of those it was. * test: log the in-process resolver's verdict when a ping fails The CI artifact showed every failing ping with empty stdout — resolution errors go to stderr, which executeShellCommand does not capture — and the tunnel's DNS trail shows only search-domain expansions being queried, never the bare peer name. What is still missing is the resolver's actual error and whether the app's own resolver (guaranteed to route through the VPN, unlike the shell-uid ping) agrees. Log InetAddress.getByName() for the target on every failed attempt so the next run's artifact answers both. * test: give the data-plane probes 90s to settle Three CI runs put the evidence together: the tunnel comes up fast, but its data plane does not. The relay-backed peer resolves and answers 8-12s after Connected (attempt 1 fails, attempt 3 pings), and the exit-node profile SERVFAILs upstream DNS until the exit-node path is ready — every failure was one of these still settling when the 20s window closed, with a different test tripping first each run. The Robot suite allows ~3 minutes for the same sequence; 90s on the ping/resolve/port/egress windows keeps failures reasonably fast while clearing the observed settling time with margin. Connect stays at 20s — the UI reports Connected within seconds. * Bump netbird submodule to main The android/gui-integration work is squash-merged into main — the only code difference left was main's relay reconnect backoff randomization (#7067), which the branch did not have. * test: drop the setup-key env fallback from the build script The CI script passes both keys as -P instrumentation arguments, so reading them from the environment never took effect. * ci: install the JDK from temurin instead of adopt setup-java fails to resolve the 'adopt' distribution — AdoptOpenJDK was renamed to Eclipse Temurin and the old endpoints are going away — so the build died before installing a JDK. Same JDK, current name. * ci: exclude the e2e package from the instrumented-tests job The e2e tests need a setup key for a live account, held only by the private mobile-e2e repo, so every one of them failed here with "setupKey instrumentation argument is required". They are meant to run from that repo's workflow, not on every push.
Targeting API 36 dropped android:statusBarColor from both themes on the assumption that the bar would then show windowBackground. An API 36 emulator behaves that way, but a real device does not: the window keeps FORCE_DRAW_STATUS_BAR_BACKGROUND set, so the system paints the strip itself and with no statusBarColor it fills it with the default black. The app meanwhile asks for dark icons to suit the light background, and dark-on-black left the clock and battery invisible. Set statusBarColor to nb_bg for the platforms where the attribute still applies, and give the root layout an nb_bg background so the strip is painted by the view hierarchy rather than depending on the system where it does not. Re-tint the icons from onConfigurationChanged as well, since switching theme in settings re-runs configuration but not onCreate.
The navigation bar background never had a colour of its own, so the system filled the strip with its own default. That follows the device theme, not the in-app one, leaving a white band under the tabs whenever the app was switched to dark while the phone stayed light. Setting navigationBarColor alone is not enough, because the root's fitsSystemWindows padded all four edges at once: the bottom navigation stopped above the strip and the root's nb_bg showed through it, which does not match the nb_bottom_nav_bg tabs above. Apply the insets from code instead so the root only takes the top and side ones, and the bottom navigation pads itself and carries its background down to the screen edge.
The focused state drew a 3dp stroke in pure white at night and in near black by day. At that weight it reads as a box around the field rather than a highlight, and it pulls attention away from the text being typed. A 1dp accent stroke still marks focus clearly against the 1dp grey of the unfocused state, without the jump in weight. The single-field dialog layout also placed its input 8dp below the label, which is close enough that the two run together once the input carries a border of its own. Match the 16dp the label already uses. Affects every screen sharing these resources: the advanced and first install fragments, the profile editor and the simple edit text dialog.
The info rows were the tail of the packed vertical chain that is centered between the profile chip and the exit node row. Opening them grew the chain, so its center moved and the whole logo/switch/status group slid upwards instead of only the rows appearing. End the chain at the address summary and anchor the info rows below it with no bottom constraint, so they expand downwards on their own.
Without a bottom constraint the expanded rows could reach into the exit node row on a short screen or at a large font scale. Constrain them against that row and let constrainedHeight cap them instead. Bias 0 keeps them pinned under the address summary: a wrap_content view held between two anchors is centered in the leftover gap by default, which would detach the card from the summary it belongs to. The chain above stays unaffected, since none of its links point at this view.
The toggle is disabled while a transition runs, but nothing painted that: the track selector has no state_enabled branch, the thumb is a flat white layer-list, and useMaterialThemeColors is off, so the Material disabled tint never applies either. The switch looked identical whether or not it accepted a tap. Fade and pulse it while disabled, as the desktop client does. Desktop combines disabled:opacity-50 with animate-pulse, and the animation's opacity keyframes win over the flat value, so a transition there reads as a two-second pulse between full and half opacity rather than a static fade. Animating alpha instead of swapping colours also keeps the orange connecting fill visible through it. The force-cancel window re-enables the toggle without going through setToggle(), so it has to clear the pulse itself; an infinite animator outlives its view, so onDestroyView cancels it before dropping the reference.
Contributor
|
Since this PR makes quite a few UI changes, we should also change the screenshots in the README. |
The centered block (logo, connect toggle, status, hostname, address summary) is a packed chain anchored between the profile chip and the exit node row, so it moved whenever either end of that span changed height. Two things changed it: The hostname was empty and the address summary GONE while disconnected, making the chain shorter and centering it lower. Both now hold their slot when empty, so filling them in on connect no longer grows the group. An INVISIBLE view still takes taps and focus, so the summary's clickable and focusable track whether there is an address to expand. The exit node row was anchored to the top of the session expiry row, so that row appearing pushed the exit node row up and dragged the chain with it. The exit node row is now pinned to the parent bottom and the session row stacks above it, growing upwards into free space instead of pushing. A barrier over both rows gives the expandable info rows a cap that follows whichever of them is currently topmost.
Collaborator
Author
Done |
Collaborator
Author
All done |
… distribution (#236) * Add manual workflow to build a version-code-consistent APK for ad-hoc distribution Adds prepare-publication.yml (workflow_dispatch) that reuses the release signing key and computes version_code as the combined run count of itself and build-release.yml, so the two never collide. A shared concurrency group on both workflows prevents a race on that computation. * Draw the release version code from the shared run counter build-release.yml derived its version code from github.run_number, which counts only its own runs and is blind to prepare-publication.yml. With 25 release runs so far, the first ad-hoc build would take 25+1+40=66 and the next release would take 26+40=66 as well, then fall behind: 67 after 68 was already published. The concurrency group cannot fix this, because run_number is assigned when the run is queued, not when the step executes. Both workflows now sum the same two counters, so every run of either one advances the code by exactly one. Reading those counts needs actions: read, which an explicit permissions block otherwise withholds. * Name the manual workflow build-snapshot prepare-publication named a step in a process, while the workflows beside it name what they produce: build-debug, build-release. What this one produces is a release-signed build from an arbitrary commit with no tag behind it, which is what snapshot means. Not build-rc: release candidates already exist here as published pre-release tags (v0.6.0-rc.1, v0.3.3-rc.2) and are built by build-release.yml, so the name would claim a meaning the repository has already given away. The version code counter is keyed by workflow file name, so the rename is free only while the workflow has no runs yet. * Label snapshot builds snapshot- rather than ci- The version name travels to the management server as the peer's ui_version and is what the about screen shows, so it is the only thing telling support which build a peer is running. build-debug.yml already emits ci-<sha> from the same expression, which left an unsigned PR build and a release-signed hand-out looking identical in the peer list. The artifact keeps just the version name; prefixing it again read as snapshot-artifacts-snapshot-<sha>. * Resolve the Go version from the submodule's release tags in CI CI builds used the version only when the submodule sat exactly on a tag and fell back to ci-<sha> otherwise, which the management server rejects in NBVersionCheck posture checks: it treats ci- as a development build everywhere except there. Since the submodule is bumped more often than it is tagged, release builds effectively always shipped as ci-<sha>. CI now resolves the version by walking the pinned commit's ancestry back to the last stable release tag and appending the commit as SemVer build metadata, e.g. 0.77.0+f06b8c762. The server strips build metadata before every comparison, so this passes the same gates as a plain 0.77.0 while still naming the exact commit in the dashboard. Pre-release tags are skipped as a base: a suffix like -rc.2 lands in SemVer pre-release position, which the server compares differently from a release. Local builds now always produce dev-<sha>, which skips every server-side version gate; a developer who needs a real version passes it as the argument. The ancestry walk needs full history, but actions/checkout clones submodules shallow — the tags arrive without the commits between HEAD and the tag, and the walk would silently come up empty. The composite action therefore unshallows the submodule before building, guarded because --unshallow on a complete repository is a hard error. * Document the three build workflows and their differences * Fail the build when a run count cannot be fetched The zero fallback existed for one legitimate case: the runs endpoint returns 404 until a workflow has run or reached the default branch, and treating that as zero is what lets build-release compute a code before build-snapshot's first run. But it also swallowed every other failure — a network error or a revoked token minted a version code far below the published ones, silently for hand-distributed snapshots. Keep the 404-means-zero case and abort on everything else, including a non-numeric response, which bash arithmetic would otherwise fold to zero.
* Pin gobind alongside gomobile instead of running gomobile init gomobile bind shells out to gobind, and gobind is the tool that actually generates the Java bindings and the JNI glue. CI cached and installed only gomobile and left the build script to call `gomobile init`, which installs gobind from @latest: the driver was pinned while the generator floated, so the generated API could change without a commit here. Take the revision from the go.mod the submodule already carries, so the two tools cannot drift apart and the cache key follows a submodule bump on its own, and have the build script check that the pair is present rather than reaching for gomobile init. The cache key names both tools now. Entries saved under the old key hold no gobind, so keeping it would hit the cache, skip the install and leave the build without a generator. * Install gomobile and gobind automatically at the go.mod pinned revision
The hostname sized itself, so a long one wrapped onto a second line and ran out to both edges of the screen. It now takes the width it is given, inside the same 20dp margin the rows below it keep, and truncates with an ellipsis on a single line. A short hostname still sits centred.
The address rows opened into whatever gap was left between the summary and the cards below, and constrainedHeight let that gap win: on this screen the rows came out squeezed into 42dp of a needed 112, the second one clipped away entirely. The session expiry card decides it — the same screen fits the rows when no session is about to expire and crushes them when one is. Raising the packed block helps but cannot settle it: even pinned to the top there is less room than the rows need, and how much is missing changes with what the cards below are showing. So the rows now open inline when they fit and as a floating panel when they do not, measured rather than assumed, since the system font size and the presence of an IPv6 address both move the number. The panel is drawn over the cards, so nothing on the screen has to move or be taken away to make room for it.
The foreground notification showed the deadline as a clock time, unlike
the home screen banner and the desktop tray. Reuse the banner's wording:
the largest non-zero unit rounded up ("Session expires in 2 hours"),
with a sub-minute "less than a minute" tail, using the banner's
translations verbatim in every locale. A silent re-post keeps the label
current, ticking faster as the deadline nears, mirroring the desktop
tray's refresh intervals.
* Add in-app SSH terminal with WebView and persistent sessions - WebView-hosted xterm.js (5.5.0 + fit-addon) terminal rendered via app/src/main/assets/terminal/. The Go gomobile SSHClient streams PTY output to Java which evaluateJavascripts base64 chunks into xterm. - Auto-detection of server type via NetBird SSH banner: NetBird-JWT triggers the existing Custom-Tabs URL opener for the OAuth 2.0 device-code flow, NetBird-no-JWT uses the NetBird private key, and a regular OpenSSH server falls back to NetBird key then optional password. One unified Connect() in Go covers all three. - Persistent sessions: SshSessionManager (application-scoped singleton) owns SSHClients with a 256 KB scrollback buffer per session, so fragments can detach (e.g. on backgrounding) and re-attach later with the scrollback replayed before live output resumes. - New "SSH" drawer entry → SshSessionsFragment lists active sessions with state indicators and a FAB to open the connect dialog for a free-form host. Peer long-press → SSH continues to work with the IP prefilled. - Connect dialog asks only for host (when not prefilled), username, port, and an optional password used by regular SSH fallback. - ActionBar auto-hides on the terminal destination for maximum screen area; BottomSheetDialogFragments auto-dismiss when navigating away from home so the terminal is not covered. Requires the matching netbird submodule on the android-client-ssh branch which adds the SSHClient gomobile binding. * Add an SSH button to each peer row The long-press menu already offered SSH, but nothing hinted at it, and a row tap opens the peer detail so it could not carry the action either. A dedicated button makes it reachable in one tap. Shown whatever the peer's status: the engine dials on demand, so an idle peer still accepts a connection. The long-press entry stays, and loses its connected-only condition for the same reason. * Drop the password field from the SSH connect dialog The NetBird auth paths never use a password, and a regular server is tried with the NetBird key first, so asking up front was wrong more often than not. The terminal now prompts only once the server has actually refused everything else. The default port follows where the connection starts from: a prefilled host is a NetBird peer on 22022, one typed by hand is an ordinary server on 22. The nav argument and the parse fallback follow suit. Enter submits, so the fields carry IME actions. setSingleLine has to precede setInputType, since it resets the type. * Prompt for the SSH password in the terminal and allow reconnecting Adds a NEEDS_PASSWORD state, which is a pause rather than a failure: the session waits for the terminal to collect a password and retries, as often as the server keeps refusing, matching what a normal ssh client allows. Cancelling ends the session instead of parking it with no way forward. A finished session can be redialled in place from a bar below the terminal, reusing the session so its scrollback stays readable. The screen is cleared only on the very first connect, so the connect chatter does not sit above the prompt while earlier output survives a reconnect. CONNECTING now prints a notice: a reconnect does not go through the create path, so it had none. * Persist the SSH session list per profile The list only lived in memory, so it was lost on restart. Connection details now go to SharedPreferences and come back as closed sessions that reconnect on demand; a live connection cannot outlive the process. Passwords are never stored, and a restored entry prompts again. Keyed by profile, because an overlay IP means a different host under a different profile, so one list must not leak into another. Switching closes whatever is live, since the tunnel goes down with the old profile. Lists belonging to deleted profiles are discarded by comparing against the live profile IDs, as deletion happens elsewhere and reports nothing. * Give the SSH session list disconnect, reconnect and dark-theme fixes Tapping a finished session reconnects when it left no output behind, and otherwise just opens it, letting the terminal's own bar offer the redial once there is something to read. A disconnect button ends a live session while keeping it listed, distinct from closing it, which also discards the scrollback and so asks for confirmation first. The row's text was constrained to the close button rather than the one beside it, so the label overlapped and hid it. The night theme inherits a Light parent, leaving colorControlNormal and the default text colour dark, so the icons and labels were invisible. Both now use the app's own theme-aware colours, which meant replacing the framework close icon with a tintable one. * Bump netbird submodule for the SSH password and error changes * Allow duplicating an SSH session to the same host Sessions were already keyed by a unique id rather than by host, so parallel connections to one target worked; what was missing was a way to ask for one, and a way to tell the results apart. Long-pressing a row now offers Duplicate, which opens a second session to the same target and connects it. The password is not carried over: it belongs to the session that was asked for it, so a server wanting one prompts again. Sessions sharing a target are numbered, the number leading the label as tmux does, since the target is long enough to be truncated on a narrow row and that would drop the part that disambiguates. A target with a single session stays unnumbered. The label also gains the ellipsize and maxLines the peer rows already use, so a long FQDN cannot wrap and make rows uneven. * Match the SSH session FAB to the profiles page design The SSH sessions floating action button used the default Material tint and the framework ic_input_add icon, so it looked out of place next to the flat orange FAB on the profiles page. Give it the same drawable, background tint, white icon and zero elevation, switch the fixed 16dp margin to fab_margin so it insets on landscape and tablet layouts, and reuse fab_content_inset for the list's bottom padding. * Update the terminal to xterm.js 6.0 Replaces the bundled xterm.js 5.5.0 and addon-fit 0.10.0 with 6.0.0 and 0.11.0, taken from the npm tarballs rather than a CDN so the files carry no third-party minification. This matches the version the iOS client already ships, so the two platforms no longer drift apart. Every API index.html relies on is unchanged in 6.0, so the only fix the upgrade needs is for the scrollbar: 6.0 renders its own scrollbar element instead of using the native one, which the existing ::-webkit-scrollbar rule no longer reaches. * Configure the SSH terminal and fix its keyboard handling Puts the xterm options that were left at their defaults to use: the full 16 colour ANSI palette, since without one the server's colours fall back to the WebView defaults and are close to unreadable on black, a contrast floor for the pairings that stay illegible anyway, and allowProposedApi so the buffer and parser APIs are reachable. Font size goes down rather than up: every point costs about four columns, and wrapped lines cost more than small glyphs. Loads the WebGL renderer as well, dropping it on context loss so a backgrounded app falls back to the DOM renderer instead of showing a blank terminal. Grows the key bar to cover what a phone keyboard makes expensive: ^C, ^D and ^Z as single keys, because arming Ctrl needs the soft keyboard to then deliver a letter and it does not always do so; the punctuation that sits behind a symbol page; and copy and paste, which the terminal had no way to reach at all. Sticky Ctrl and Alt stay for every other combination. The keyboard used to cover the terminal outright. The manifest asks for adjustPan, which slides the window up and carries the key bar off screen, so the fragment switches to adjustResize while it is visible. From API 35 that mode is ignored and the keyboard simply draws over the window, so the IME inset is padded instead. Either way the WebView ends up shorter, which needs .xterm to track its container height, or the row count never shrinks. Also gives the password prompt the dialog theme the rest of the app uses. It was building a bare AlertDialog, so the theme's global text colour made the title white on white; that theme deliberately leaves the window transparent and expects the shared rounded layout to supply the body. * Remember the SSH username instead of defaulting to one A developer's own login name was baked into four places: the connect dialog's fallback, the terminal fragment's argument default, a string resource and the navigation graph. Anyone else got that name silently substituted whenever the field was left empty, which fails authentication against a remote account that does not exist. There is no sensible default to replace it with, since the login name is the remote account. The dialog now prefills whatever was last connected with, empty on a fresh install, and stores it again on connect. The key is not per profile: the name belongs to whoever holds the phone, and the same account is usually used whichever profile is active. Connecting with an empty host or username now marks the field and leaves the dialog open rather than dismissing it, which is what the substituted default used to paper over. * Bump netbird submodule to dismiss the SSH auth browser Picks up the SSH JWT flow calling OnLoginSuccess once it has a token, so the Custom Tab opened for device-code auth closes itself instead of staying in front of the terminal. No app-side change is needed: the SSH URL opener is the same CustomTabURLOpener the login flow uses, and its onLoginSuccess already brings the activity forward. MainActivity is singleTask, so that returns to the existing instance and the terminal fragment is still on the stack. * Run the SSO Custom Tab calls on the main thread Bumps the submodule for the SSH JWT flow calling its URL opener in turn rather than from two racing goroutines, which is what left the browser in front of the terminal after the token had arrived. Calling in turn exposed two problems here that the goroutines had been hiding. launch() and startActivity() drive activity machinery and have to run on the main thread, so a synchronous call from a Go thread would raise a wrong-thread error; both are posted now. isOpened is set before that post rather than inside it, because the caller may report success straight after and onLoginSuccess does nothing unless the surface is already marked as opened, and it is volatile since the two threads share it. onLoginSuccess deliberately leaves isOpened set: MainActivity.onStop reads it to keep the service bound while the SSO surface is in front, and the launcher callback clears it when the tab actually goes away. * Let a stored SSH session be edited A session saved with the wrong address or login name could only be closed and recreated from scratch. Long-pressing an entry now offers Edit alongside Duplicate, prefilled with the session's own details. The details are final on a session, so the entry is rebuilt rather than mutated: the old one is closed and replaced under the same id. That keeps its place in the list, since a LinkedHashMap put on an existing key holds the original position, and overwrites the stored entry instead of appending a second one. The scrollback goes with it, having come from a different host. Editing leaves the session disconnected on purpose. Redialling here would connect before the user has seen whether the new details are right, and the list already offers a reconnect. The host field is always shown in the editor, including for a peer session where connecting hides it, because correcting the address is half of what the editor is for. * Allow rotation while the SSH terminal is open MainActivity locks portrait on phones, which suits every screen it has. A terminal is the exception: landscape roughly doubles the column count, which is what long command lines and full-screen programs need. The fragment unlocks the orientation while it is on screen and restores the lock on the way out, so nothing else gains a rotation it was not designed for. The session survives the rotation on its own: it belongs to the manager rather than the fragment, onDestroyView only detaches the listener, and attaching replays the scrollback into the recreated view. The arguments needed one fix for this. A fragment opened from the connect dialog carries host details and no session id, so a recreated view took the create path and would have dialled a second session to the same target on every turn of the screen. The id of a session created here is written back into the arguments, and the password dropped from them now that it has been handed to the session. * Prompt to trust an unknown SSH host key Regular SSH servers previously connected without any host-key check. Show the presented fingerprint for an untrusted host and, once the user confirms it, reconnect with the key trusted; the Go side then stores it in a per-profile known-hosts file and verifies against it thereafter. The store is per profile, since an overlay IP is a different host under a different profile, and a profile's file is removed with the profile. A host's key is also dropped once no session targets it, so deleting the last session for a host clears its trusted key while a shared host keeps it. Bumps the netbird submodule for the host-key verification changes. * Localize the SSH feature into every supported language The SSH terminal strings only existed in the base resources, so the whole feature showed in English under de, es, fr, hu, it, ja, pt, ru and zh-rCN. Add the 34 strings to each, port numbers and format placeholders left intact. * Bump the netbird submodule to the main merge * Bump netbird submodule for the SSH close and reconnect fixes * Keep the terminal key bar above the navigation bar * Give the SSH session list a title bar and room to breathe * Call deleting a session what it is * Refuse an SSH redial the stopped engine cannot serve * Give the settings list a title bar too * Fix threading and lifecycle faults around SSH sessions Synchronize the session snapshot, keep view writes on the main thread, survive an activity recreate, and stop leaking session listeners. * Reject an out-of-range port in the SSH connect dialog * Localize the SSH status and state text * Make the SSH rows reachable and match the peer list Name the symbol keys for screen readers, grow the peer SSH button to the 48dp touch target, and give the session state bar the peer list's shape. * Bump netbird submodule to v0.77.0 (#238) Co-authored-by: netbirddev <dev@netbird.io> * Update netbird submodule to deduplicated SSH client * Update netbird submodule to serialized wasm SSH startup * Post the device-code login opener's UI work to the main thread The Go flows now invoke URLOpener.open synchronously from a Go thread, so openers must not do UI work inline. The device-code login opener was the only one still showing the QR dialog and starting the browser on the calling thread; post both to the main thread like the extend opener does, and post onLoginSuccess as well so the dialog field is only touched from the main thread. Bump netbird for the shared OAuth token flow. * Bump netbird for constructor-style login hints * Bump netbird to the main merge * Default the peer SSH port to 22 Peers now serve SSH on the standard port, so the dialog no longer prefills 22022. * Store SSH sessions and known hosts in the profile's preferences The session list moves out of Java SharedPreferences and the known-hosts files out of filesDir, into the per-profile preference store the profile manager now owns. Deleting a profile deletes both with it, so the sweep that discarded lists and key files left behind by deleted profiles is gone, and setProfile no longer needs the set of live profile IDs. SshSessionStore keeps only the last-used login name, which is deliberately per device rather than per profile. Bump netbird to the profile preference store. * Support ad-hoc snapsot APK from CI (#242) * Add manual workflow to build a version-code-consistent APK for ad-hoc distribution (#236) * Add manual workflow to build a version-code-consistent APK for ad-hoc distribution Adds prepare-publication.yml (workflow_dispatch) that reuses the release signing key and computes version_code as the combined run count of itself and build-release.yml, so the two never collide. A shared concurrency group on both workflows prevents a race on that computation. * Draw the release version code from the shared run counter build-release.yml derived its version code from github.run_number, which counts only its own runs and is blind to prepare-publication.yml. With 25 release runs so far, the first ad-hoc build would take 25+1+40=66 and the next release would take 26+40=66 as well, then fall behind: 67 after 68 was already published. The concurrency group cannot fix this, because run_number is assigned when the run is queued, not when the step executes. Both workflows now sum the same two counters, so every run of either one advances the code by exactly one. Reading those counts needs actions: read, which an explicit permissions block otherwise withholds. * Name the manual workflow build-snapshot prepare-publication named a step in a process, while the workflows beside it name what they produce: build-debug, build-release. What this one produces is a release-signed build from an arbitrary commit with no tag behind it, which is what snapshot means. Not build-rc: release candidates already exist here as published pre-release tags (v0.6.0-rc.1, v0.3.3-rc.2) and are built by build-release.yml, so the name would claim a meaning the repository has already given away. The version code counter is keyed by workflow file name, so the rename is free only while the workflow has no runs yet. * Label snapshot builds snapshot- rather than ci- The version name travels to the management server as the peer's ui_version and is what the about screen shows, so it is the only thing telling support which build a peer is running. build-debug.yml already emits ci-<sha> from the same expression, which left an unsigned PR build and a release-signed hand-out looking identical in the peer list. The artifact keeps just the version name; prefixing it again read as snapshot-artifacts-snapshot-<sha>. * Resolve the Go version from the submodule's release tags in CI CI builds used the version only when the submodule sat exactly on a tag and fell back to ci-<sha> otherwise, which the management server rejects in NBVersionCheck posture checks: it treats ci- as a development build everywhere except there. Since the submodule is bumped more often than it is tagged, release builds effectively always shipped as ci-<sha>. CI now resolves the version by walking the pinned commit's ancestry back to the last stable release tag and appending the commit as SemVer build metadata, e.g. 0.77.0+f06b8c762. The server strips build metadata before every comparison, so this passes the same gates as a plain 0.77.0 while still naming the exact commit in the dashboard. Pre-release tags are skipped as a base: a suffix like -rc.2 lands in SemVer pre-release position, which the server compares differently from a release. Local builds now always produce dev-<sha>, which skips every server-side version gate; a developer who needs a real version passes it as the argument. The ancestry walk needs full history, but actions/checkout clones submodules shallow — the tags arrive without the commits between HEAD and the tag, and the walk would silently come up empty. The composite action therefore unshallows the submodule before building, guarded because --unshallow on a complete repository is a hard error. * Document the three build workflows and their differences * Fail the build when a run count cannot be fetched The zero fallback existed for one legitimate case: the runs endpoint returns 404 until a workflow has run or reached the default branch, and treating that as zero is what lets build-release compute a code before build-snapshot's first run. But it also swallowed every other failure — a network error or a revoked token minted a version code far below the published ones, silently for hand-distributed snapshots. Keep the 404-means-zero case and abort on everything else, including a non-numeric response, which bash arithmetic would otherwise fold to zero. * Pin gobind alongside gomobile instead of running gomobile init (#240) * Pin gobind alongside gomobile instead of running gomobile init gomobile bind shells out to gobind, and gobind is the tool that actually generates the Java bindings and the JNI glue. CI cached and installed only gomobile and left the build script to call `gomobile init`, which installs gobind from @latest: the driver was pinned while the generator floated, so the generated API could change without a commit here. Take the revision from the go.mod the submodule already carries, so the two tools cannot drift apart and the cache key follows a submodule bump on its own, and have the build script check that the pair is present rather than reaching for gomobile init. The cache key names both tools now. Entries saved under the old key hold no gobind, so keeping it would hit the cache, skip the install and leave the build without a generator. * Install gomobile and gobind automatically at the go.mod pinned revision * ci: install the JDK from temurin instead of adopt setup-java fails to resolve the 'adopt' distribution — AdoptOpenJDK was renamed to Eclipse Temurin and the old endpoints are going away — so the build died before installing a JDK. Same JDK, current name. (cherry picked from commit 8592de3) * Cache the Android NDK and Go modules in the build action --------- Co-authored-by: DevBot NetBird <91386968+netbirddev@users.noreply.github.com> Co-authored-by: netbirddev <dev@netbird.io>
# Conflicts: # .github/workflows/build-snapshot.yml # docs/versioning.md
Collaborator
Author
done |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reworks the Android UI, and brings over features the desktop client already had.
Navigation
Home
New screens and features
SSO session expiry
Fixes