New issue
Advanced search Search tips

Issue 887656 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Implement drop target spec and remove "double" highlight

Project Member Reported by mccanny@chromium.org, Sep 20

Issue description

Create special highlight state for app + tab drop targets, resize the plus icon and remove it for app drop targets. See spec: https://drive.google.com/file/d/1qBlaXPiQUmddymY5Qoy2rx2lDJymMJ2V/view?usp=sharing

Current state: https://drive.google.com/open?id=11K4DyymAOo6x4_wDHWH9fvuWt7YO9otL


xdai@ assigning to you to determine who should work on this.
 
Components: UI>Shell>OverviewMode
For reference, here's the code I used to set the plus icon size in my prototype:

// @width and @height are the width and height of the rounded rect target
smallerDim = Math.min(@width, @height)
if(smallerDim < 96) {
  w = smallerDim * 0.5
} else {
  // icon is 60px @ 225px smaller dim size, scales 1:1 to 48px @ 180px and 72px @ 270px
  w = Math.max(Math.min(smallerDim / 225 * 60, 72), 48)
}
@icon.width = @icon.height = w
Cc: x...@chromium.org
Owner: minch@chromium.org
Talked with Min. Assign to her for now. 
I'm also able to help her to implement part of them, after I finish my current refactoring work (expect to be later next week). 
Labels: OS-Chrome
Status: Started (was: Untriaged)
Sure. Gardening this week, doing it when PFQ is green.
Hi Ben, I have one question for the size of the plus icon.
The plus icon includes two lines. Each line should have two dimension sizes, one longer and one shorter. e.g, for the horizontal line, its width is larger than its height. Seems that you provided the size of the longer side of the icon? Then how about the shorter side? like the height of the horizontal line?
BTW, its current value is fixed, equals to 0.03 * display_height.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b

commit d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b
Author: Min Chen <minch@google.com>
Date: Tue Oct 09 19:56:14 2018

Implement new drop target for window drag in tablet mode.

Changes in this cl,
- Implemented the plus icon in new selector item with image instead of
  two rectangles.

- Show the background of new selector item only if the dragged window
  has been dragged into it.

- Show the plus icon only if drag a tab window from a multi-tab window.
  And adjust its size based on the size of the new selector item.

- Remove the "double" highlight of the selected new selector item, which
  means do not move the selection widget to the new selector item if
  selected. Drop the dragged window into overview based on the drag
  position.

- SetBounds for the new selector item instead of SetTransform for it.
  The transform of the new selector item will be identity, which means
  we can set its size, border thickness and radius corner directly.

Spec:
https://drive.google.com/file/d/1qBlaXPiQUmddymY5Qoy2rx2lDJymMJ2V/view

Recorded video:
https://drive.google.com/file/d/0B5I0jFeLxqIiZDR0UDRVcElPV1FqWjNiYjJNbU1MY1hJaDZB/view?usp=sharing

Test: Manually
Bug:  887656 
Change-Id: I6fc91f623b0149cd58fc91849df03a87dbeade1b
Reviewed-on: https://chromium-review.googlesource.com/c/1257554
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598049}
[modify] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/BUILD.gn
[modify] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/resources/vector_icons/overview_new_selector_item_plus.icon
[add] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/wm/overview/new_selector_item_view.cc
[add] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/wm/overview/new_selector_item_view.h
[modify] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/wm/overview/scoped_transform_overview_window.cc
[modify] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/wm/overview/window_grid.cc
[modify] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/wm/overview/window_grid.h
[modify] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/wm/overview/window_selector_item.cc
[modify] https://crrev.com/d1aab415ca9cc3a9afa803bb44f1f0d587c4a08b/ash/wm/splitview/split_view_controller_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99c1cee4aaf357ed658b1175407bfcb9ba771cd7

commit 99c1cee4aaf357ed658b1175407bfcb9ba771cd7
Author: Min Chen <minch@google.com>
Date: Wed Oct 10 21:09:59 2018

Rename new selector item to drop target.

No functional change, just rename to have shorter name and better indent.

Bug:  887656 
Change-Id: If5b1eebac13371416df072c40549fc426a714b05
Reviewed-on: https://chromium-review.googlesource.com/c/1274003
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598505}
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/BUILD.gn
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/resources/vector_icons/BUILD.gn
[rename] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/resources/vector_icons/overview_drop_target_plus.icon
[rename] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/overview/drop_target_view.cc
[rename] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/overview/drop_target_view.h
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/overview/scoped_transform_overview_window.cc
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/overview/window_grid.cc
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/overview/window_grid.h
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/overview/window_selector.h
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/overview/window_selector_item.cc
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/splitview/split_view_controller_unittest.cc
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/tablet_mode/tablet_mode_window_drag_delegate.cc
[modify] https://crrev.com/99c1cee4aaf357ed658b1175407bfcb9ba771cd7/ash/wm/tablet_mode/tablet_mode_window_drag_delegate.h

Sign in to add a comment