-
-
Notifications
You must be signed in to change notification settings - Fork 876
[client] Add Edit Button to Profile Window #4289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds an Edit button to the NetBird client's profile window that allows users to edit active profiles. The changes enhance the user interface by providing a more intuitive way to access profile settings directly from the profiles list.
Key changes:
- Added an Edit button that appears only for active profiles and opens the settings window
- Enhanced profile display to show email addresses alongside profile names when available
- Increased window width to accommodate the additional UI elements
// Try to get email from profile state | ||
email := "" | ||
profileState, err := s.profileManager.GetProfileState(profile.Name) | ||
if err == nil && profileState.Email != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The profile state is fetched for every profile in a loop, which could result in N+1 query performance issues. Consider batching these operations or caching profile states if this method is called frequently.
if err == nil && profileState.Email != "" { | |
// Batch fetch profile states to avoid N+1 queries | |
profileNames := make([]string, 0, len(profilesResp.Profiles)) | |
for _, profile := range profilesResp.Profiles { | |
profileNames = append(profileNames, profile.Name) | |
} | |
profileStates := batchGetProfileStates(s.profileManager, profileNames) | |
for _, profile := range profilesResp.Profiles { | |
// Try to get email from profile state | |
email := "" | |
if profileState, ok := profileStates[profile.Name]; ok && profileState != nil && profileState.Email != "" { |
Copilot uses AI. Check for mistakes.
profileState, err := p.profileManager.GetProfileState(profile.Name) | ||
if err == nil && profileState.Email != "" { | ||
email = profileState.Email | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for fetching profile state and email is duplicated between two methods. Consider extracting this into a helper function to reduce code duplication and improve maintainability.
} | |
email := p.getProfileEmail(profile.Name) |
Copilot uses AI. Check for mistakes.
s.wProfiles = s.app.NewWindow("NetBird Profiles") | ||
s.wProfiles.SetContent(content) | ||
s.wProfiles.Resize(fyne.NewSize(400, 300)) | ||
s.wProfiles.Resize(fyne.NewSize(600, 300)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The hardcoded window dimensions (600, 300) should be defined as constants or computed based on content to improve maintainability and make the UI more responsive to different screen sizes.
s.wProfiles.Resize(fyne.NewSize(600, 300)) | |
s.wProfiles.Resize(fyne.NewSize(profilesWindowWidth, profilesWindowHeight)) |
Copilot uses AI. Check for mistakes.
|
[client] Add Edit Button to Profile Window
Describe your changes
Issue ticket number and link
Stack
Checklist