-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update to v1.1.0 #5
Conversation
Release/2
setSrvState({ status: "error", response: undefined }); | ||
return; | ||
} | ||
|
||
const serviceName = eStopAction === "go" ? config.goServiceName : config.stopServiceName; | ||
|
||
if (!serviceName) { | ||
setReqState({ status: "error", value: "Service name is not configured" }); | ||
setSrvState({ status: "error", response: undefined }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't these errors be propagated to the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was previously contained in the value field. I don't think it was used, instead I put the service response there and if it fails then a notification window will pop up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
capabilities: allowToggleSrvButton ? [PlayerCapabilities.toggleSrvButtons] : [], | ||
toggleSrvButton, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checks have failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least some of them seem to originate from this PR
Description:
ToggleSrvButton