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

[OPEN] Move the Project Preferences Dialog variables to the template #3286

Merged
merged 4 commits into from
Jun 17, 2013
Merged

[OPEN] Move the Project Preferences Dialog variables to the template #3286

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
d5ea90b
Move the project preference dialog variables to the template
TomMalbran Mar 29, 2013
231d0db
Merge with Master
TomMalbran Jun 14, 2013
be5e767
Use 2 curly braces instead of 3 for the error message
TomMalbran Jun 14, 2013
79bbd1e
Pass Strings as a reference instead of copying it and re-add the 3 cu…
TomMalbran Jun 15, 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
9 changes: 6 additions & 3 deletions src/htmlContent/project-settings-dialog.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
<div class="project-settings-dialog modal">
<div class="modal-header">
<h1 class="dialog-title"></h1>
<h1 class="dialog-title">{{title}}</h1>
</div>
<div class="modal-body">
<div class="field-container">
<label>{{PROJECT_SETTING_BASE_URL}}: <input type="text" placeholder="{{PROJECT_SETTING_BASE_URL_HINT}}" class="url" /></label>
</div>
<label>
{{PROJECT_SETTING_BASE_URL}}: <input type="text" placeholder="{{PROJECT_SETTING_BASE_URL_HINT}}" value="{{baseUrl}}" class="url" />
</label>
{{#errorMessage}}<div class="alert-message" style="margin-bottom: 0">{{{errorMessage}}}</div>{{/errorMessage}}
Copy link
Contributor

@redmunds redmunds Jun 13, 2013

Choose a reason for hiding this comment

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

Why does {{{errorMessage}}} have triple (not double) curly braces?

Copy link
Contributor Author

@TomMalbran TomMalbran Jun 14, 2013

Choose a reason for hiding this comment

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

If I remember right, I think that previously the error message string was using &mdash; for the dash which required the 3 curly braces to display it correctly, but now it doesn't use it, so replaced them with 2.

</div>
</div>
<div class="modal-footer">
<a href="#" class="dialog-button btn primary" data-button-id="ok">{{OK}}</a>
Expand Down
51 changes: 20 additions & 31 deletions src/preferences/PreferencesDialogs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,26 @@ define(function (require, exports, module) {
* project settings and clicks OK, or rejected if user clicks Cancel.
*/
function showProjectPreferencesDialog(baseUrl, errorMessage) {

var $dlg,
$title,
$baseUrlControl,
var $baseUrlControl,
promise;

promise = Dialogs.showModalDialogUsingTemplate(Mustache.render(SettingsDialogTemplate, Strings))

// Title
var projectName = "",
projectRoot = ProjectManager.getProjectRoot(),
title;
if (projectRoot) {
projectName = projectRoot.name;
}
title = StringUtils.format(Strings.PROJECT_SETTINGS_TITLE, projectName);

var templateVars = $.extend({
title : title,
baseUrl : baseUrl,
errorMessage : errorMessage
}, Strings);
Copy link
Contributor

@redmunds redmunds Jun 14, 2013

Choose a reason for hiding this comment

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

The Strings Object is big (and getting bigger all of the time), so I am wondering if we should avoid copying the entire Strings Object here for only 4 strings. Maybe this would be better:

    var templateVars = {
        title                         : title,
        baseUrl                       : baseUrl,
        errorMessage                  : errorMessage,
        PROJECT_SETTING_BASE_URL      : Strings.PROJECT_SETTING_BASE_URL,
        PROJECT_SETTING_BASE_URL_HINT : Strings.PROJECT_SETTING_BASE_URL_HINT,
        CANCEL                        : Strings.CANCEL,
        OK                            : Strings.OK
    };

Copy link

@njx njx Jun 14, 2013

Choose a reason for hiding this comment

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

Another trick I've used in similar situations is just to include Strings: Strings in the template vars, and then use {{Strings.FOO_BAR}} in the template (since Mustache handles dot expressions now).

Copy link
Contributor Author

@TomMalbran TomMalbran Jun 15, 2013

Choose a reason for hiding this comment

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

I would prefer @njx idea of passing the object and using the dot notation to make the template easier to expand later, since you don't need to keep adding the new strings to it.

console.log(templateVars, Mustache.render(SettingsDialogTemplate, templateVars));

promise = Dialogs.showModalDialogUsingTemplate(Mustache.render(SettingsDialogTemplate, templateVars))
.done(function (id) {
if (id === Dialogs.DIALOG_BTN_OK) {
var baseUrlValue = $baseUrlControl.val();
Expand All @@ -100,32 +113,8 @@ define(function (require, exports, module) {
}
});

// Populate project settings
$dlg = $(".project-settings-dialog.instance");

// Title
$title = $dlg.find(".dialog-title");
var projectName = "",
projectRoot = ProjectManager.getProjectRoot(),
title;
if (projectRoot) {
projectName = projectRoot.name;
}
title = StringUtils.format(Strings.PROJECT_SETTINGS_TITLE, projectName);
$title.text(title);

// Base URL
$baseUrlControl = $dlg.find(".url");
if (baseUrl) {
$baseUrlControl.val(baseUrl);
}

// Error message
if (errorMessage) {
$dlg.find(".field-container").append("<div class='alert-message' style='margin-bottom: 0'>" + errorMessage + "</div>");
}

// Give focus to first control
$baseUrlControl = $(".project-settings-dialog.instance").find(".url");
$baseUrlControl.focus();

return promise;
Expand Down