Aerogear 3466 delete datasource #27
Conversation
| variables: { dataSourceId }, | ||
| refetchQueries: [{ | ||
| query: GetDataSources, | ||
| variables: { name: "" } |
| <MenuItem | ||
| eventKey={2} | ||
| onSelect={() => { | ||
| if (window.confirm("Are you sure you wish to delete this item?")) { |
There was a problem hiding this comment.
We should use a patternfly confirmation box here: https://rawgit.com/patternfly/patternfly-react/gh-pages/index.html?selectedKind=patternfly-react%2FCommunication%2FMessage%20Dialog&selectedStory=Question%20Dialog%20Example&full=0&addons=1&stories=1&panelRight=0&addonPanel=storybooks%2Fstorybook-addon-knobs
There was a problem hiding this comment.
Agree, this component shouldn't contain specific graphql logic (otherwise edit should be here as well). Let's add a new component in DataSourcesContainer that renders patternfly's MessageDialog and add this logic inside.
You can take a look at how's implemented in #29 (the same but with a Modal):
in DataSourcesList
in DataSourcesListItem
Important also not to change the export mode data-sources/index
| variables: { dataSourceId }, | ||
| refetchQueries: [{ | ||
| query: GetDataSources, | ||
| variables: { name: undefined } |
There was a problem hiding this comment.
@pb82
wasn't this have to be "" instead?
There was a problem hiding this comment.
No, "" is interpreted as a filter value while undefined returns all items.
| } | ||
|
|
||
| removeOneDatasource(dataSourceId) { | ||
| const self = this; |
There was a problem hiding this comment.
ES6 takes care of the scope for you. Whenever you are in a class, you can forget about this.
|
|
||
| class DataSourcesListItem extends Component { | ||
|
|
||
| constructor(props) { //eslint-disable-line |
There was a problem hiding this comment.
Don't disable and remove the constructor instead
| } | ||
|
|
||
| render() { | ||
| const self = this; |
| import DeleteDataSource from "../../graphql/DeleteDataSource.graphql"; | ||
| import GetDataSources from "../../graphql/GetDataSources.graphql"; | ||
|
|
||
| class DataSourcesListItem extends Component { |
There was a problem hiding this comment.
Just a comment: unless you need to make use of the State, you don't need a class component. In this case, better stick with the functional component.
cfoskin
force-pushed
the
AEROGEAR-3466-delete-datasource
branch
from
4e46682
to
b900c86
Compare
|
ping @pb82 @josemigallas updated pr with restructure |
| }).then(() => { | ||
| onClose(); | ||
| }).catch(err => { | ||
| console.log("Error", err); |
cfoskin
force-pushed
the
AEROGEAR-3466-delete-datasource
branch
from
db07249
to
d7e6e83
Compare
JIRA: https://issues.jboss.org/browse/AEROGEAR-3466
Deletes the chosen data source when this option is selected from the dropdown
Provides a confirm window before deleting