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

Fixes #24361: Status point next the node hostname doesn't provide much info #5508

Conversation

RaphaelGauthier
Copy link
Member

@RaphaelGauthier RaphaelGauthier commented Mar 21, 2024

}
&.empty-policies:after{
&.empty-policies{
background-color: #7591c599;
Copy link
Member

Choose a reason for hiding this comment

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

Isn't a typo on the color here ? I think it's #7591c5 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not a typo, the last two digits represent the alpha channel. So in this case, 99 corresponds to 60% opacity!

@VinceMacBuche
Copy link
Member

I think that's too big and it really catches the eye and you see to much. I don't see the node name anymore

@VinceMacBuche
Copy link
Member

I would :

  • Keep a circle but the color needs to change with some status.
  • When not enabled (and maybe others), the status needs to be displayed in a pretty label not too much invasive (no caps) and probably on the right (after RAM)
  • probably a tooltip on the circle

@ElaadF
Copy link
Member

ElaadF commented Mar 27, 2024

  • It is almost a standard to have an indication of the status next to the name, and doesn't make sens to me to move it else where, since this is important information when it is not enabled. Two locations for the same information is not the best choice IMO
  • If the meaning of the status circle color is not obvious (the circle green is ok) we should use a label, like the previous screen with the name in it, tooltips are nightmare to maintain and it is not the best way to provide information, I think other status than enabled are not very common, making them usefull when they are written and visible, and keep the green circle when it is the most frequent state to not attract too much attention because, I assume, it will be the most frequent scenario

@RaphaelGauthier
Copy link
Member Author

PR rebased

@RaphaelGauthier RaphaelGauthier force-pushed the bug_24361/status_point_next_the_node_hostname_doesn_t_provide_much_info branch from d5beb71 to a0814ce Compare March 27, 2024 15:49
@RaphaelGauthier
Copy link
Member Author

RaphaelGauthier commented Mar 27, 2024

PR updated with a new commit

  • If enabled, no label is displayed
  • for other states, the label has been reduced to a minimum (label text in capitals and not in uppercase, and padding reduced)
  • there is only a small rotating circle for the initializing state, as it is a temporary state (but it can also be removed)

nodestate-4
nodestate-3
nodestate-2
nodestate-1

@RaphaelGauthier
Copy link
Member Author

RaphaelGauthier commented Mar 27, 2024

PR updated with a new commit

Another (very) small change, I reduced the gap between machine info items :
nodestate-B

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5508
-- Your faithful QA
Kant merge: "Live your life as though your every act were to become a universal law."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/82413/console)

@VinceMacBuche
Copy link
Member

At least it covers the needs to display the status so i merge it

@RaphaelGauthier
Copy link
Member Author

OK, squash merging this PR

@RaphaelGauthier RaphaelGauthier force-pushed the bug_24361/status_point_next_the_node_hostname_doesn_t_provide_much_info branch from 3b3ed30 to 659d77a Compare March 28, 2024 10:14
@fanf
Copy link
Member

fanf commented Mar 28, 2024

OK, merging this PR

@fanf fanf merged commit 0b98ebc into Normation:branches/rudder/8.1 Mar 28, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants