Hide eye icon on error #1819
Hide eye icon on error #1819
Conversation
s9ferech
added
Priority: High
| frp::extend! { network | ||
| out.source.skip <+ action_bar.action_skip; | ||
| out.source.freeze <+ action_bar.action_freeze; | ||
| frp.show_quick_action_bar_on_hover <+ no_error_set; |
There was a problem hiding this comment.
This might be more tricky than this solution. We don't necessarily want to hide ALL quick action icons. While there is just one at the moment, there will be others, and they might be independent of the error state of the node. So hiding all of them would not be what is needed. Instead, we just want to hide the visibility action icon.
There was a problem hiding this comment.
What should happen with the other icons then? Should they move to the left to take the place of the eye icon, or should they stay where they are?
There was a problem hiding this comment.
They should move to the left. If I do remember correctly, this might require some refactoring of the logic, though, as the positions might be fixed at the moment.
There was a problem hiding this comment.
Moving icons is bad solution IMO. The eye icon can be grayed out instead. Changing controls placement is super confusing to users. Peopel will get used to the fact that first icon is visualization. Imagine a situation when you try to click it, but then error occurs and you click other icon because it just changed place - this is a very bad UX
There was a problem hiding this comment.
@wdanilo Then we need to come up with a proper visual design for this. Fading out would probably not work because it would be confusable with the current active/not-active semantics, unless we re-design those.
Pull Request Description
This solves issue #1595. The eye icon on nodes will not be shown when the node is in an error state.
Important Notes
The will hide the whole action bar when there is an error. (There is only a single action at the moment.) If that behavior is not desired then some design decisions are necessary.
Checklist
Please include the following checklist in your PR:
CHANGELOG.mdwas updated with the changes introduced in this PR.