Add Media Router route-details.

Add Media Router route-details.

This custom polymer element shows some information from media that is currently cast to a device. It is styled after the current cast extension's route details view.

BUG=464222
Committed: https://crrev.com/0aadb1da6531a92db44ebb34874c3932db25a9ba
Cr-Commit-Position: refs/heads/master@{#327332}

[apacible, 2015-04-08 00:03:25]

Patchset #1 (id:1) has been deleted

[apacible, 2015-04-08 00:03:29]

Patchset #1 (id:20001) has been deleted

[apacible, 2015-04-08 00:06:41]

Patchset #1 (id:40001) has been deleted

[apacible, 2015-04-08 00:27:51]

Patchset #1 (id:60001) has been deleted

[apacible, 2015-04-08 00:29:09]

apacible@chromium.org changed reviewers:
+ jhawkins@chromium.org, jlklein@chromium.org, mfoltz@chromium.org,
michaelpg@chromium.org

[apacible, 2015-04-08 00:56:42]

Patchset #1 (id:80001) has been deleted

[apacible, 2015-04-08 00:57:24]

PTAL, thanks!

TODO: Strings need to be localized; i18n for polymer under investigation.

Screenshots:
- LTR: https://screenshot.googleplex.com/FEouPovcVb.png
- LTR (w/hover): https://screenshot.googleplex.com/W7OqoTPVV5.png
- RTL: https://screenshot.googleplex.com/2ZjunaQo8b.png
- RTL (w/hover): https://screenshot.googleplex.com/0HHctBURdo.png

[michaelpg, 2015-04-08 06:00:17]

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.css
(right):

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:14:
-webkit-padding-start: 10px;
can we avoid some of this duplicationt? e.g. by wrapping the two "activity-*"
<div>s.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:32:
background: linear-gradient(to top, rgb(254,254,254), rgb(222,222,222));
nit: spaces after commas in rgb values

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:33:
border: 1px solid rgb(193, 193, 195);
just checking that 195 isn't a typo here, since the other colors are shades of
gray

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:46:
line-height: 1.125em;
line-height is inherited so line 46 is redundant, right?

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:47:
overflow: hidden;
Hmm, I may be mistaken but I don't believe overflow: hidden affects an inline
element like this <span>. Have you tested this with long names to see what
happens?

Depending on what you're going for, you could try making this inline-block
(maybe make <div class="activity-status"> display: flex) or applying oveflow:
none to <div class="activity-status">.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.js
(right):

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:5:
Polymer('route-details', {
Please add a comment describing this element.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:12:
* @default: null
no colon:
@default null

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:14:
route: null
optional nit: I usually recommend using trailing commas for the last value in
lists/objects, partly so that the "git blame" for the line isn't affected when
somebody adds a new value, partly to avoid someone accidentally forgetting to
add the comma.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:30:
}
same (optional) nit

[apacible, 2015-04-08 18:25:48]

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.css
(right):

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:14:
-webkit-padding-start: 10px;
On 2015/04/08 06:00:17, michaelpg wrote:
> can we avoid some of this duplicationt? e.g. by wrapping the two "activity-*"
> <div>s.

Done.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:32:
background: linear-gradient(to top, rgb(254,254,254), rgb(222,222,222));
On 2015/04/08 06:00:17, michaelpg wrote:
> nit: spaces after commas in rgb values

Done.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:33:
border: 1px solid rgb(193, 193, 195);
On 2015/04/08 06:00:17, michaelpg wrote:
> just checking that 195 isn't a typo here, since the other colors are shades of
> gray

Not a typo. This is also a shade of gray (originally #c1c1c3 in the extension).

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:46:
line-height: 1.125em;
On 2015/04/08 06:00:17, michaelpg wrote:
> line-height is inherited so line 46 is redundant, right?

Done.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:47:
overflow: hidden;
On 2015/04/08 06:00:17, michaelpg wrote:
> Hmm, I may be mistaken but I don't believe overflow: hidden affects an inline
> element like this <span>. Have you tested this with long names to see what
> happens?
> 
> Depending on what you're going for, you could try making this inline-block
> (maybe make <div class="activity-status"> display: flex) or applying oveflow:
> none to <div class="activity-status">.

Removed. Talked to UX designer, and super long names will look something like
this:
https://screenshot.googleplex.com/e2WnwRDYku.png

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.js
(right):

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:5:
Polymer('route-details', {
On 2015/04/08 06:00:17, michaelpg wrote:
> Please add a comment describing this element.

Done. I'll add this to the other elements in a separate CL.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:12:
* @default: null
On 2015/04/08 06:00:17, michaelpg wrote:
> no colon:
> @default null

Done.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:14:
route: null
On 2015/04/08 06:00:17, michaelpg wrote:
> optional nit: I usually recommend using trailing commas for the last value in
> lists/objects, partly so that the "git blame" for the line isn't affected when
> somebody adds a new value, partly to avoid someone accidentally forgetting to
> add the comma.

Sounds good, done. I'll add this in the other elements in a separate CL.

https://codereview.chromium.org/1027253004/diff/100001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.js:30:
}
On 2015/04/08 06:00:17, michaelpg wrote:
> same (optional) nit

Done.

[James Hawkins, 2015-04-08 19:02:36]

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.css
(right):

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:17:
color: #7d7d7d;
nit: Use either #rgb or rgb() consistently in a given file.

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.html
(right):

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.html:7:
‹  Back to device list
Are you sure we should be using &lsaquo instead of an image?

[apacible, 2015-04-08 19:58:24]

Patchset #3 (id:140001) has been deleted

[apacible, 2015-04-08 20:02:02]

FYI -- holding off on this CL until we have localization for Media Router done.

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.css
(right):

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:17:
color: #7d7d7d;
On 2015/04/08 19:02:36, James Hawkins wrote:
> nit: Use either #rgb or rgb() consistently in a given file.

Done.

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.html
(right):

https://codereview.chromium.org/1027253004/diff/120001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.html:7:
‹  Back to device list
On 2015/04/08 19:02:36, James Hawkins wrote:
> Are you sure we should be using &lsaquo instead of an image?

Talked offline, keeping as is.

[mark a. foltz, 2015-04-08 22:24:54]

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.css
(right):

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:33:
button {
Is this duplicate with the button class declared elsewhere?  Can the common
styles be factored out?

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:40:
height: 32px;
How does fixed-height interact with font resizing?

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.html
(right):

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.html:16:
<!-- TODO(apacible): Show the sink's name rather than ID -->
Can we show the route title (if set) and fall back on the sink name?
What about passing the Sink object as part of the route details?

[mark a. foltz, 2015-04-08 22:26:27]

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.css
(right):

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:33:
button {
This is what I was thinking of.

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

[mark a. foltz, 2015-04-21 20:49:28]

Just checking - can this be picked up again?

[apacible, 2015-04-21 20:52:27]

On 2015/04/21 20:49:28, mark a. foltz wrote:
> Just checking - can this be picked up again?

Not yet; I need to wire up the media router action to opening a dialog and
upstream the localization work first.

[apacible, 2015-04-24 16:13:27]

Patchset #5 (id:200001) has been deleted

[apacible, 2015-04-24 17:54:59]

Patchset #6 (id:240001) has been deleted

[apacible, 2015-04-24 18:25:08]

Patchset #6 (id:260001) has been deleted

[apacible, 2015-04-24 18:29:08]

mfoltz@ has a CL out with our resource/string providers to land the first part
of our localization work.

See: https://codereview.chromium.org/1056023003/

Screenshots:
LTR: https://screenshot.googleplex.com/ViONmomHO7.png
LTR long sink name: https://screenshot.googleplex.com/SauzwcpLMJ.png
RTL: https://screenshot.googleplex.com/eCYtaM6PEL.png

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.css
(right):

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:33:
button {
On 2015/04/08 22:26:27, mark a. foltz wrote:
> This is what I was thinking of.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

Done, added media_router_common.css file.

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.css:40:
height: 32px;
On 2015/04/08 22:24:54, mark a. foltz wrote:
> How does fixed-height interact with font resizing?

Font sizes don't affect fixed heights.

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route-details/route_details.html
(right):

https://codereview.chromium.org/1027253004/diff/180001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route-details/route_details.html:16:
<!-- TODO(apacible): Show the sink's name rather than ID -->
On 2015/04/08 22:24:54, mark a. foltz wrote:
> Can we show the route title (if set) and fall back on the sink name?
> What about passing the Sink object as part of the route details?

Added Sink object as part of route details. Renamed route.description to
route.title.

I will have media-router-container (currently in private repo) handle checking
that the route.sinkId == sink.id.

[mark a. foltz, 2015-04-24 20:46:59]

lgtm % a couple of minor comments

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/issue_banner/issue_banner.html
(right):

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/issue_banner/issue_banner.html:5:
<link rel="stylesheet" href="../../media_router_common.css">
Should the common stylesheet go first (so that the element-specific styles can
extend or override)?

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route_details/route_details.html
(right):

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route_details/route_details.html:5:
<link rel="stylesheet" href="../../media_router_common.css">
Similar comment here

[apacible, 2015-04-24 21:02:35]

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/issue_banner/issue_banner.html
(right):

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/issue_banner/issue_banner.html:5:
<link rel="stylesheet" href="../../media_router_common.css">
On 2015/04/24 20:46:59, mark a. foltz wrote:
> Should the common stylesheet go first (so that the element-specific styles can
> extend or override)?

Done.

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
File
chrome/browser/resources/media_router/elements/route_details/route_details.html
(right):

https://codereview.chromium.org/1027253004/diff/280001/chrome/browser/resourc...
chrome/browser/resources/media_router/elements/route_details/route_details.html:5:
<link rel="stylesheet" href="../../media_router_common.css">
On 2015/04/24 20:46:59, mark a. foltz wrote:
> Similar comment here

Done.

[apacible, 2015-04-27 23:28:52]

Patchset #8 (id:320001) has been deleted

[apacible, 2015-04-27 23:31:28]

Patchset #8 (id:340001) has been deleted

[apacible, 2015-04-27 23:32:06]

mfoltz landed his patch earlier today. Just rebased with his changes.

[James Hawkins, 2015-04-28 17:32:57]

lgtm

[apacible, 2015-04-28 17:36:32]

The CQ bit was checked by apacible@chromium.org

[apacible, 2015-04-28 17:36:33]

The patchset sent to the CQ was uploaded after l-g-t-m from mfoltz@chromium.org
Link to the patchset: https://codereview.chromium.org/1027253004/#ps360001
(title: "Rebase")

[commit-bot: I haz the power, 2015-04-28 17:36:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027253004/360001

[commit-bot: I haz the power, 2015-04-28 18:24:44]

Committed patchset #8 (id:360001)

[commit-bot: I haz the power, 2015-04-28 18:25:50]

Patchset 8 (id:??) landed as
https://crrev.com/0aadb1da6531a92db44ebb34874c3932db25a9ba
Cr-Commit-Position: refs/heads/master@{#327332}