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

Fix #4750: It still will switch language if you click the cancel button. #4753

Merged
merged 1 commit into from
Aug 16, 2013
Merged

Fix #4750: It still will switch language if you click the cancel button. #4753

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 2 additions & 4 deletions src/extensions/default/DebugCommands/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,8 @@ define(function (require, exports, module) {
});

var template = Mustache.render(LanguageDialogTemplate, {languages: languages, Strings: Strings});
Dialogs.showModalDialogUsingTemplate(template).done(function () {
if (locale === undefined) {
return;
} else if (locale !== curLocale) {
Dialogs.showModalDialogUsingTemplate(template).done(function (id) {
if (id === Dialogs.DIALOG_BTN_OK && locale !== curLocale) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you want to use the localized OK string to compare. Dialogs.DIALOG_BTN_OK is translated for each language so just compare to the non-localized version: "ok".

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we're comparing the button label here. Tom is comparing the id of the button and not the value of the button.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep. I was thinking that Dialogs.DIALOG_BTN_OK was a translated string but it doesn't look like it is used consistently for the button ID. Some dialog templates use this value while others just use "ok" and the extension manager dialog checks against "ok" instead of the constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Dialogs.DIALOG_BTN_OK = "ok" and is the constant that we should use for ok. The only problem I see with our code is that inside the templates we use "ok" instead of the constant. We should start changing this by passing the Dialogs constants to Mustache, so that it also renders that instead. But that should be part of a bigger cleanup.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see it as an issue in using data-button-id="ok" in the template. It is a non-localizable string used as an id for the element and not to be rendered in the UI.

brackets.setLocale(locale);
CommandManager.execute(DEBUG_REFRESH_WINDOW);
}
Expand Down