Issue 16295012: Made the EndDrag logic clearer by using |drag_view_| as Drag state.

Issue 16295012: Made the EndDrag logic clearer by using |drag_view_| as Drag state. (Closed)

Created:
7 years, 6 months ago by Mr4D (OOO till 08-26)

Modified:
7 years, 6 months ago

Reviewers:
xiyuan

CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, tfarina

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Made the EndDrag logic clearer by using |drag_view_| as Drag state. BUG=246116 TEST=none R=xiyuan@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203806

Patch Set 2 : Addressed #

Created: 7 years, 6 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+13 lines, -7 lines)			Patch
	M	ui/app_list/views/apps_grid_view.cc	View	1	4 chunks	+13 lines, -7 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

xiyuan

LGTM https://codereview.chromium.org/16295012/diff/1/ui/app_list/views/apps_grid_view.cc File ui/app_list/views/apps_grid_view.cc (right): https://codereview.chromium.org/16295012/diff/1/ui/app_list/views/apps_grid_view.cc#newcode195 ui/app_list/views/apps_grid_view.cc:195: if (drag_view_ || pulsing_blocks_model_.view_size() || !view) nit: DCHECK(view) ...

7 years, 6 months ago (2013-06-03 18:03:34 UTC) #2

Mr4D (OOO till 08-26)

Addressed. Thanks! https://codereview.chromium.org/16295012/diff/1/ui/app_list/views/apps_grid_view.cc File ui/app_list/views/apps_grid_view.cc (right): https://codereview.chromium.org/16295012/diff/1/ui/app_list/views/apps_grid_view.cc#newcode195 ui/app_list/views/apps_grid_view.cc:195: if (drag_view_ || pulsing_blocks_model_.view_size() || !view) Done.

7 years, 6 months ago (2013-06-03 18:35:16 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skuhne@chromium.org/16295012/3002

7 years, 6 months ago (2013-06-03 18:35:42 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/skuhne@chromium.org/16295012/3002

7 years, 6 months ago (2013-06-04 00:26:14 UTC) #5

Mr4D (OOO till 08-26)

7 years, 6 months ago (2013-06-04 01:05:51 UTC) #6

Message was sent while issue was closed.

Committed patchset #2 manually as r203806 (presubmit successful).

Expand Messages | Collapse Messages