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

feat(targets): target select component can be expanded to reveal details #501

Merged
merged 21 commits into from
Nov 3, 2022

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Aug 23, 2022

Related to #429
Related to #468

@andrewazores andrewazores added the feat New feature or request label Aug 23, 2022
@andrewazores andrewazores marked this pull request as draft August 23, 2022 13:56
@andrewazores andrewazores force-pushed the target-select-ids branch 2 times, most recently from 922edd8 to 48e52ca Compare November 2, 2022 20:16
@andrewazores andrewazores changed the title feat(targets): include target ID in dropdown feat(targets): target select component can be expanded to reveal details Nov 2, 2022
@andrewazores andrewazores marked this pull request as ready for review November 2, 2022 21:13
@andrewazores andrewazores requested review from tthvo and maxcao13 and removed request for tthvo November 2, 2022 21:13
Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Looks great! Feel very nice! Just a couple of questions :D

Copy link
Member

@maxcao13 maxcao13 left a comment

Choose a reason for hiding this comment

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

Looks really good, just a general question for the future and not content for this PR.

When we start to populate the Dashboard with various things listed in the Epic here https://github.com/cryostatio/cryostat/issues/945, does it make sense that SerializedTargets will start to include some data around things like CPU, memory, network, most recent archived recordings, Automated Analysis reports, etc. ? Or maybe have another wrapper around the SerializedTarget which includes that extra info, and keeps the Targets modular as they are?

@andrewazores
Copy link
Member Author

Looks really good, just a general question for the future and not content for this PR.

When we start to populate the Dashboard with various things listed in the Epic here cryostatio/cryostat#945, does it make sense that SerializedTargets will start to include some data around things like CPU, memory, network, most recent archived recordings, Automated Analysis reports, etc. ? Or maybe have another wrapper around the SerializedTarget which includes that extra info, and keeps the Targets modular as they are?

I don't think that info would go inside the SerializedTarget component. Perhaps there could be a modular/dynamic card on the Dashboard that also wraps a SerializedTarget so the user can have that more prominently and permanently displayed, but those other bits of information like summaries of recordings on the target or metrics pull from cryostat-reports/jfr-datasource would go on their own dynamic dashboard cards, not within the SerializedTarget.

Copy link
Member

@tthvo tthvo left a comment

Choose a reason for hiding this comment

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

Looks great to me! Very nicee :D

@andrewazores andrewazores merged commit 45842ca into cryostatio:main Nov 3, 2022
@andrewazores andrewazores deleted the target-select-ids branch November 3, 2022 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants