Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(484)

Issue 12677002: Adds animation on a hovered directory/volume when dragging files in Files.app (Closed)

Created:
7 years, 9 months ago by mtomasz
Modified:
7 years, 9 months ago
Reviewers:
yoshiki
CC:
chromium-reviews, rginda+watch_chromium.org, arv+watch_chromium.org
Visibility:
Public.

Description

Adds animation on a hovered directory/volume when dragging files in Files.app To give more visible feedback, an animation has been added. The target directory blinks three times before it is opened. Along the way, now the directories which can't accept files are not openable. TEST=Drag a file to some directory on a different volume. BUG=137980 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187245

Patch Set 1 #

Total comments: 8

Patch Set 2 : Addressed comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -28 lines) Patch
M chrome/browser/resources/file_manager/css/file_manager.css View 1 2 chunks +26 lines, -0 lines 0 comments Download
M chrome/browser/resources/file_manager/js/file_transfer_controller.js View 1 6 chunks +45 lines, -28 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
mtomasz
@yoshiki: PTAL.
7 years, 9 months ago (2013-03-08 04:59:47 UTC) #1
yoshiki
https://chromiumcodereview.appspot.com/12677002/diff/1/chrome/browser/resources/file_manager/js/file_transfer_controller.js File chrome/browser/resources/file_manager/js/file_transfer_controller.js (right): https://chromiumcodereview.appspot.com/12677002/diff/1/chrome/browser/resources/file_manager/js/file_transfer_controller.js#newcode427 chrome/browser/resources/file_manager/js/file_transfer_controller.js:427: opt_destinationPath) { Could you add @param annotations? https://chromiumcodereview.appspot.com/12677002/diff/1/chrome/browser/resources/file_manager/js/file_transfer_controller.js#newcode435 chrome/browser/resources/file_manager/js/file_transfer_controller.js:435: ...
7 years, 9 months ago (2013-03-08 08:10:38 UTC) #2
mtomasz
@yoshiki: I've addressed your comments. PTAL one more time. https://codereview.chromium.org/12677002/diff/1/chrome/browser/resources/file_manager/js/file_transfer_controller.js File chrome/browser/resources/file_manager/js/file_transfer_controller.js (right): https://codereview.chromium.org/12677002/diff/1/chrome/browser/resources/file_manager/js/file_transfer_controller.js#newcode427 chrome/browser/resources/file_manager/js/file_transfer_controller.js:427: ...
7 years, 9 months ago (2013-03-11 03:17:50 UTC) #3
yoshiki
lgtm
7 years, 9 months ago (2013-03-11 03:56:28 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtomasz@chromium.org/12677002/4001
7 years, 9 months ago (2013-03-11 04:16:57 UTC) #5
commit-bot: I haz the power
7 years, 9 months ago (2013-03-11 07:12:08 UTC) #6
Message was sent while issue was closed.
Change committed as 187245

Powered by Google App Engine
This is Rietveld 408576698