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

Use templates instead of jQuery in the Debug Commands extension #3336

Merged
merged 4 commits into from
May 13, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<div class="switch-language modal">
<div class="modal-header">
<a href="#" class="close">&times;</a>
<h1 class="dialog-title">{{LANGUAGE_TITLE}}</h1>
</div>
<div class="modal-body">
<p class="dialog-message">
{{LANGUAGE_MESSAGE}}
<select>
{{#languages}}
<option value="{{language}}">{{label}}</option>
{{/languages}}
</select>
</p>
</div>
<div class="modal-footer">
<button class="dialog-button btn primary" data-button-id="ok" disabled>{{LANGUAGE_SUBMIT}}</button>
<button class="dialog-button btn left" data-button-id="cancel">{{CANCEL}}</button>
</div>
</div>
23 changes: 23 additions & 0 deletions src/extensions/default/DebugCommands/htmlContent/perf-dialog.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<div class="modal">
<div class="modal-header">
<a href="#" class="close">&times;</a>
<h1 class="dialog-title">Performance Data</h1>
<div style="text-align: right">
Raw data (copy paste out):
Copy link
Contributor

Choose a reason for hiding this comment

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

These strings could be localized. This is a very specific dialog though, so not sure if it's really worth it, or if it's even here to stay. @jasonsanjose what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Skip it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@TomMalbran you heard the man ;)

<textarea rows="1" style="width:30px; height:8px; overflow: hidden; resize: none" id="brackets-perf-raw-data">{{delimitedPerfData}}</textarea>
</div>
</div>
<div class="modal-body" style="padding: 0; max-height: 500px; overflow: auto; width: 592px">
<table class="zebra-striped condensed-table">
<thead><th>Operation</th><th>Time (ms)</th></thead>
<tbody>
{{#perfData}}
<tr>
<td>{{{testName}}}</td>
<td>{{value}}</td>
</tr>
{{/perfData}}
</tbody>
</table>
</div>
</div>
170 changes: 55 additions & 115 deletions src/extensions/default/DebugCommands/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,28 @@


/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */
/*global define, $, brackets, window, WebSocket */
/*global define, $, brackets, window, WebSocket, Mustache */

define(function (require, exports, module) {
"use strict";

var Commands = brackets.getModule("command/Commands"),
CommandManager = brackets.getModule("command/CommandManager"),
Editor = brackets.getModule("editor/Editor").Editor,
FileUtils = brackets.getModule("file/FileUtils"),
KeyBindingManager = brackets.getModule("command/KeyBindingManager"),
Menus = brackets.getModule("command/Menus"),
Strings = brackets.getModule("strings"),
PerfUtils = brackets.getModule("utils/PerfUtils"),
ProjectManager = brackets.getModule("project/ProjectManager"),
NativeApp = brackets.getModule("utils/NativeApp"),
NativeFileSystem = brackets.getModule("file/NativeFileSystem").NativeFileSystem,
NodeDebugUtils = require("NodeDebugUtils");
var Commands = brackets.getModule("command/Commands"),
CommandManager = brackets.getModule("command/CommandManager"),
KeyBindingManager = brackets.getModule("command/KeyBindingManager"),
Menus = brackets.getModule("command/Menus"),
Editor = brackets.getModule("editor/Editor").Editor,
FileUtils = brackets.getModule("file/FileUtils"),
NativeFileSystem = brackets.getModule("file/NativeFileSystem").NativeFileSystem,
ProjectManager = brackets.getModule("project/ProjectManager"),
PerfUtils = brackets.getModule("utils/PerfUtils"),
NativeApp = brackets.getModule("utils/NativeApp"),
CollectionUtils = brackets.getModule("utils/CollectionUtils"),
StringUtils = brackets.getModule("utils/StringUtils"),
Dialogs = brackets.getModule("widgets/Dialogs"),
Strings = brackets.getModule("strings"),
NodeDebugUtils = require("NodeDebugUtils"),
PerfDialogTemplate = require("text!htmlContent/perf-dialog.html"),
LanguageDialogTemplate = require("text!htmlContent/language-dialog.html");

var KeyboardPrefs = JSON.parse(require("text!keyboard.json"));

Expand Down Expand Up @@ -87,20 +92,9 @@ define(function (require, exports, module) {
}

function _handleShowPerfData() {
var $perfHeader = $("<div class='modal-header' />")
.append("<a href='#' class='close'>&times;</a>")
.append("<h1 class='dialog-title'>Performance Data</h1>")
.append("<div align=right>Raw data (copy paste out): <textarea rows=1 style='width:30px; height:8px; overflow: hidden; resize: none' id='brackets-perf-raw-data'>" + PerfUtils.getDelimitedPerfData() + "</textarea></div>");

var $perfBody = $("<div class='modal-body' style='padding: 0; max-height: 500px; overflow: auto;' />");

var $data = $("<table class='zebra-striped condensed-table'>")
.append("<thead><th>Operation</th><th>Time (ms)</th></thead>")
.append("<tbody />")
.appendTo($perfBody);

var makeCell = function (content) {
return $("<td/>").text(content);
var templateVars = {
delimitedPerfData: PerfUtils.getDelimitedPerfData(),
perfData: []
};

var getValue = function (entry) {
Expand All @@ -121,28 +115,18 @@ define(function (require, exports, module) {
return entry;
}
};

var testName;

var perfData = PerfUtils.getData();
for (testName in perfData) {
if (perfData.hasOwnProperty(testName)) {
// Add row to error table
$("<tr/>")
.append(makeCell(testName))
.append(makeCell(getValue(perfData[testName])))
.appendTo($data);
}
}

$("<div class='modal hide' />")
.append($perfHeader)
.append($perfBody)
.appendTo(window.document.body)
.modal({
backdrop: "static",
show: true
CollectionUtils.forEach(perfData, function (value, testName) {
templateVars.perfData.push({
testName: StringUtils.breakableUrl(testName),
value: getValue(value)
});

});

var template = Mustache.render(PerfDialogTemplate, templateVars);
Dialogs.showModalDialogUsingTemplate(template);

// Select the raw perf data field on click since select all doesn't
// work outside of the editor
$("#brackets-perf-raw-data").click(function () {
Expand All @@ -153,95 +137,37 @@ define(function (require, exports, module) {
function _handleNewBracketsWindow() {
window.open(window.location.href);
}

function _handleSwitchLanguage() {
var stringsPath = FileUtils.getNativeBracketsDirectoryPath() + "/nls";
NativeFileSystem.requestNativeFileSystem(stringsPath, function (fs) {
fs.root.createReader().readEntries(function (entries) {

var $submit,
var $dialog,
$submit,
$select,
locale,
curLocale = (brackets.isLocaleDefault() ? null : brackets.getLocale());
curLocale = (brackets.isLocaleDefault() ? null : brackets.getLocale()),
languages = [];

function setLanguage(event) {
locale = $select.val();
$submit.attr("disabled", false);
}

var $modal = $("<div class='modal hide' />");

var $header = $("<div class='modal-header' />")
.append("<a href='#' class='close'>&times;</a>")
.append("<h1 class='dialog-title'>" + Strings.LANGUAGE_TITLE + "</h1>")
.appendTo($modal);

var $body = $("<div class='modal-body' style='max-height: 500px; overflow: auto;' />")
.appendTo($modal);

var $p = $("<p class='dialog-message'>")
.text(Strings.LANGUAGE_MESSAGE)
.appendTo($body);

$select = $("<select>")
.on("change", setLanguage)
.appendTo($p);

var $footer = $("<div class='modal-footer' />")
.appendTo($modal);

var $cancel = $("<button class='dialog-button btn left'>")
.on("click", function () {
$modal.modal('hide');
})
.text(Strings.LANGUAGE_CANCEL)
.appendTo($footer);

$submit = $("<button class='dialog-button btn primary'>")
.text(Strings.LANGUAGE_SUBMIT)
.on("click", function () {
if (locale === undefined) {
return;
} else if (locale !== curLocale) {
brackets.setLocale(locale);
CommandManager.execute(DEBUG_REFRESH_WINDOW);
}
})
.attr("disabled", "disabled")
.appendTo($footer);

$modal
.appendTo(window.document.body)
.modal({
backdrop: "static",
show: true
})
.on("hidden", function () {
$(this).remove();
});

function addLocale(text, val) {
var $option = $("<option>")
.text(text)
.val(val)
.appendTo($select);
}

// returns the localized label for the given locale
// or the locale, if nothing found
function getLocalizedLabel(locale) {

var key = "LOCALE_" + locale.toUpperCase().replace("-", "_"),
var key = "LOCALE_" + locale.toUpperCase().replace("-", "_"),
i18n = Strings[key];

return i18n === undefined ? locale : i18n;
}

// add system default
addLocale(Strings.LANGUAGE_SYSTEM_DEFAULT, null);
languages.push({label: Strings.LANGUAGE_SYSTEM_DEFAULT, language: null});

// add english
addLocale(getLocalizedLabel("en"), "en");
languages.push({label: getLocalizedLabel("en"), language: "en"});

// inspect all children of dirEntry
entries.forEach(function (entry) {
Expand All @@ -256,12 +182,26 @@ define(function (require, exports, module) {
label += match[2].toUpperCase();
}

addLocale(getLocalizedLabel(label), language);
languages.push({label: getLocalizedLabel(label), language: language});
}
}
});

$select.val(curLocale);
var template = Mustache.render(LanguageDialogTemplate, $.extend({languages: languages}, Strings));
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a little bit overkill to pass the full Strings object here since we're just using 4 of them. Also, if we use $.extend, I think Strings should go in the first place to avoid extra operations (not that it should be noticeable, but better to iterate through less keys).

I really like that with this approach, the strings in the template are called like the variables in the string files. It makes it very easy to find them later.

Copy link

Choose a reason for hiding this comment

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

The problem with Strings being in the first place is that it will actually modify the Strings object, which we don't want.

We've used this pattern elsewhere (and I had the same concern initially, but it doesn't seem to be that much of a performance impact). If we're concerned about performance, the right fix is probably to have a small convenience function that copies over a specified subset of keys.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, you're right! I knew we'll be modifying the Strings object but didn't realize that we'll be modifying THE Strings object!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It makes sense that is the object, because to not be it, it would have to copy all the strings, and that isn't a cheap operation, but we are still doing later when using this extend pattern.

Another alternative once it becomes a problem is to split the strings into extensions/dialogs/teplates so that then you could just load the strings you want and not all.

Dialogs.showModalDialogUsingTemplate(template).done(function () {
if (locale === undefined) {
return;
} else if (locale !== curLocale) {
brackets.setLocale(locale);
CommandManager.execute(DEBUG_REFRESH_WINDOW);
}
});

$dialog = $(".switch-language.instance");
$submit = $dialog.find(".dialog-button[data-button-id='ok']");
$select = $dialog.find("select");

$select.on("change", setLanguage).val(curLocale);
});
});
}
Expand Down