-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Inline Label Editing from Dashboards Index #12384
Conversation
Co-Authored-By: Delmer <[email protected]>
Co-Authored-By: Delmer <[email protected]>
Co-Authored-By: Andrew Watkins <[email protected]>
…fluxdata/influxdb into feat/dashboards-inline-labels
Everything the unit test is testing is also tested by the e2e test Co-Authored-By: Delmer <[email protected]>
Co-Authored-By: Delmer <[email protected]> Co-Authored-By: Andrew Watkins <[email protected]>
Co-Authored-By: Delmer <[email protected]> Co-Authored-By: Andrew Watkins <[email protected]>
Co-Authored-By: Delmer <[email protected]> Co-Authored-By: Andrew Watkins <[email protected]>
proto/bin_gen.go
Outdated
@@ -84,7 +84,7 @@ func systemJson() (*asset, error) { | |||
return nil, err | |||
} | |||
|
|||
info := bindataFileInfo{name: "system.json", size: 30129, mode: os.FileMode(420), modTime: time.Unix(1548273842, 0)} | |||
info := bindataFileInfo{name: "system.json", size: 30129, mode: os.FileMode(420), modTime: time.Unix(1548960115, 0)} |
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 is this? Was it meant to be part of this PR?
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’ll have to revert these changes
if (onDelete) { | ||
onDelete(id) | ||
} | ||
onDelete(id) |
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.
onDelete
is optional, perhaps this should remain in an if
statement.
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 took it out because the button that calls the delete function is also conditionally rendered with the same check, seemed redundant. I can add this guard back in to be extra safe
Closes #12233
ILabel
type from@influxdata/influx
wherever possible, had to touch a lot of files because of thiscreateLabelAJAX
which returns the newly created label for use in inline label creation (must be created the immediately added to a dashboard)createLabel
into two functions:createAndAddLabel
createLabel
Checklist: