Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

[ARCH] Improving the Dialog API #3086

Merged
merged 7 commits into from
May 28, 2013
Merged

[ARCH] Improving the Dialog API #3086

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c9ca76b
Improving the Dialog API
TomMalbran Mar 9, 2013
e95df02
Reversing the buttons order
TomMalbran Mar 9, 2013
a58515b
Merged with master. Fixed conflict issues.
TomMalbran May 17, 2013
064b108
Updates to address the review comments
TomMalbran May 18, 2013
f37ebd0
JSHint indentation fixes
TomMalbran May 19, 2013
bd08117
Fixes after review
TomMalbran May 21, 2013
d2e8b02
Merge with master
TomMalbran May 28, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions src/LiveDevelopment/LiveDevelopment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,28 +644,33 @@ define(function LiveDevelopment(require, exports, module) {
Dialogs.showModalDialog(
Dialogs.DIALOG_ID_LIVE_DEVELOPMENT,
Strings.LIVE_DEVELOPMENT_RELAUNCH_TITLE,
Strings.LIVE_DEVELOPMENT_ERROR_MESSAGE
).done(function (id) {
if (id === Dialogs.DIALOG_BTN_OK) {
// User has chosen to reload Chrome, quit the running instance
_setStatus(STATUS_INACTIVE);
NativeApp.closeLiveBrowser()
.done(function () {
browserStarted = false;
window.setTimeout(function () {
open().done(result.resolve).fail(result.reject);
Strings.LIVE_DEVELOPMENT_ERROR_MESSAGE,
[
{ className: "left", id: "cancel", text: Strings.CANCEL },
Copy link
Member

Choose a reason for hiding this comment

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

The button ids should be Dialogs.DIALOG_BTN_*** constants instead of hard-coded strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Do you think we should use constants for the class names too?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, having constants for the class names sounds good.

{ className: "primary", id: "ok", text: Strings.RELAUNCH_CHROME }
]
)
.done(function (id) {
if (id === Dialogs.DIALOG_BTN_OK) {
// User has chosen to reload Chrome, quit the running instance
_setStatus(STATUS_INACTIVE);
NativeApp.closeLiveBrowser()
.done(function () {
browserStarted = false;
window.setTimeout(function () {
open().done(result.resolve).fail(result.reject);
});
})
.fail(function (err) {
// Report error?
_setStatus(STATUS_ERROR);
browserStarted = false;
result.reject("CLOSE_LIVE_BROWSER");
});
})
.fail(function (err) {
// Report error?
_setStatus(STATUS_ERROR);
browserStarted = false;
result.reject("CLOSE_LIVE_BROWSER");
});
} else {
result.reject("CANCEL");
}
});
} else {
result.reject("CANCEL");
}
});
return;
}
retryCount++;
Expand Down
104 changes: 56 additions & 48 deletions src/document/DocumentCommandHandlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@ define(function (require, exports, module) {
var AppInit = require("utils/AppInit"),
CommandManager = require("command/CommandManager"),
Commands = require("command/Commands"),
KeyBindingManager = require("command/KeyBindingManager"),
NativeFileSystem = require("file/NativeFileSystem").NativeFileSystem,
ProjectManager = require("project/ProjectManager"),
DocumentManager = require("document/DocumentManager"),
EditorManager = require("editor/EditorManager"),
FileViewController = require("project/FileViewController"),
FileUtils = require("file/FileUtils"),
StringUtils = require("utils/StringUtils"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This 2 modules aren't required anymore so I removed them.

Async = require("utils/Async"),
Expand Down Expand Up @@ -209,8 +207,6 @@ define(function (require, exports, module) {
// Prompt the user with a dialog
NativeFileSystem.showOpenDialog(true, false, Strings.OPEN_FILE, _defaultOpenDialogFullPath,
null, function (paths) {
var i;

if (paths.length > 0) {
// Add all files to the working set without verifying that
// they still exist on disk (for faster opening)
Expand Down Expand Up @@ -401,7 +397,7 @@ define(function (require, exports, module) {

function handleError(error, fileEntry) {
showSaveFileError(error.name, fileEntry.fullPath)
.always(function () {
.done(function () {
result.reject(error);
});
}
Expand Down Expand Up @@ -581,36 +577,42 @@ define(function (require, exports, module) {
StringUtils.format(
Strings.SAVE_CLOSE_MESSAGE,
StringUtils.breakableUrl(filename)
)
).done(function (id) {
if (id === Dialogs.DIALOG_BTN_CANCEL) {
result.reject();
} else if (id === Dialogs.DIALOG_BTN_OK) {
// "Save" case: wait until we confirm save has succeeded before closing
doSave(doc)
.done(function () {
doClose(file);
result.resolve();
})
.fail(function () {
result.reject();
});
} else {
// "Don't Save" case: even though we're closing the main editor, other views of
// the Document may remain in the UI. So we need to revert the Document to a clean
// copy of whatever's on disk.
doClose(file);

// Only reload from disk if we've executed the Close for real,
// *and* if at least one other view still exists
if (!promptOnly && DocumentManager.getOpenDocumentForPath(file.fullPath)) {
doRevert(doc)
.pipe(result.resolve, result.reject);
),
[
{ className: "left", id: "dontsave", text: Strings.DONT_SAVE },
{ className: "", id: "cancel", text: Strings.CANCEL },
{ className: "primary", id: "ok", text: Strings.SAVE }
]
)
.done(function (id) {
if (id === Dialogs.DIALOG_BTN_CANCEL) {
result.reject();
} else if (id === Dialogs.DIALOG_BTN_OK) {
// "Save" case: wait until we confirm save has succeeded before closing
doSave(doc)
.done(function () {
doClose(file);
result.resolve();
})
.fail(function () {
result.reject();
});
} else {
result.resolve();
// "Don't Save" case: even though we're closing the main editor, other views of
// the Document may remain in the UI. So we need to revert the Document to a clean
// copy of whatever's on disk.
doClose(file);

// Only reload from disk if we've executed the Close for real,
// *and* if at least one other view still exists
if (!promptOnly && DocumentManager.getOpenDocumentForPath(file.fullPath)) {
doRevert(doc)
.pipe(result.resolve, result.reject);
} else {
result.resolve();
}
}
}
});
});
result.always(function () {
EditorManager.focusEditor();
});
Expand Down Expand Up @@ -673,22 +675,28 @@ define(function (require, exports, module) {
Dialogs.showModalDialog(
Dialogs.DIALOG_ID_SAVE_CLOSE,
Strings.SAVE_CLOSE_TITLE,
message
).done(function (id) {
if (id === Dialogs.DIALOG_BTN_CANCEL) {
result.reject();
} else if (id === Dialogs.DIALOG_BTN_OK) {
// Save all unsaved files, then if that succeeds, close all
saveAll().done(function () {
result.resolve();
}).fail(function () {
message,
[
{ className: "left", id: "dontsave", text: Strings.DONT_SAVE },
{ className: "", id: "cancel", text: Strings.CANCEL },
{ className: "primary", id: "ok", text: Strings.SAVE }
]
)
.done(function (id) {
if (id === Dialogs.DIALOG_BTN_CANCEL) {
result.reject();
});
} else {
// "Don't Save" case--we can just go ahead and close all files.
result.resolve();
}
});
} else if (id === Dialogs.DIALOG_BTN_OK) {
// Save all unsaved files, then if that succeeds, close all
saveAll().done(function () {
result.resolve();
}).fail(function () {
result.reject();
});
} else {
// "Don't Save" case--we can just go ahead and close all files.
result.resolve();
}
});
}

// If all the unsaved-changes confirmations pan out above, then go ahead & close all editors
Expand Down
2 changes: 1 addition & 1 deletion src/extensibility/ExtensionManagerDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ define(function (require, exports, module) {
// Open the dialog.
Dialogs.showModalDialogUsingTemplate(
Mustache.render(dialogTemplate, Strings)
).always(function () {
).done(function () {
view.dispose();
});

Expand Down
25 changes: 17 additions & 8 deletions src/extensibility/ExtensionManagerView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,15 @@ define(function (require, exports, module) {

// If an extension was removed, prompt the user to quit Brackets.
if (!_skipRemoval && this.model.hasExtensionsToRemove()) {
Dialogs.showModalDialog("remove-marked-extensions", Strings.REMOVE_AND_QUIT_TITLE,
Strings.REMOVE_AND_QUIT_MESSAGE)
Dialogs.showModalDialog(
Dialogs.DIALOG_ID_REMOVE_EXTENSIONS,
Strings.REMOVE_AND_QUIT_TITLE,
Strings.REMOVE_AND_QUIT_MESSAGE,
[
{ className: "", id: "cancel", text: Strings.CANCEL },
{ className: "primary", id: "ok", text: Strings.REMOVE_AND_QUIT }
]
)
.done(function (buttonId) {
if (buttonId === "ok") {
self.model.removeMarkedExtensions()
Expand All @@ -287,12 +294,14 @@ define(function (require, exports, module) {
errorArray.forEach(function (errorObj) {
ids.push(errorObj.item);
});
Dialogs.showModalDialog("error-dialog", Strings.EXTENSION_MANAGER_REMOVE,
StringUtils.format(Strings.EXTENSION_MANAGER_REMOVE_ERROR, ids.join(", ")))
.done(function () {
// We still have to quit even if some of the removals failed.
CommandManager.execute(Commands.FILE_QUIT);
});
Dialogs.showModalDialog(
Dialogs.DIALOG_ID_ERROR,
Strings.EXTENSION_MANAGER_REMOVE,
StringUtils.format(Strings.EXTENSION_MANAGER_REMOVE_ERROR, ids.join(", "))
).done(function () {
// We still have to quit even if some of the removals failed.
CommandManager.execute(Commands.FILE_QUIT);
});
});
} else {
self.model.dispose();
Expand Down
7 changes: 1 addition & 6 deletions src/extensibility/InstallExtensionDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,7 @@ define(function (require, exports, module) {

// We ignore the promise returned by showModalDialogUsingTemplate, since we're managing the
// lifecycle of the dialog ourselves.
Dialogs.showModalDialogUsingTemplate(
Mustache.render(InstallDialogTemplate, Strings),
null,
null,
false
);
Dialogs.showModalDialogUsingTemplate(Mustache.render(InstallDialogTemplate, Strings), false);

this.$dlg = $(".install-extension-dialog.instance");
this.$url = this.$dlg.find(".url").focus();
Expand Down
24 changes: 1 addition & 23 deletions src/extensions/samples/LocalizationExample/htmlContent/sampleHTMLFragment.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,4 @@
<!--
Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved.

Permission is hereby granted, free of charge, to any person obtaining a
copy of this software and associated documentation files (the "Software"),
to deal in the Software without restriction, including without limitation
the rights to use, copy, modify, merge, publish, distribute, sublicense,
and/or sell copies of the Software, and to permit persons to whom the
Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN THE SOFTWARE.
-->

<div class="sample-localized-dialog template modal hide">
<div class="sample-localized-dialog modal">
<div class="modal-header">
<h1 class="dialog-title">{{DIALOG_TITLE}}</h1>
</div>
Expand Down
8 changes: 3 additions & 5 deletions src/extensions/samples/LocalizationExample/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ define(function (require, exports, module) {
function testCommand() {
alert(Strings.ALERT_MESSAGE);

Dialogs.showModalDialog("sample-localized-dialog");
// Localize the dialog using Strings as the datasource and use it as the dialog template
var localizedTemplate = Mustache.render(browserWrapperHtml, Strings);
Dialogs.showModalDialogUsingTemplate(localizedTemplate);
}


Expand All @@ -68,8 +70,4 @@ define(function (require, exports, module) {

var menu = Menus.getMenu(Menus.AppMenuBar.EDIT_MENU);
menu.addMenuItem(myCommandID, null, Menus.AFTER, myCommandID);

// Localize the dialog using Strings as the datasource and insert it into the DOM
var localizedHTML = $(Mustache.render(browserWrapperHtml, Strings));
$('body').append(localizedHTML);
});
2 changes: 1 addition & 1 deletion src/htmlContent/about-dialog.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ <h2>{{APP_NAME_ABOUT_BOX}}</h2>
</div>
</div>
<div class="modal-footer">
<a href="#" class="dialog-button btn primary" data-button-id="ok">{{CLOSE}}</a>
<button class="dialog-button btn primary" data-button-id="ok">{{CLOSE}}</button>
</div>
</div>
14 changes: 14 additions & 0 deletions src/htmlContent/dialog-template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<div class="{{dlgClass}} template modal">
<div class="modal-header">
<a href="#" class="close">&times;</a>
<h1 class="dialog-title">{{{title}}}</h1>
</div>
<div class="modal-body">
<p class="dialog-message">{{{message}}}</p>
</div>
<div class="modal-footer">
{{#buttons}}
<button class="dialog-button btn {{className}}" data-button-id="{{id}}">{{{text}}}</button>
{{/buttons}}
</div>
</div>
Loading