Show destination offline status in the print preview.

Show destination offline status in the print preview.

BUG=143230
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251457

[Vitaly Buka (NO REVIEWS), 2014-02-12 00:39:18]

lgtm

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/data/destination.js (right):

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/data/destination.js:255: this.location_ =
'';
Why do we initialize this in null in first place?

[Toscano, 2014-02-12 22:10:29]

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/data/destination.js (right):

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/data/destination.js:255: this.location_ =
'';
On 2014/02/12 00:39:19, Vitaly Buka wrote:
> Why do we initialize this in null in first place?

Agreed, I think that would be simpler.

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/search/destination_list_item.css
(left):

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/search/destination_list_item.css:38:
opacity: 0.6;
Not sure I understand the removal of this line. Can you explain?

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/settings/destination_settings.js
(right):

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/destination_settings.js:127: var
offlineStatusEl = this.getElement().getElementsByClassName(
Why not use this.getChildElement()?

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/destination_settings.js:134:
this.getElement().querySelector('.destination-settings-box');
Why not use this.getChildElement()?

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/destination_settings.js:141: }
What about simplifying this if-else statement to:

destinationSettingsBoxEl.classList.toggle(
    DestinationSettings.Classes_.STALE, isOffline);

[Aleksey Shlyapnikov, 2014-02-13 19:10:18]

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/data/destination.js (right):

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/data/destination.js:255: this.location_ =
'';
On 2014/02/12 22:10:29, Toscano wrote:
> On 2014/02/12 00:39:19, Vitaly Buka wrote:
> > Why do we initialize this in null in first place?
> 
> Agreed, I think that would be simpler.

Null indicates that we have not tried to look it up yet. Empty string means that
there's no location specified for this printer and we won't run this for loop
next time get location is called.

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/search/destination_list_item.css
(left):

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/search/destination_list_item.css:38:
opacity: 0.6;
On 2014/02/12 22:10:29, Toscano wrote:
> Not sure I understand the removal of this line. Can you explain?

The entire item opacity is already set to 0.4 for offline printers (see .stale
class), both styles combined, this text becomes practically invisible.

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/settings/destination_settings.js
(right):

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/destination_settings.js:127: var
offlineStatusEl = this.getElement().getElementsByClassName(
On 2014/02/12 22:10:29, Toscano wrote:
> Why not use this.getChildElement()?

Done.

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/destination_settings.js:134:
this.getElement().querySelector('.destination-settings-box');
On 2014/02/12 22:10:29, Toscano wrote:
> Why not use this.getChildElement()?

Done.

https://codereview.chromium.org/150943006/diff/1/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/settings/destination_settings.js:141: }
On 2014/02/12 22:10:29, Toscano wrote:
> What about simplifying this if-else statement to:
> 
> destinationSettingsBoxEl.classList.toggle(
>     DestinationSettings.Classes_.STALE, isOffline);

Done.

[Toscano, 2014-02-13 19:19:04]

lgtm

[Aleksey Shlyapnikov, 2014-02-13 20:00:27]

The CQ bit was checked by alekseys@chromium.org

[commit-bot: I haz the power, 2014-02-13 20:12:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alekseys@chromium.org/150943006/80001

[commit-bot: I haz the power, 2014-02-13 20:37:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-13 20:37:13]

Retried try job too often on win_rel for step(s) app_list_unittests,
ash_unittests, aura_unittests, base_unittests, browser_tests,
cacheinvalidation_unittests, cc_unittests, check_deps, check_deps2git,
chrome_elf_unittests, chromedriver_unittests, components_unittests,
compositor_unittests, content_browsertests, content_unittests, crypto_unittests,
events_unittests, google_apis_unittests, gpu_unittests,
installer_util_unittests, interactive_ui_tests, ipc_tests, jingle_unittests,
media_unittests, mini_installer_test, nacl_integration, net_unittests,
ppapi_unittests, printing_unittests, remoting_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, telemetry_perf_unittests,
telemetry_unittests, unit_tests, views_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Aleksey Shlyapnikov, 2014-02-13 23:55:37]

The CQ bit was checked by alekseys@chromium.org

[commit-bot: I haz the power, 2014-02-13 23:59:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alekseys@chromium.org/150943006/80001

[commit-bot: I haz the power, 2014-02-14 01:37:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-14 01:37:09]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Vitaly Buka (NO REVIEWS), 2014-02-14 18:35:55]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-02-14 18:36:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alekseys@chromium.org/150943006/80001

[commit-bot: I haz the power, 2014-02-14 18:36:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-14 18:36:47]

Failed to apply patch for
chrome/browser/resources/print_preview/search/destination_list_item.css:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
chrome/browser/resources/print_preview/search/destination_list_item.css
  Hunk #1 FAILED at 35.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/resources/print_preview/search/destination_list_item.css.rej

Patch:      
chrome/browser/resources/print_preview/search/destination_list_item.css
Index: chrome/browser/resources/print_preview/search/destination_list_item.css
diff --git
a/chrome/browser/resources/print_preview/search/destination_list_item.css
b/chrome/browser/resources/print_preview/search/destination_list_item.css
index
9fd7a0698cb37866f6580baa604dc13ded1e0a3e..a24ab7e787217063b334dda2b2b7129d4f827e57
100644
--- a/chrome/browser/resources/print_preview/search/destination_list_item.css
+++ b/chrome/browser/resources/print_preview/search/destination_list_item.css
@@ -35,7 +35,6 @@
 .destination-list-item .offline-status {
   -webkit-margin-start: 1em;
   font-size: 75%;
-  opacity: 0.6;
 }
 
 .register-promo {

[Aleksey Shlyapnikov, 2014-02-14 19:07:33]

The CQ bit was checked by alekseys@chromium.org

[Aleksey Shlyapnikov, 2014-02-14 19:07:33]

The CQ bit was unchecked by alekseys@chromium.org

[Aleksey Shlyapnikov, 2014-02-14 19:23:27]

The CQ bit was checked by alekseys@chromium.org

[commit-bot: I haz the power, 2014-02-14 19:25:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alekseys@chromium.org/150943006/580001

[commit-bot: I haz the power, 2014-02-15 00:34:35]

Change committed as 251457