Skip to content
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

Jump to bottom

ctx: Show unselectable items for unreachable Spaces #555

Merged
merged 2 commits into from
Jun 7, 2024
Merged

ctx: Show unselectable items for unreachable Spaces #555

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
505ebf3
ctx: Show unselectable items for unreachable Spaces
adamwg Jun 6, 2024
f00367f
fixup! ctx: Show unselectable items for unreachable Spaces
adamwg Jun 6, 2024
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
46 changes: 39 additions & 7 deletions cmd/up/ctx/navigation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package ctx

import (
"context"
"errors"
"fmt"
"os"
"os/exec"
Expand Down Expand Up @@ -304,16 +305,34 @@ func (o *Organization) Items(ctx context.Context, upCtx *upbound.Context, navCtx
}
}

// todo(redbackthomson): Should the space still appear, even if it's
// unreachable? Maybe mark it with an icon and make it unselectable?
if space.Status.ConnectionDetails.Status == upboundv1alpha1.ConnectionStatusUnreachable {
mu.Lock()
items = append(items, item{
text: space.GetObjectMeta().GetName() + " (unreachable)",
kind: "space",
notSelectable: true,
})
mu.Unlock()
continue
}

ingress, err := navCtx.ingressReader.Get(ctx, space)
if err != nil {
// TODO(adamwg): Add an unselectable item for the Space with
// relevant text depending on the type of error.
mu.Lock()
if errors.Is(err, spaces.SpaceConnectionError) {
items = append(items, item{
text: space.GetObjectMeta().GetName() + " (unreachable)",
kind: "space",
notSelectable: true,
})
} else {
items = append(items, item{
text: fmt.Sprintf("%s (error: %v)", space.GetObjectMeta().GetName(), err),
kind: "space",
notSelectable: true,
})
}
mu.Unlock()
continue
}

Expand Down Expand Up @@ -360,9 +379,22 @@ func (o *Organization) Breadcrumbs() string {

type sortedItems []list.Item

func (s sortedItems) Len() int { return len(s) }
func (s sortedItems) Less(i, j int) bool { return s[i].(item).text < s[j].(item).text }
func (s sortedItems) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
func (s sortedItems) Len() int { return len(s) }
func (s sortedItems) Less(i, j int) bool {
itmI := s[i].(item)
itmJ := s[j].(item)

// Sort unselectable items last.
if itmI.notSelectable && !itmJ.notSelectable {
return false
}
if !itmI.notSelectable && itmJ.notSelectable {
return true
}

return s[i].(item).text < s[j].(item).text
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is something we always want to commit to? There's a chance we'd want to put a header label, or a separator element, midway through the list and this would conflict?

Wdyt about, when we create the list of items, we have a second list of unreachable items. Then before returning the full list, we append the unreachable ones at the end?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had the same thought but discovered that this worked for all the current uses of sortedItems. I like the idea of making the ordering explicit in in Items() instead - updated to do that. As these lists get more complex we might think about adding more explicit structure to them so each Items() method doesn't have to do the same append+sort+append dance.

func (s sortedItems) Swap(i, j int) { s[i], s[j] = s[j], s[i] }

var _ Back = &Space{}

Expand Down
Loading