UI: mark visually motor positions outside of limits #1640
Conversation
8f8c0a5 to
3dd9683
Compare
There was a problem hiding this comment.
The motor can refuse to move to that position, but the widget will still display the invalid value. Thus the displayed value in the UI will be different from the motor's real position, without any visual indications.
I feel like the changes you propose are more of a workaround for this than a fix. Sure, it's always good to avoid making an invalid request, but if this request happens, the error should be dealt with in the UI. In other words, front-end validation should be an enhancement, not a requirement.
To recap the problem: when the back-end receives a value outside of [min, max], it should respond with an error (I assume it already does), and this error should be conveyed somehow to the user.
To do so, the setAttribute Redux thunk should be made asynchronous so that dispatch(setAttribute(attribute, val)) can be awaited and wrapped in try/catch. Then, in MotorInput, you can manage an error state and maybe pass an invalid prop to BaseMotorInput.
To reset this error/invalid state, you could:
- pass an
onBlurprop toBaseMotorInputto reset on blur; - start a timer in an effect in
MotorInputto reset after a few seconds; - leave the field in error state until the user changes the motor value again.
Your choice of resetting on blur seems fine to me in terms of UX.
If this all sounds too obscure, we can have a go at it if you like.
|
We need to be quite careful to make sure that we are consistent across the various underlying implementations (in I think its a great idea to show that a input is outside of the valid range, it helps the user to understand that the value is indeed out of range.
The limits of most actuators are available in the UI so we could also add an additional check. Keep in mind that, to complicate things, limits of some actuators can be a function of some other actuators position (i.e resolution). I think thats the only one at the moment. As @axelboc points out above it should be possible to catch this error in an |
|
The back-end does indeed respond with an error if you try to move motor outside of the limits. I agree with @axelboc that we should catch error responses for motor moves and display some kind of error message in the UI. It's possible for the move to fail due to some other problem, not necessary due to hitting a limit. I'll work on adding that. However, visually showing that the entered position is invalid is very useful feature for us. We are using MD3Up and it's easy get to the edges of motor limits. Currently it's a bit cryptic why some moves fail, you basically need to read the log. |
We completely agree, as mentioned above the limits are available on the UI side so you can easily add client side validation and some nice display of when the user tries to enter a value that is out of range. Our concern was more that this should not replace any server side handling and be done in such a way so that it reflects the actual state of the hardware with the correct message/display. I think that is also what you have in mind ? The |
Changes behavior of motor widgets when position outside of motor limits is entered. If entered position is outside of limits, change the widget style to 'invalid' mode. Block sending the 'invalid' position to the backend. When focus is lost, restore widget to display motor's real position. This changes the current behavior, where it's possible to enter and send a position outside of limits. The motor can refuse to move to that position, but the widget will still display the invalid value. Thus the displayed value in the UI will be different from the motor's real position, without any visual indications.
0a34141 to
b29b5c4
Compare
Make the setAttribute action async, this way we can await it and catch errors. When issuing 'move motor' action from MotorInput widget, catch errors thrown by the request. Show the error to the user with the error panel.
b29b5c4 to
f0d45fb
Compare
|
I have added code that catches errors on moving motors. The errors are displayed to the user with the error panel. To summarize current behavior:
Is this what you have in mind? |
|
yeah, that sounds reasonable to me. Do you have a screenshot of the error display ? |
| @@ -35,6 +36,14 @@ function MotorInput(props) { | |||
| return null; | |||
| } | |||
|
|
|||
| async function handleChange(val) { | |||
It shows the standard 'error bubble' with the error message from the back-end: |
Changes behavior of motor widgets when position outside of motor limits is entered.
If entered position is outside of limits, change the widget style to invalid mode. Block sending the invalid position to the back-end. When focus is lost, restore widget to display motor's real position.
This changes the current behavior, where it's possible to enter and send a position outside of limits. The motor can refuse to move to that position, but the widget will still display the invalid value. Thus the displayed value in the UI will be different from the motor's real position, without any visual indications.