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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bec4454
Use templates instead of jQuery in the Debug Commands extension
TomMalbran Apr 3, 2013
2af6b67
Restore strings and fix long urls.
TomMalbran May 2, 2013
96ba63c
Bumping
TomMalbran May 2, 2013
96fd522
Merge with master
TomMalbran May 13, 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
20 changes: 20 additions & 0 deletions src/extensions/default/DebugCommands/htmlContent/language-dialog.html
View file
Open in desktop
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
View file
Open in desktop
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>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you try wrapping this in a div and setting a width here so that the second column (the one with the results) doesn't get pushed to the right for long paths?

I'm currently in the case where I don't see them right ahead and have to scroll to get them to show.

<td>{{value}}</td>
</tr>
{{/perfData}}
</tbody>
</table>
</div>
</div>
164 changes: 52 additions & 112 deletions src/extensions/default/DebugCommands/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,27 @@


/*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"),
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 +91,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 +114,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: 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 @@ -158,80 +141,23 @@ define(function (require, exports, module) {
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);
}

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

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

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

addLocale(label, language);
languages.push({label: 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
Contributor

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
14 changes: 7 additions & 7 deletions src/nls/root/strings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,6 @@ define({
"ERROR_FETCHING_UPDATE_INFO_TITLE" : "Error getting update info",
"ERROR_FETCHING_UPDATE_INFO_MSG" : "There was a problem getting the latest update information from the server. Please make sure you are connected to the internet and try again.",

// Switch language
"LANGUAGE_TITLE" : "Switch Language",
"LANGUAGE_MESSAGE" : "Language:",
"LANGUAGE_SUBMIT" : "Reload {APP_NAME}",
"LANGUAGE_CANCEL" : "Cancel",
"LANGUAGE_SYSTEM_DEFAULT" : "System Default",

/**
* ProjectManager
*/
Expand Down Expand Up @@ -331,6 +324,13 @@ define({
"UNKNOWN_ERROR" : "Unknown internal error.",
// For NOT_FOUND_ERR, see generic strings above


// extensions/default/DebugComments - Switch language
"LANGUAGE_TITLE" : "Switch Language",
"LANGUAGE_MESSAGE" : "Language:",
"LANGUAGE_SUBMIT" : "Reload {APP_NAME}",
"LANGUAGE_SYSTEM_DEFAULT" : "System Default",

Copy link
Contributor

Choose a reason for hiding this comment

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

This changes are currently in conflict. Since we'll be working on #3575, we could just discard this here and take care of it all together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove this, I did this change before the other request where I found that there were a lot of this problems of the strings not being where they should be.

// extensions/default/JSLint
"JSLINT_ERRORS" : "JSLint Errors",
"JSLINT_ERROR_INFORMATION" : "1 JSLint Error",
Expand Down