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.

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

Refresh look of cluster pods admin tab #1583

Merged
merged 6 commits into from
Jul 13, 2023

Conversation

NickLanam
Copy link
Member

@NickLanam NickLanam commented Jun 27, 2023

Summary: Adds summary info, cleans up presentation.
Before...
image

After (with no pod selected)...
image

And with one selected...
image

Relevant Issues: #1174

Type of change: /kind cleanup

Test Plan: Go to /admin/clusters. Click a cluster. Go to the Pixie Pods tab.

Signed-off-by: Nick Lanam <nlanam@pixielabs.ai>
@NickLanam NickLanam requested a review from vihangm June 27, 2023 18:41
@vihangm
Copy link
Member

vihangm commented Jun 28, 2023

IMO having both the healthy and unhealthy counts in the right side but also having a healthy pods section and unhealthy pods section of the table feels a bit strange and redundant.
What if we collapse it into one table with the aggregate counts but sort it by unhealthy pods first (so that any potentially problematic pods are first up).

@NickLanam
Copy link
Member Author

NickLanam commented Jun 28, 2023

IMO having both the healthy and unhealthy counts in the right side but also having a healthy pods section and unhealthy pods section of the table feels a bit strange and redundant. What if we collapse it into one table with the aggregate counts but sort it by unhealthy pods first (so that any potentially problematic pods are first up).

First group is control plane pods (which can be a mix of healthy and unhealthy), the other is data plane pods, though I suppose only unhealthy ones so I should at least remove the healthy counter on that one.

Are you suggesting combining the groups anyway and distinguishing them a different way?

@vihangm
Copy link
Member

vihangm commented Jun 28, 2023

First group is control plane pods (which can be a mix of healthy and unhealthy), the other is data plane pods, though I suppose only unhealthy ones so I should at least remove the healthy counter on that one.

Are you suggesting combining the groups anyway and distinguishing them a different way?

Ah I wasn't paying enough attention, yeah the count in dataplane pods is misleading then.
I still think this page is quite challenging to parse, and the counts don't really add much info since a single control plan pod being unhealthy means that pixie is likely not working.

I do still think that combining the tables and using some iconography to help indicate dataplane/controlplane might improve the flow here, what do you think?

Signed-off-by: Nick Lanam <nlanam@pixielabs.ai>
@NickLanam
Copy link
Member Author

image With some improvements to the padding, spacing, general C.R.A.P. principles of design.

@vihangm
Copy link
Member

vihangm commented Jun 29, 2023

Another nit: the before had chevrons to indicate that the rows could be expanded. Now there's no such indicators. can you add some right arrows? >

Signed-off-by: Nick Lanam <nlanam@pixielabs.ai>
@NickLanam
Copy link
Member Author

Another nit: the before had chevrons to indicate that the rows could be expanded. Now there's no such indicators. can you add some right arrows? >

It was already there for the selected row, but here it is with it always showing and just being brighter for that row:
image

Signed-off-by: Nick Lanam <nlanam@pixielabs.ai>
@NickLanam
Copy link
Member Author

image And now with last round of suggestions.

Signed-off-by: Nick Lanam <nlanam@pixielabs.ai>
@aimichelle
Copy link
Member

Some more feedback on the design: how does this look if the status message is very long?
And how does it look if we change the table header to "Status Message" instead of "Status" for clarity? (I think just "Status" is a bit confusing since the status is technically the green checkmark).

And for the expanded view when you've clicked into a pod: do you think it'll look better with some padding at the top so that the pod name is a little lower?

Signed-off-by: Nick Lanam <nlanam@pixielabs.ai>
@NickLanam
Copy link
Member Author

Some more feedback on the design: how does this look if the status message is very long? And how does it look if we change the table header to "Status Message" instead of "Status" for clarity? (I think just "Status" is a bit confusing since the status is technically the green checkmark).

And for the expanded view when you've clicked into a pod: do you think it'll look better with some padding at the top so that the pod name is a little lower?

image

@aimichelle aimichelle merged commit 682a41a into pixie-io:main Jul 13, 2023
@NickLanam NickLanam deleted the pod-tab-refresh branch July 13, 2023 20:23
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.

3 participants