Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

Add indicator badge when deleting service instance #1159

Merged
merged 3 commits into from
Jul 17, 2017

Conversation

el-mapache
Copy link
Contributor

Previously, we didn't give the user any feedback after they clicked the
'confirm delete' button.

Handy gif reference:
delete-service-instance

Previously, we didn't give the user any feedback after they clicked the
'confirm delete' button
};
}

const propTypes = {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put this after the imports. (at the top)

* Add function to create and add service instance to reduce duplicate
code
* Update specs I touched to ES^ to improve code style consistency
@el-mapache el-mapache force-pushed the ab-confirm-service-instance-delete branch from 935f539 to 3a0fa76 Compare July 14, 2017 20:20
@jcscottiii
Copy link
Contributor

we need a way to handle the error case of deleting the service. (should turn off _updating). (it's a lot of stuff to add so if you want to make a separate PR, you can do that.) let me know your thoughts.

@el-mapache
Copy link
Contributor Author

el-mapache commented Jul 14, 2017

@jcscottiii I can add a test to the actions that verifies that the same method is called from both then and catch outcomes of the promise. That will be pretty simple.

We don't really have any kind of error handling around this action, so I would definitely want to have any of that work in a separate ticket/PR

@jcscottiii
Copy link
Contributor

@el-mapache that would be really good!

@el-mapache el-mapache force-pushed the ab-confirm-service-instance-delete branch from c394677 to 559eea6 Compare July 17, 2017 14:15
@el-mapache
Copy link
Contributor Author

@jcscottiii Test file updated!

@jcscottiii
Copy link
Contributor

jcscottiii commented Jul 17, 2017

@el-mapache could you make a card for taking care of the error handling in a separate PR? (you can put it in for this sprint)

@jcscottiii jcscottiii merged commit b534ec6 into master Jul 17, 2017
@jcscottiii jcscottiii deleted the ab-confirm-service-instance-delete branch July 17, 2017 15:03
@el-mapache
Copy link
Contributor Author

@jcscottiii Definitely!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants