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

reworked systems admin page #405

Merged
merged 10 commits into from
Apr 12, 2023

Conversation

west270
Copy link
Contributor

@west270 west270 commented Mar 17, 2023

Closes #392, #393, #307

Reworked the systems admin page to look and feel like the old UI and changed the UnassociatedRunners to list each runner under a path rather than showing just the path.

Also on the request view page I changed the import for RemakeRequestButton so the spacing around the navbar is the same as the other pages.

@west270 west270 requested a review from a team March 17, 2023 18:32
@west270 west270 marked this pull request as draft March 23, 2023 13:22
@scott-taubman
Copy link
Contributor

Two general observations from trying it out:

  • The text all being blue makes everything look like a link. Especially if / when we remove the underlines from links, that's going to be very confusing. I'd say make the non-link text black.
  • The status badges are definitely harder to notice at a glance than in the old UI. The old way had the a nice strong color for the badge, with white text. The new way has a super light color for the badge and then it's really only the text that has the stronger color. It just feels way easier to miss at a glance that something is in "stopped" status.

compre:
image

to:
image

That older style one stands out a lot more to me.

Copy link
Collaborator

@obr42 obr42 left a comment

Choose a reason for hiding this comment

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

I wish there were tests added for the new files

src/pages/SystemAdmin/SystemAdmin.tsx Show resolved Hide resolved
src/containers/PermissionsContainer.tsx Show resolved Hide resolved
src/pages/SystemAdmin/SystemsCard.tsx Outdated Show resolved Hide resolved
src/pages/SystemAdmin/UnassociatedRunners.tsx Outdated Show resolved Hide resolved
src/pages/SystemAdmin/UnassociatedRunners.tsx Show resolved Hide resolved
src/pages/SystemAdmin/SystemsCard.tsx Outdated Show resolved Hide resolved
src/pages/SystemAdmin/RunnerCardInstances.tsx Show resolved Hide resolved
src/pages/SystemAdmin/RunnerCardInstances.tsx Outdated Show resolved Hide resolved
@west270 west270 marked this pull request as ready for review March 30, 2023 18:07
@west270 west270 requested a review from obr42 March 30, 2023 18:07
Copy link
Collaborator

@obr42 obr42 left a comment

Choose a reason for hiding this comment

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

Code looks good otherwise to me, ops check looks good

src/pages/CommandIndex/CommandIndex.tsx Outdated Show resolved Hide resolved
src/pages/SystemAdmin/SystemsCard.test.tsx Outdated Show resolved Hide resolved
@west270 west270 requested a review from obr42 April 7, 2023 17:40
Copy link
Collaborator

@obr42 obr42 left a comment

Choose a reason for hiding this comment

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

LGTM

@scott-taubman
Copy link
Contributor

Some observations from testing:

  • There is no confirmation dialog when deleting a system.
  • The buttons for the system feel like they should be right aligned with the buttons for the instances (as they were in the old UI).
  • There is no feedback or response when you stop a system or instance. The status does not change from RUNNING to STOPPED (or vice versa). You have to reload the page to see the update. The websocket connection does not appear to be working correctly so that is possible an unrelated issue that needs to be addressed.

@scott-taubman scott-taubman merged commit 54d8ac5 into beer-garden:main Apr 12, 2023
@west270 west270 deleted the rework-system-admin-page branch May 17, 2023 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants