Issue 7951011: Moving where I'm doing dropEffect normalization.

Issue 7951011: Moving where I'm doing dropEffect normalization. (Closed)

Created:
9 years, 3 months ago by Dan Beam

Modified:
9 years, 3 months ago

Reviewers:
csilv, Evan Stade

CC:
chromium-reviews, estade+watch_chromium.org, arv (Not doing code reviews)

Base URL:
http://git.chromium.org/git/chromium.git@trunk

Visibility:
Public.

Description

Moving where I'm doing dropEffect normalization. BUG=95719 R=estade@chromium.org TEST=Drag a tile element over the trash or navdot on Windows and check the e.dataTransfer.dropEffect in TilePage#onDragEnd_. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102239

Patch Set 1 #

Patch Set 2 : code review changes from conversation with estade over chat #

Total comments: 5

Patch Set 3 : incorporating csilv's review #

Total comments: 2

Patch Set 4 : small whitespace changes #

Created: 9 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+23 lines, -10 lines)			Patch
M	chrome/browser/resources/ntp4/apps_page.js	View	1	1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/resources/ntp4/bookmarks_page.js	View	1 2	1 chunk	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/resources/ntp4/nav_dot.js	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/resources/ntp4/tile_page.js	View	1 2 3	5 chunks	+17 lines, -4 lines	0 comments	Download
M	chrome/browser/resources/ntp4/trash.js	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Dan Beam

ping: estade - what else particularly do you want me to do here?

9 years, 3 months ago (2011-09-20 02:05:07 UTC) #1

csilv

http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp4/tile_page.js File chrome/browser/resources/ntp4/tile_page.js (right): http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp4/tile_page.js#newcode20 chrome/browser/resources/ntp4/tile_page.js:20: } Add a linefeed here.

9 years, 3 months ago (2011-09-20 19:05:33 UTC) #2

Dan Beam

9 years, 3 months ago (2011-09-20 23:00:21 UTC) #3

Evan Stade

forgot to press publish. lgtm http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp4/tile_page.js File chrome/browser/resources/ntp4/tile_page.js (right): http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp4/tile_page.js#newcode24 chrome/browser/resources/ntp4/tile_page.js:24: currentlyDraggingTile.lastDropEffect = dataTransfer.dropEffect; = ...

9 years, 3 months ago (2011-09-21 21:58:29 UTC) #4

Dan Beam

http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp4/tile_page.js File chrome/browser/resources/ntp4/tile_page.js (right): http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp4/tile_page.js#newcode24 chrome/browser/resources/ntp4/tile_page.js:24: currentlyDraggingTile.lastDropEffect = dataTransfer.dropEffect; On 2011/09/21 21:58:29, Evan Stade wrote: ...

9 years, 3 months ago (2011-09-21 23:28:38 UTC) #5

Evan Stade

9 years, 3 months ago (2011-09-21 23:56:27 UTC) #7

http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp...
File chrome/browser/resources/ntp4/tile_page.js (right):

http://codereview.chromium.org/7951011/diff/4001/chrome/browser/resources/ntp...
chrome/browser/resources/ntp4/tile_page.js:24:
currentlyDraggingTile.lastDropEffect = dataTransfer.dropEffect;
On 2011/09/21 23:28:38, Dan Beam wrote:
> On 2011/09/21 21:58:29, Evan Stade wrote:
> > = effect
> 
> I did this to filter out bad values.  Do you not want to?  i.e. "Let's set the
> drop effect to 'copyPasta'", it doesn't work, and now you have a .dropEffect
> that's whatever it was before and a lastDropEffect of 'copyPasta'.

oh right, you said that before. I never deleted my comment.

Expand Messages | Collapse Messages