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
Implement Monaco editor for Alertmanager YAML editor #3567
Implement Monaco editor for Alertmanager YAML editor #3567
Conversation
frontend/public/components/monitoring/alert-manager-yaml-editor.tsx
Outdated
Show resolved
Hide resolved
frontend/public/components/monitoring/alert-manager-yaml-editor.tsx
Outdated
Show resolved
Hide resolved
Generally LGTM 👍 . Just one issue with the header area layout. |
c3a8d7b
to
f9cce51
Compare
f9cce51
to
862cbaa
Compare
862cbaa
to
5f8a13e
Compare
/test analyze |
@@ -666,7 +666,7 @@ const EditYAML_ = connect(stateToProps)( | |||
}); | |||
|
|||
const { error, success, stale, yaml, height, showSidebar } = this.state; | |||
const { obj, download = true, header } = this.props; | |||
const { obj, download = true, header, showReload = true } = this.props; |
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.
Booleans that default to true are confusing. I'd make this noReload
.
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.
changed to nonK8s
, but seems a little wonky specifing {stale && !nonK8s && ( ...
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.
changed to
nonK8s
, but seems a little wonky specifing{stale && !nonK8s && ( ...
Is stale
ever true if this isn't a k8s object? I wonder if the nonK8s
check is needed
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.
for stale
the nonK8s
check isn't necessary. Still have {!create && !nonK8s && (..
- I just don't like '!non...`.
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.
You could change nonK8s
to something like anyYAML
or genericYAML
. I think that's more clear anyway.
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.
We should be skipping all the logic that checks for schemas, etc., when set.
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.
I like it, changed to genericYAML
-thanks
@alimobrem fyi |
We should hide |
5f8a13e
to
245f36e
Compare
245f36e
to
35a02d6
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dtaylor113, spadgett The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
22 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
https://jira.coreos.com/browse/CONSOLE-1765
Before
After