Skip to content

[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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[client] Add Edit Button to Profile Window #4289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26a6085
[client] Enhance profile management UI with email display and edit fu…
hakansa Aug 5, 2025
4df7940
[client] Update profile display to include email if available
hakansa Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion client/ui/client_ui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,15 +424,28 @@ func (s *serviceClient) showSettingsUI() {
func (s *serviceClient) getSettingsForm() *widget.Form {

var activeProfName string
var activeProfEmail string
activeProf, err := s.profileManager.GetActiveProfile()
if err != nil {
log.Errorf("get active profile: %v", err)
} else {
activeProfName = activeProf.Name
// Try to get email from profile state
profileState, err := s.profileManager.GetProfileState(activeProf.Name)
if err == nil && profileState.Email != "" {
activeProfEmail = profileState.Email
}
}

// Create profile display with email if available
profileDisplay := activeProfName
if activeProfEmail != "" {
profileDisplay = fmt.Sprintf("%s (%s)", activeProfName, activeProfEmail)
}

return &widget.Form{
Items: []*widget.FormItem{
{Text: "Profile", Widget: widget.NewLabel(activeProfName)},
{Text: "Profile", Widget: widget.NewLabel(profileDisplay)},
{Text: "Quantum-Resistance", Widget: s.sRosenpassPermissive},
{Text: "Interface Name", Widget: s.iInterfaceName},
{Text: "Interface Port", Widget: s.iInterfacePort},
Expand Down
61 changes: 49 additions & 12 deletions client/ui/profile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ func (s *serviceClient) showProfilesUI() {
list := widget.NewList(
func() int { return len(profiles) },
func() fyne.CanvasObject {
// Each item: Selected indicator, Name, spacer, Select, Logout & Remove buttons
// Each item: Selected indicator, Name (with email), spacer, Select, Edit (for active), Logout & Remove buttons
return container.NewHBox(
widget.NewLabel(""), // indicator
widget.NewLabel(""), // profile name
widget.NewLabel(""), // profile name with email
layout.NewSpacer(),
widget.NewButton("Select", nil),
widget.NewButton("Edit", nil), // Edit button for active profiles
widget.NewButton("Logout", nil),
widget.NewButton("Remove", nil),
)
Expand All @@ -56,8 +57,9 @@ func (s *serviceClient) showProfilesUI() {
indicator := row.Objects[0].(*widget.Label)
nameLabel := row.Objects[1].(*widget.Label)
selectBtn := row.Objects[3].(*widget.Button)
logoutBtn := row.Objects[4].(*widget.Button)
removeBtn := row.Objects[5].(*widget.Button)
editBtn := row.Objects[4].(*widget.Button)
logoutBtn := row.Objects[5].(*widget.Button)
removeBtn := row.Objects[6].(*widget.Button)

profile := profiles[i]
// Show a checkmark if selected
Expand All @@ -66,7 +68,13 @@ func (s *serviceClient) showProfilesUI() {
} else {
indicator.SetText("")
}
nameLabel.SetText(profile.Name)

// Display profile name with email if available
displayName := profile.Name
if profile.Email != "" {
displayName = fmt.Sprintf("%s (%s)", profile.Name, profile.Email)
}
nameLabel.SetText(displayName)

// Configure Select/Active button
selectBtn.SetText(func() string {
Expand Down Expand Up @@ -127,6 +135,18 @@ func (s *serviceClient) showProfilesUI() {
)
}

// Configure Edit button (only for active profiles)
if profile.IsActive {
editBtn.Show()
editBtn.SetText("Edit")
editBtn.OnTapped = func() {
// Open settings window
s.eventHandler.runSelfCommand(s.ctx, "settings", "true")
}
} else {
editBtn.Hide()
}

logoutBtn.Show()
logoutBtn.SetText("Logout")
logoutBtn.OnTapped = func() {
Expand All @@ -143,7 +163,7 @@ func (s *serviceClient) showProfilesUI() {
if !confirm {
return
}

err = s.removeProfile(profile.Name)
if err != nil {
log.Errorf("failed to remove profile: %v", err)
Expand Down Expand Up @@ -221,7 +241,7 @@ func (s *serviceClient) showProfilesUI() {
content := container.NewBorder(nil, newBtn, nil, nil, list)
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))
Copy link
Preview

Copilot AI Aug 5, 2025

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.

Suggested change
s.wProfiles.Resize(fyne.NewSize(600, 300))
s.wProfiles.Resize(fyne.NewSize(profilesWindowWidth, profilesWindowHeight))

Copilot uses AI. Check for mistakes.

s.wProfiles.SetOnClosed(s.cancel)

s.wProfiles.Show()
Expand Down Expand Up @@ -301,6 +321,7 @@ func (s *serviceClient) removeProfile(profileName string) error {
type Profile struct {
Name string
IsActive bool
Email string
}

func (s *serviceClient) getProfiles() ([]Profile, error) {
Expand All @@ -323,9 +344,17 @@ func (s *serviceClient) getProfiles() ([]Profile, error) {
var profiles []Profile

for _, profile := range profilesResp.Profiles {
// Try to get email from profile state
email := ""
profileState, err := s.profileManager.GetProfileState(profile.Name)
if err == nil && profileState.Email != "" {
Copy link
Preview

Copilot AI Aug 5, 2025

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.

Suggested change
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.

email = profileState.Email
}

profiles = append(profiles, Profile{
Name: profile.Name,
IsActive: profile.IsActive,
Email: email,
})
}

Expand All @@ -340,21 +369,21 @@ func (s *serviceClient) handleProfileLogout(profileName string, refreshCallback
if !confirm {
return
}

conn, err := s.getSrvClient(defaultFailTimeout)
if err != nil {
log.Errorf("failed to get service client: %v", err)
dialog.ShowError(fmt.Errorf("failed to connect to service"), s.wProfiles)
return
}

currUser, err := user.Current()
if err != nil {
log.Errorf("failed to get current user: %v", err)
dialog.ShowError(fmt.Errorf("failed to get current user"), s.wProfiles)
return
}

username := currUser.Username
_, err = conn.Logout(s.ctx, &proto.LogoutRequest{
ProfileName: &profileName,
Expand All @@ -365,13 +394,13 @@ func (s *serviceClient) handleProfileLogout(profileName string, refreshCallback
dialog.ShowError(fmt.Errorf("logout failed"), s.wProfiles)
return
}

dialog.ShowInformation(
"Logged Out",
fmt.Sprintf("Successfully logged out from '%s'", profileName),
s.wProfiles,
)

refreshCallback()
},
s.wProfiles,
Expand Down Expand Up @@ -457,9 +486,17 @@ func (p *profileMenu) getProfiles() ([]Profile, error) {
var profiles []Profile

for _, profile := range profilesResp.Profiles {
// Try to get email from profile state
email := ""
profileState, err := p.profileManager.GetProfileState(profile.Name)
if err == nil && profileState.Email != "" {
email = profileState.Email
}
Copy link
Preview

Copilot AI Aug 5, 2025

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.

Suggested change
}
email := p.getProfileEmail(profile.Name)

Copilot uses AI. Check for mistakes.


profiles = append(profiles, Profile{
Name: profile.Name,
IsActive: profile.IsActive,
Email: email,
})
}

Expand Down
Loading