[Chromoting] Improve the text on the buttons in the connection-failed dialog.

[Chromoting] Improve the text on the buttons in the connection-failed dialog.

BUG=117300
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128269

[simonmorris, 2012-03-21 21:57:20]

ptal

[Jamie, 2012-03-21 22:52:12]

Having OK and Cancel reversed feels wrong, both because it's unconventional and
the "reconnect" button moves depending on whether you've connected or not. I
think that's likely to be confusing. I'm also not a fan of OK as a button label
that will be displayed below (semi-)arbitrary text. It's too generic. How about,
the following for a failed connection:

Connect failed.
Retry   Cancel

Versus the following for an interrupted one:

Connection closed
Reconnect  OK

(The distinction between Retry and Reconnect may be unnecessary). Of course, OK
has the same problem here--something like Done or Finished may be more
appropriate.

http://codereview.chromium.org/9827002/diff/1/remoting/webapp/client_screen.js
File remoting/webapp/client_screen.js (right):

http://codereview.chromium.org/9827002/diff/1/remoting/webapp/client_screen.j...
remoting/webapp/client_screen.js:506: setConnectionFailedButtonsText_(false);
This isn't strictly necessary because the HTML has already been localized with
the correct values. I think I'd prefer to rely on that than to pass a Boolean to
setConnectionFailedButtonText.

[simonmorris, 2012-03-22 00:02:42]

I've made the changes you suggested, except that I haven't replaced RECONNECT by
RETRY, partly because that involves changing the string table, and partly
because the unused string name checker doesn't seem to check files other than
main.html.

http://codereview.chromium.org/9827002/diff/1/remoting/webapp/client_screen.js
File remoting/webapp/client_screen.js (right):

http://codereview.chromium.org/9827002/diff/1/remoting/webapp/client_screen.j...
remoting/webapp/client_screen.js:506: setConnectionFailedButtonsText_(false);
On 2012/03/21 22:52:13, Jamie wrote:
> This isn't strictly necessary because the HTML has already been localized with
> the correct values. I think I'd prefer to rely on that than to pass a Boolean
to
> setConnectionFailedButtonText.

Done.

[Jamie, 2012-03-22 04:24:05]

LGTM once the i18n annotation has gone in.

I think I still have a slight preference for Retry over Reconnect in the
"failed" case, so if you were just holding off because you weren't sure how to
get the script to check this file, please make that change too. If you disagree
on aesthetic grounds, then my preference is not strong enough to argue the
point.

http://codereview.chromium.org/9827002/diff/5001/remoting/webapp/client_scree...
File remoting/webapp/client_screen.js (right):

http://codereview.chromium.org/9827002/diff/5001/remoting/webapp/client_scree...
remoting/webapp/client_screen.js:357: l10n.localizeElementFromTag(button1,
'RECONNECT');
Please add /*i18n-content*/ to this so that the script will pick it up correctly
(you might also need to modify remoting.gyp to make sure that this file is
included in the checked set. The script is not very smart about spacing--look
elsewhere in JS files to see what it expects.

http://codereview.chromium.org/9827002/diff/5001/remoting/webapp/client_scree...
remoting/webapp/client_screen.js:360: l10n.localizeElementFromTag(button2,
'OK');
Ditto

[simonmorris, 2012-03-22 17:08:25]

I've changed "reconnect" to "retry", so please check my changes to
messages.json.

http://codereview.chromium.org/9827002/diff/5001/remoting/webapp/client_scree...
File remoting/webapp/client_screen.js (right):

http://codereview.chromium.org/9827002/diff/5001/remoting/webapp/client_scree...
remoting/webapp/client_screen.js:357: l10n.localizeElementFromTag(button1,
'RECONNECT');
On 2012/03/22 04:24:05, Jamie wrote:
> Please add /*i18n-content*/ to this so that the script will pick it up
correctly
> (you might also need to modify remoting.gyp to make sure that this file is
> included in the checked set. The script is not very smart about spacing--look
> elsewhere in JS files to see what it expects.

Done.

http://codereview.chromium.org/9827002/diff/5001/remoting/webapp/client_scree...
remoting/webapp/client_screen.js:360: l10n.localizeElementFromTag(button2,
'OK');
On 2012/03/22 04:24:05, Jamie wrote:
> Ditto

Done.

[Jamie, 2012-03-22 17:10:35]

lgtm

[commit-bot: I haz the power, 2012-03-22 18:01:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonmorris@chromium.org/9827002/9003

[commit-bot: I haz the power, 2012-03-22 19:40:42]

Change committed as 128269