NTP4: Dragging app over trash should be more apparent.

NTP4: Dragging app over trash should be more apparent.

R=estade@chromium.org,csilv@chromium.org
BUG=92912
TEST=Drag a [removable] app over the trash in either LTR or RTL and be greeted
by a sweet trash can lid opening animation.

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

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

[Evan Stade, 2011-12-05 02:33:54]

code looks fine, want to see animation before LG though

why did you use class instead of ID for can/lid?

[Dan Beam, 2011-12-05 16:34:52]

On 2011/12/05 02:33:54, Evan Stade wrote:
> code looks fine, want to see animation before LG though

sgtm, will show you when you get here.

> why did you use class instead of ID for can/lid?

Because IDs are guaranteed to be global and I didn't want to possibly conflict
(and if you notice, all my classes are local as well, i.e. #trash > .can). 
Also, I'm only doing local querySelector()s from #trash to get these nodes
(which probably aren't cheaper than $('trash-can') but also isn't expensive in
this case).

Overall, using more "OO" CSS is one of the best way to ensure not colliding with
other styles and to easily create re-usable components in CSS (AFAIK).  For
example:

  #page {}
  #page .hdr {}
  #page .hdr h1 {}
  #page .hdr h1 a {}
  #page .nav {}
  #page .nav ul {}
  #page .nav ul li {}
  #page .nav ul li a {}
  #page .foot {}
  #page .foot a {}

It's a lot easier to read this given the hierarchy in the selector than a flat
list of IDs and classes, even if they're well named, and much less likely to
collide.

If you don't agree with this, though, let me know and I can change.

[Evan Stade, 2011-12-05 20:47:13]

your reasoning is sound but I think using classes implies that there's more than
one, whereas if you use and ID it's obvious there's only one. Using a class
doesn't seem to fix the collision problem because you could still accidentally
re-use that class name, at which point you're pretty likely to create a rule
that unintentionally affects the #trash .can as well as the other .can.

I don't really care too strongly one way or the other.

[Dan Beam, 2011-12-05 20:55:25]

On 2011/12/05 20:47:13, Evan Stade wrote:
> your reasoning is sound but I think using classes implies that there's more
than
> one, whereas if you use and ID it's obvious there's only one. Using a class
> doesn't seem to fix the collision problem because you could still accidentally
> re-use that class name, at which point you're pretty likely to create a rule
> that unintentionally affects the #trash .can as well as the other .can.

I think it's entirely possible we could have 2 trash cans and .class collision
only happens when you use .global classes, not #scoped .classes.

> I don't really care too strongly one way or the other.

[Dan Beam, 2011-12-05 22:40:41]

changed .png and simplified CSS a touch

[csilv, 2011-12-05 22:47:43]

lgtm

[Evan Stade, 2011-12-05 23:22:06]

http://codereview.chromium.org/8786010/diff/8001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/new_tab.css (right):

http://codereview.chromium.org/8786010/diff/8001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/new_tab.css:302: -webkit-transform-origin: 15px
100%;
percent better

[Dan Beam, 2011-12-06 00:28:47]

ptal

http://codereview.chromium.org/8786010/diff/8001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/new_tab.css (right):

http://codereview.chromium.org/8786010/diff/8001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/new_tab.css:302: -webkit-transform-origin: 15px
100%;
On 2011/12/05 23:22:06, Evan Stade wrote:
> percent better

Done.

[Evan Stade, 2011-12-06 00:40:34]

lgtm

[commit-bot: I haz the power, 2011-12-06 00:48:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/8786010/2008

[commit-bot: I haz the power, 2011-12-06 01:27:58]

Try job failure for 8786010-2008 (retry) on linux_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2011-12-06 02:17:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/8786010/2008

[commit-bot: I haz the power, 2011-12-06 07:30:07]

Can't apply patch for file chrome/browser/resources/ntp4/new_tab.css.
While running patch -p1 --forward --force;
patching file chrome/browser/resources/ntp4/new_tab.css
Hunk #1 FAILED at 240.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/resources/ntp4/new_tab.css.rej

[commit-bot: I haz the power, 2011-12-06 07:42:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/8786010/13001

[commit-bot: I haz the power, 2011-12-06 09:17:06]

Change committed as 113167