Skip to content

feat(android): improve sharing credential actions#31

Open
AmirrezaFarnamTaheri wants to merge 1 commit into
Hidden-Node:mainfrom
AmirrezaFarnamTaheri:android-sharing-credential-actions
Open

feat(android): improve sharing credential actions#31
AmirrezaFarnamTaheri wants to merge 1 commit into
Hidden-Node:mainfrom
AmirrezaFarnamTaheri:android-sharing-credential-actions

Conversation

@AmirrezaFarnamTaheri
Copy link
Copy Markdown
Contributor

Summary

  • add generate and rotate actions for internet sharing credentials
  • show concrete SOCKS5 and HTTP LAN endpoints when sharing is enabled
  • keep existing blocking validation for empty sharing credentials

Validation

  • git diff --check
  • gradle :app:compileDebugKotlin reaches Kotlin compilation but is blocked by the existing missing gomobile mobile package/AAR setup.

@Hidden-Node
Copy link
Copy Markdown
Owner

Thanks for the hard work on these improvements! The new sharing credential actions and the local IP display are great features that significantly enhance the user experience.

However, given the large volume of changes in this PR, I have a couple of suggestions to ensure the long-term maintainability and robustness of the code:

Reactive Network IP Updates: Currently, the local IP seems to be remembered only when the screen is first composed. If a user changes their network state (e.g., toggles Wi-Fi or enables a Hotspot) while the screen is open, the displayed IP will become stale. It would be better to monitor network changes and update the IP address dynamically.
Code Organization & Refactoring: GlobalSettingsScreen.kt has become quite large and complex. To improve readability, I suggest moving the logic for password generation and local IP discovery into the ViewModel or a dedicated Utility/Helper class.
Also, please make sure to thoroughly test these changes on a physical device or emulator yourself before submitting the updates, specifically checking how the UI behaves during network transitions.

Could you please address these points? Once the logic is refactored and the IP display is more reactive, I'll be happy to review it again for merging. Thanks!

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