Skip to content
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

Change error dialogs #450

Merged
merged 13 commits into from
May 12, 2017
Merged

Change error dialogs #450

merged 13 commits into from
May 12, 2017

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented May 12, 2017

  • Remove old dialogs and use modal dialogs generated by Vue in ApplicationListView and ApplicationView (when start or stop fails).
  • ConfirmDialog component now uses the adminlte classes instead of custom CSS classes.
  • ConfirmDialog component now display a Cancel button only if an associated callback is defined.

@codecov-io
Copy link

codecov-io commented May 12, 2017

Codecov Report

Merging #450 into switch_to_vue will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##           switch_to_vue     #450   +/-   ##
==============================================
  Coverage          95.33%   95.33%           
==============================================
  Files                 88       88           
  Lines               4077     4077           
  Branches             259      259           
==============================================
  Hits                3887     3887           
  Misses               138      138           
  Partials              52       52

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f60908...c90095d. Read the comment docs.

@@ -175,17 +174,17 @@ define([

var url_id = appStopping.appData.container.url_id;

resources.Container.delete(url_id)
return resources.Container.delete(url_id)
.done(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't have to do a done. Let the calling client handle the failure.

@@ -154,18 +153,18 @@ define([
configurablesData[tag] = configurable.asConfigDict();
});

Copy link
Contributor

Choose a reason for hiding this comment

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

create a promise. if you hit any of the fail, you resolve the promise by reject. otherwise, you say success.

mapping_id: currentApp.appData.mapping_id,
configurables: configurablesData
}).done(function() {
this.updateIdx(selectedIndex)
.fail(function(error) {
currentApp.status = Status.STOPPED;
dialogs.webapi_error_dialog(error);
return error;
Copy link
Contributor

Choose a reason for hiding this comment

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

you have to add a done here to resolve the promise

'<section class="sidebar">' +
'<section class="sidebar">' +
' <!-- Error dialog -->' +
' <confirm-dialog v-if="showErrorDialog"' +
Copy link
Contributor

Choose a reason for hiding this comment

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

{
errorDialog: {
visible: true
}
}

'<section class="sidebar">' +
' <!-- Error dialog -->' +
' <confirm-dialog v-if="showErrorDialog"' +
' :title="getErrorTitle()"' +
Copy link
Contributor

Choose a reason for hiding this comment

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

you just need {"whatever string "+errorMessage}

return { 'searchInput': '' };
return {
'searchInput': '',
showErrorDialog: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

errorDialog: {
show: true,
message: ""
}

var stoppingAppName = this.$options.filters.appName(
this.model.appList[index].appData.image);
this.model.stopApplication(index).fail(function(error) {
this.stoppingError = error;
Copy link
Contributor

Choose a reason for hiding this comment

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

changes here...

this.showErrorDialog = true;
}.bind(this));
},
getErrorTitle: function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

goes away

@@ -11,6 +12,16 @@ define([
'<section id="appview"' +
' v-if="currentApp !== null"' +
' :class="{ content: true, \'no-padding\': currentApp.isRunning() }">' +
' <!-- Error dialog -->' +
' <confirm-dialog v-if="showErrorDialog"' +
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

And send the startApplication event to GA only if the start was a success
@stefanoborini stefanoborini merged commit fe6ebe6 into switch_to_vue May 12, 2017
@stefanoborini stefanoborini deleted the remove_old_dialogs branch May 12, 2017 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants