ControlSystemStudio / phoebus

A framework and set of tools to monitor and operate large scale control systems, such as the ones in the accelerator community.
http://phoebus.org/
Eclipse Public License 1.0
92 stars 89 forks source link

Improve alarm views #2474

Open georgweiss opened 1 year ago

georgweiss commented 1 year ago

To follow up on recent changes in customization of colors (text and background) used to render alarm information, I'd like to propose even further customizations. The basic idea is that currently we convey information primarily through color, and the number of colors is doubled to account for acknowledge actions. This results in a large number of colors which we think is a bit problematic from a UI perspective.

At ESS we would like use an additional icon to show when an item has been acknowledged, see screen shot below. This instead of modifying colors and showing a different icon which on a high resolution screen is difficult to distinguish from the "unacknowledged" one.

Also shown in screen shots is a proposal for simplified icons for minor/major/undefined. These would be available in both black and white (if a darker background is used).

All in all I'd like to put this forward as a suggestion to add customization of:

In any case we'd like to exchange the current icons for the simplified ones.

Comments, anyone?

Screenshot 2022-11-28 at 09 42 08 Screenshot 2022-11-28 at 09 43 06 Screenshot 2022-11-28 at 09 44 55
georgweiss commented 1 year ago

@kasemir , @shroffk , my users are repeatedly asking for updates in the alarm views. I'd like to handle this such that also the icon handling is addressed as proposed in this issue. In particular we propose to use separate (and simpler) icons for state and acknowledge.

kasemir commented 1 year ago

Well, icons are a personal preference. See who proudly provided the current set of icons 7 years ago, which cleverly add a checkmark decorator for the acknowledged variants, see https://github.com/ControlSystemStudio/cs-studio/tree/master/core/ui/ui-plugins/org.csstudio.ui.resources/icons So whatever you do now, somebody won't like it, and being able to hide the icons column from the alarm table becomes an important option. Still, simpler is good. Since the text colors are now adjustable, having icons that follow those colors would be good, for example using two-color icons where the colors are then derived from the configurable alarm colors. Or just black and white. Wider icons or basically two icons to separately indicate acknowledgement are fine, but if you contemplate a checkbox-type indicator, that could be mistaken for a usable checkbox. So you might as well use an actual checkbox, which can then be clicked to ack/un-ack an individual alarm tree entry.

georgweiss commented 1 year ago

Thanks for your feedback, @kasemir.

The thing with the current icon scheme is that the checkmark decorator can be difficult to see, especially on high-resolution monitors.

hide the icons column from the alarm table becomes an important option.

Not sure I understand this. Currently I see no way to remove the icon column in the alarm table.

I would absolutely vote for black and white: simple and easy to "read". A clickable checkbox is certainly a good alternative, and takes care of the "decoration" at the same time. But that would of course mean that a checkbox is rendered for all items in the view.

kasemir commented 1 year ago

remove the icon column in the alarm table

See the "alarm_table_columns" preference setting, https://github.com/ControlSystemStudio/phoebus/blob/master/app/alarm/model/src/main/resources/alarm_preferences.properties#L64 Simply omit "Icon" and it's gone.

georgweiss commented 1 year ago

Thanks, I wasn't aware of this customization option. Of course we'll make sure not to break this.

georgweiss commented 1 year ago

@kasemir, @shroffk, would this be acceptable in terms of using a checkbox to handle and show acknowledge state?

Screenshot 2023-03-09 at 14 55 30

kasemir commented 1 year ago

On each PV, yes, but for higher level nodes you can have some PVs in the subtree ack'ed and others not, so that would require a "half checked" checkbox..

georgweiss commented 1 year ago

But the current behavior is in my view the same, though using decorated icon. Right?

kasemir commented 1 year ago

Well, yes. Each node shows the "maximized" severity. If all sub-nodes are OK, that's OK. If all sub-nodes are OK or in some ack'ed state, the summary is the highest ack'ed state. If any sub node is in an active, non-ack'ed state, the summary is the highest active state.

Would this make sense with a clickable checkbox?

Some sub-nodes in some ack'ed state or OK-> checkbox will be checked? When then clicking the box, will it un-ack the sub tree?

Some sub-nodes are in active alarm -> Checkbox is unchecked? Clicking the box will ack all alarms in the sub-tree?

All sub-nodes are OK --> node shows OK. What will the checkbox show? Since there is nothing to ack or un-ack, I think the checkbox should not be shown at all in this case.

georgweiss commented 1 year ago

When then clicking the box, will it un-ack the sub tree

Currently ack or unack on higher level node does ack/unack all active in subtree.

The idea is that the checkbox would replace the context menu item. It's state would replace the decoration on the icon.

Checkbox should not be shown on any item if entire subtree is OK.

kasemir commented 1 year ago

OK, then go ahead and try that

georgweiss commented 1 year ago

Well, after some experimenting with clickable checkbox widget I decided that this option messes up the UI more than it improves it. So I decided to ditch that option and follow-up later to propose new set of icons (as I still think current once are difficult to read when decorated with acknowledge).

In any case, I'm also following up on some work done by @shroffk to support customization if alarm views in terms of background and text color. In connection to this I realize the alarm table and the alarm log table adopt different strategies when rendering table cells, see screen shots below.

@kasemir, @shroffk, I suggest we make this a bit more consistent and set font and background color in both alarm table and alarm log table in the same manner.

Please share your thoughts.

Screenshot 2023-03-22 at 16 52 27 Screenshot 2023-03-22 at 16 52 34

kasemir commented 1 year ago

The top screenshot is the alarm log, the bottom is the alarm table, right?

Alarm table now uses the globally configured alarm text and background colors (#2449, #2458, #2473). By default they look like the bottom snapshot, white background, colored text, but sites could customize it to use black/white text and alarm-based background.

So this should simply be a matter of updating the alarm log to do the same.

kasemir commented 1 year ago

Regarding icons, a for-what-it's-worth look at the original implementation:

AlarmTree

There was a circle colored based on the severity. The left half indicates the alarm severity (latched to highest, may be "acknowledged") while the right half indicated the highest current severity. It was easy to spot a latched major alarm where the PVs have by now all returned to OK:

Screenshot 2023-03-22 at 12 16 09 PM

Icons for such a scheme are easy to create based on the configured alarm colors, so they would adapt to local settings, which is one big issue with the currently used set of static icon images.

I'd say that basic idea beat its successors, except for color vision impaired users who only had the text to go by. The new icons fixed that by being equally problematic no matter how good your color vision.