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 #23056 - Host discovery list uses wrong icon for new, reported and non reported statuses #416

Merged
merged 1 commit into from Apr 25, 2018

Conversation

ik5
Copy link
Contributor

@ik5 ik5 commented Mar 29, 2018

No description provided.

@theforeman-bot
Copy link
Member

Issues: #23056

@ohadlevy
Copy link
Member

please add a before/after image - thanks :)

@ik5
Copy link
Contributor Author

ik5 commented Mar 29, 2018

@ohadlevy Worked on doing just that :)

before:
gnome-shell-screenshot-b84cgz

After:
gnome-shell-screenshot-gmzngz

Copy link

@Rohoover Rohoover left a comment

Choose a reason for hiding this comment

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

Is the info icon intentionally green as opposed to grey or blue?

@ik5
Copy link
Contributor Author

ik5 commented Apr 1, 2018

@Rohoover I kept the colors, because I do not know what colors to place there instead.

@lzap
Copy link
Member

lzap commented Apr 4, 2018

If I look on the old screenshot, I can tell that the plus icon represents new hosts recently added. Now, on the proposal I can't immediately recognize difference between info and check. Do you have better ideas?

Copy link

@Rohoover Rohoover left a comment

Choose a reason for hiding this comment

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

I would change the info icon to blue to align with PF and provide an additional visual differentiator from the check. I believe this is the correct color: 0088ce

Sorry for the delay, I was on PTO.

@ik5
Copy link
Contributor Author

ik5 commented Apr 10, 2018

@Rohoover how is that?
new host discovery icons color info

No green anymore, just two shades of blue and a a red for warning

@lzap
Copy link
Member

lzap commented Apr 10, 2018

Let me explain the three icons here:

  • Host is up to date - discovery host reported in last 24 hours and everything is ok
  • Host was recently added - the above plus the report is 4 hours old (maximum)
  • Host is down - discovery host did not respond in last 24 hours (something is going on - perhaps it was turned off or network issue)

@lzap
Copy link
Member

lzap commented Apr 10, 2018

Now, I am trying to find best icons in the set: https://www.patternfly.org/styles/icons/

But I think what we have represents the above best, but maybe you have better idea. I don't like info icon, it's not telling anything in particular and it's too generic. Similarly to container icon, also you can imagine anything. How about:

  • Up to date: pficon-on-running
  • Recently added: pficon-on-running (in some different color or in bold if we have that)
  • Unknown / down: pficon-pending

@ohadlevy
Copy link
Member

ohadlevy commented Apr 10, 2018 via email

@lzap
Copy link
Member

lzap commented Apr 10, 2018

Yeah I kinda like it. If folks don't like colors, we can go with pficon-on-running in gray for up-to-date hosts and pficon-on-running in black for recently added to draw attention on them.

@ik5
Copy link
Contributor Author

ik5 commented Apr 10, 2018

@lzap, @ohadlevy
I did made some checking before hand of the icons, I didn't find any better icons to use.
I think the icons on patternfly are not that much, and because of it there are not much to choose from.

Also note that the patternfly page uses a table that explain the type of usage for most icons.

@lzap
Copy link
Member

lzap commented Apr 10, 2018

We need to step out of PatternFly constraints here, we have special situation - there's OK host, FRESH host and FAILED host. PatternFly only provides OK icon, we need something that's actually "above OK", for this reason we need to ditch PF's OK because that's too much IMHO. Here is another idea:

  • pficon-on-running - means "on and running" - for "recent hosts"
  • pficon-on - means "on" - for "ok hosts"
  • pficon-paused (or pending) - means "paused or pending" - for "out of sync" hosts

@ohadlevy
Copy link
Member

@lzap lets wait for @Rohoover feedback regarding which icon to use....

Copy link

@Rohoover Rohoover left a comment

Choose a reason for hiding this comment

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

So I saw a few statuses listed with the definitions colliding at time. It's important to clearly define these status and not conflate them because the iconography changes. Below is a list of what I'm seeing described by all the different posters. I'm also for brevity including what icon I would put with each status, even though not all those statuses are valid.

Host (on and running) - pficon-on-running
Host (on and okay) - pficon-ok
Host Pending - pficon-pending
Host Paused - pficon-paused
Host Error - pficon-error-circle-o
Host Warning - pficon-warning-triangle-o

Let's confirm which are valid and it's clear definition. For example, I think it's important to use the pending icon, when something is actually pending, not when a status is unknown.

Also there is a list of PF icons but we can also potentially use icons from Font Awesome, which is the base set that PF uses.

@ohadlevy The PF icon for ok and running is relatively new, at least since we did the original host set, so I do recommend using it if it applies.

As far as colors, this is how we assign colors from UX:
Red = Error
Yellow = Warning
Blue = Interesting or primary action
Grey = Not as interesting, not primary

@ik5
Copy link
Contributor Author

ik5 commented Apr 11, 2018

@Rohoover , @lzap , @ohadlevy
If I understand this discussion right, you are referring to this type of look, right?
tmp-colors

Copy link

@Rohoover Rohoover left a comment

Choose a reason for hiding this comment

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

Yes, though I would comment that "on and running" may be green. Green is telling the user things are fine, nothing to worry about. I would ask oneself "Which status best represents that?"

Is the power icon meaning the same as just 'on and okay'?

@ik5
Copy link
Contributor Author

ik5 commented Apr 12, 2018

Green in this case was on a newly added server, and that's imo is not "good and running", but rather "discovered and known", so I'm not sure that green is the right color here.

I'm adding the code here, though, as seen on the screenshot

Copy link

@Rohoover Rohoover left a comment

Choose a reason for hiding this comment

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

@ik5

I think we are agreed. On and running = green, new = blue.

@lzap
Copy link
Member

lzap commented Apr 17, 2018

@ik5 can you please add another screenshot with the final version so we can all approve here, I see there was another commit after last screenshot. Want to be sure we all comment the correct picture. Thanks!

@ik5
Copy link
Contributor Author

ik5 commented Apr 22, 2018

Here it is:
new host discovery icons color info2

status_message = _('Not reported in more than 7 days')
status_color = '#AA4643'
status_color = ' #ec7a08'
Copy link
Member

Choose a reason for hiding this comment

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

Leading space, other than that I am good thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, thanks

@lzap lzap merged commit 2472d09 into theforeman:develop Apr 25, 2018
@lzap
Copy link
Member

lzap commented Apr 25, 2018

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants