Skip to content

Conversation

@lixmal
Copy link
Collaborator

@lixmal lixmal commented Dec 1, 2025

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes
    • Exit Node menu item now correctly enables when connected to the service.
    • Improved reliability of Settings and Profiles menu state synchronization to ensure consistent availability and responsiveness.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 1, 2025

Walkthrough

This change adds internal state tracking for UI feature toggles in the serviceClient. It introduces two new boolean fields (settingsEnabled, profilesEnabled) and refines the updateStatus and checkAndUpdateFeatures methods to use stateful comparisons with synchronization locks instead of unconditional enable/disable operations.

Changes

Cohort / File(s) Summary
Feature toggle state tracking
client/ui/client_ui.go
Added two new internal boolean fields (settingsEnabled, profilesEnabled) to track feature state. Refined updateStatus to conditionally enable the Exit Node menu when connected and UI switch is active. Modified checkAndUpdateFeatures to use guarded, stateful comparisons and updates for settings and profiles via new fields with synchronization lock.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review the synchronization lock placement in checkAndUpdateFeatures to ensure proper thread safety
  • Verify stateful comparisons in updateStatus correctly handle the UI switch conditional flow
  • Confirm the new settingsEnabled and profilesEnabled fields are properly initialized and maintained throughout the component lifecycle

Poem

🐰 Two toggles now track what we need,
State-synchronized at stateful speed,
Feature flags dance with locks in place,
Menu items appear with newfound grace!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. Critical sections like 'Describe your changes' and 'Issue ticket number and link' are empty or missing substantive details. Fill in the 'Describe your changes' section with a clear explanation of the bug fix and why it was needed. Add the issue ticket number and link. Ensure documentation selection is completed properly.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: fixing Linux UI flickering on state updates, which aligns with the code changes that add stateful feature tracking to prevent UI element flickering.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-linux-ui-flickering

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants