Fix #4750: It still will switch language if you click the cancel button. #4753
Conversation
| return; | ||
| } else if (locale !== curLocale) { | ||
| Dialogs.showModalDialogUsingTemplate(template).done(function (id) { | ||
| if (id === Dialogs.DIALOG_BTN_OK && locale !== curLocale) { |
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
ghost
assigned 
|
Looks good! Merging. |
Fix #4750: It still will switch language if you click the cancel button.
Fix #4750.
It should only change the language and reload the window when the OK button was pressed, since done only means that the dialog was closed, and not that was accepted.