Port downloads.ItemView to a Polymer component.

Port downloads.ItemView to a Polymer component.

This results in a very ugly, somewhat functional MD chrome://downloads.

R=michaelpg@chromium.org
BUG=425626
NOPRESUBMIT=true

Committed: https://crrev.com/ec7d18e9d10b9988c2a8b40572d598fd56711e0a
Cr-Commit-Position: refs/heads/master@{#338221}

[Dan Beam, 2015-07-07 02:19:14]

Patchset #1 (id:1) has been deleted

[Dan Beam, 2015-07-07 19:02:51]

dbeam@chromium.org changed reviewers:
+ jlklein@chromium.org
- michaelpg@chromium.org

[arv (Not doing code reviews), 2015-07-07 22:35:28]

Drive by...

https://codereview.chromium.org/1224623013/diff/20001/ui/webui/resources/js/i...
File ui/webui/resources/js/i18n_template.js (right):

https://codereview.chromium.org/1224623013/diff/20001/ui/webui/resources/js/i...
ui/webui/resources/js/i18n_template.js:7: (function() {
We have real block scope now =P

[Dan Beam, 2015-07-08 02:21:17]

dbeam@chromium.org changed reviewers:
+ michaelpg@chromium.org
- jlklein@chromium.org

[Dan Beam, 2015-07-08 02:22:30]

https://codereview.chromium.org/1224623013/diff/20001/ui/webui/resources/js/i...
File ui/webui/resources/js/i18n_template.js (right):

https://codereview.chromium.org/1224623013/diff/20001/ui/webui/resources/js/i...
ui/webui/resources/js/i18n_template.js:7: (function() {
On 2015/07/07 22:35:28, arv wrote:
> We have real block scope now =P

Done. (in https://codereview.chromium.org/1229573003/)

[michaelpg, 2015-07-08 19:39:52]

I think I need some context for item_view.*. Why not use data binding instead of
a 100-line ready method? Is this about retaining focus on updates, and does
Polymer definitely break that?

https://codereview.chromium.org/1224623013/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.js (right):

https://codereview.chromium.org/1224623013/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.js:17: this.$.safe.ondragstart =
this.onSafeDragstart_.bind(this);
y u no use annotated events in the template?

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.html (right):

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.html:1: <link rel="import"
href="chrome://resources/html/cr.html">
nit: include polymer

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.html:14: <a is="action-link"
id="file-link" column-type="name"></a>
Not 100% clear on how focusing works, but could the element manage its own focus
state?

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.js (right):

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.js:13: return
this.$['date-container'];
I'm not sure this is safe -- is the local DOM guaranteed to be stamped out, and
$ initialized, before this function is evaluated? (I doubt it; the created
callback is called before local DOM is initialized, and I would expect to be
able to access properties in the created callback.)

If you're not sure, move it to ready().

https://www.polymer-project.org/1.0/docs/devguide/registering-elements.html#i...

[Dan Beam, 2015-07-09 18:13:45]

https://codereview.chromium.org/1224623013/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.js (right):

https://codereview.chromium.org/1224623013/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.js:17: this.$.safe.ondragstart =
this.onSafeDragstart_.bind(this);
On 2015/07/08 19:39:52, michaelpg wrote:
> y u no use annotated events in the template?

what's the advantage?  we don't use inline event handlers or styles anywhere
else... and then we have to flip between multiple files to find event handlers
(html and js)

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.html (right):

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.html:1: <link rel="import"
href="chrome://resources/html/cr.html">
On 2015/07/08 19:39:52, michaelpg wrote:
> nit: include polymer

Done.

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.html:14: <a is="action-link"
id="file-link" column-type="name"></a>
On 2015/07/08 19:39:52, michaelpg wrote:
> Not 100% clear on how focusing works, but could the element manage its own
focus
> state?

it's possible this component could have it's own cr.ui.FocusRow, but I haven't
really thought about any of that yet (I'm going to add it back soon).

I can remove column-type if you'd like, but I didn't see a huge issue with
leaving them for now.

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.js (right):

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.js:13: return
this.$['date-container'];
On 2015/07/08 19:39:52, michaelpg wrote:
> I'm not sure this is safe -- is the local DOM guaranteed to be stamped out,
and
> $ initialized, before this function is evaluated? (I doubt it; the created
> callback is called before local DOM is initialized, and I would expect to be
> able to access properties in the created callback.)
> 
> If you're not sure, move it to ready().
> 
>
https://www.polymer-project.org/1.0/docs/devguide/registering-elements.html#i...

Done. (in an alternate way)

[Dan Beam, 2015-07-09 21:29:52]

now with updated patchset!

[Dan Beam, 2015-07-09 21:44:39]

note: most of the code I'm adding is being ported from:

chrome/browser/resources/downloads/{item_view,manager}.js

[michaelpg, 2015-07-10 00:56:26]

lgtm

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.html (right):

https://codereview.chromium.org/1224623013/diff/40001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.html:14: <a is="action-link"
id="file-link" column-type="name"></a>
On 2015/07/09 18:13:45, Dan Beam wrote:
> On 2015/07/08 19:39:52, michaelpg wrote:
> > Not 100% clear on how focusing works, but could the element manage its own
> focus
> > state?
> 
> it's possible this component could have it's own cr.ui.FocusRow, but I haven't
> really thought about any of that yet (I'm going to add it back soon).
> 
> I can remove column-type if you'd like, but I didn't see a huge issue with
> leaving them for now.

Acknowledged.

https://codereview.chromium.org/1224623013/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.html (right):

https://codereview.chromium.org/1224623013/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.html:54: <link rel="import"
href="chrome://resources/polymer/v1_0/polymer/polymer.html">
Is there a difference between putting these links here vs. outside of
<dom-module>?

(other than, technically, I think these link wouldn't be loaded without Polymer
handling <dom-module>)

[Dan Beam, 2015-07-10 01:02:29]

https://codereview.chromium.org/1224623013/diff/80001/chrome/browser/resource...
File chrome/browser/resources/md_downloads/item_view.html (right):

https://codereview.chromium.org/1224623013/diff/80001/chrome/browser/resource...
chrome/browser/resources/md_downloads/item_view.html:54: <link rel="import"
href="chrome://resources/polymer/v1_0/polymer/polymer.html">
On 2015/07/10 00:56:26, michaelpg wrote:
> Is there a difference between putting these links here vs. outside of
> <dom-module>?
> 
> (other than, technically, I think these link wouldn't be loaded without
Polymer
> handling <dom-module>)

¯\_(ツ)_/¯

https://github.com/Polymer/docs/issues/1124
https://github.com/Polymer/docs/pull/1176/files

[Dan Beam, 2015-07-10 01:51:08]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2015-07-10 01:51:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/1224623013/#ps120001
(title: "merge")

[commit-bot: I haz the power, 2015-07-10 01:51:58]

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

[commit-bot: I haz the power, 2015-07-10 02:00:47]

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

[commit-bot: I haz the power, 2015-07-10 02:00:49]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Dan Beam, 2015-07-10 02:03:32]

will fix pesky presubmit issues in a followup

[Dan Beam, 2015-07-10 02:03:37]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2015-07-10 02:04:29]

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

[commit-bot: I haz the power, 2015-07-10 02:36:08]

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

[commit-bot: I haz the power, 2015-07-10 02:36:09]

Failed to request the patch to try. Please note that binary files are still
unsupported at the moment, this is being worked on.

Thanks for your patience.

Transient error: Can't process patch for file
chrome/browser/browser_resources.grd.
Diff seems corrupted.

[Dan Beam, 2015-07-10 02:38:19]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2015-07-10 02:38:52]

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

[commit-bot: I haz the power, 2015-07-10 02:43:06]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-07-10 02:44:28]

Patchset 6 (id:??) landed as
https://crrev.com/ec7d18e9d10b9988c2a8b40572d598fd56711e0a
Cr-Commit-Position: refs/heads/master@{#338221}