Draggable peeking/fullscreen launcher with transparent background.

ui/app_list/views/app_list_view.cc

Index: ui/app_list/views/app_list_view.cc
diff --git a/ui/app_list/views/app_list_view.cc b/ui/app_list/views/app_list_view.cc
index 08ff586e0718196f00aa7d3b2ef2b0615af3deba..82a87973eed1cd89fe5e0679f0d5113f47ea18fe 100644
--- a/ui/app_list/views/app_list_view.cc
+++ b/ui/app_list/views/app_list_view.cc
@@ -56,6 +56,15 @@ namespace {
 // The margin from the edge to the speech UI.
 const int kSpeechUIMargin = 12;
 
+int kShelfSize = 48;

[vadimt, 2017/05/22 23:36:58]

Comment pls.

[newcomer, 2017/05/25 23:10:51]

Done.

+
+// The y position of the peeking launcher, used in ComputeLauncherConstants().
+int kPeekingLauncherHeight = 320;

[vadimt, 2017/05/22 23:36:57]

Remove 'used in'. Leave the meaning.

[newcomer, 2017/05/25 23:10:52]

Done.

+
+// todo(newcomer): Support half launcher mode.
+// The y position of the half launcher, used in ComputeLauncherConstants().
+// int kHalfLauncherHeight = 561;

[vadimt, 2017/05/22 23:36:58]

I see. Please don't leave commented blocks like this.

[newcomer, 2017/05/25 23:10:52]

Done.

+
 // The vertical position for the appearing animation of the speech UI.
 const float kSpeechUIAppearingPosition = 12;
 
@@ -175,6 +184,11 @@ AppListView::AppListView(AppListViewDelegate* delegate)
       search_box_focus_host_(nullptr),
       search_box_widget_(nullptr),
       search_box_view_(nullptr),
+      initial_drag_separation_(0),

[vadimt, 2017/05/22 23:36:57]

Use initialization in header.
type var = val;

[newcomer, 2017/05/25 23:10:51]

Done.

+      last_fling_velocity_(0),
+      default_peeking_launcher_y_(0),
+      is_fullscreen_launcher_(false),
+      fullscreen_widget_bounds_(),
       overlay_view_(nullptr),
       animation_observer_(new HideViewAnimationObserver()) {
   CHECK(delegate);
@@ -183,6 +197,10 @@ AppListView::AppListView(AppListViewDelegate* delegate)
 }
 
 AppListView::~AppListView() {
+  // Remove this instance from the DisplayManager's list of observers.
+  if (features::IsFullscreenAppListEnabled())
+    display::Screen::GetScreen()->RemoveObserver(this);

[xiyuan, 2017/05/24 18:28:18]

Suggest to use ScopedObserver so that this happens automatically.

[newcomer, 2017/05/25 23:10:52]

Cool! and done.

+
   delegate_->GetSpeechUI()->RemoveObserver(this);
   animation_observer_.reset();
   // Remove child views first to ensure no remaining dependencies on delegate_.
@@ -203,8 +221,10 @@ void AppListView::Initialize(gfx::NativeView parent, int initial_apps_page) {
 
   InitChildWidgets();
   AddChildView(overlay_view_);
+
   if (delegate_)
     delegate_->ViewInitialized();
+
   UMA_HISTOGRAM_TIMES("Apps.AppListCreationTime",
                       base::Time::Now() - start_time);
 }
@@ -330,6 +350,15 @@ void AppListView::InitContents(gfx::NativeView parent, int initial_apps_page) {
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "440224, 441028 AppListView::InitContents"));
 
+  if (features::IsFullscreenAppListEnabled()) {
+    // The shield view that colors the background of the launcher and makes it
+    // transparent.
+    launcher_background_shield_ = new views::View;
+    launcher_background_shield_->SetPaintToLayer(ui::LAYER_SOLID_COLOR);
+    launcher_background_shield_->layer()->SetColor(SK_ColorBLACK);
+    launcher_background_shield_->layer()->SetOpacity(0.8f);
+    AddChildView(launcher_background_shield_);
+  }
   app_list_main_view_ = new AppListMainView(delegate_);
   AddChildView(app_list_main_view_);
   app_list_main_view_->SetPaintToLayer();
@@ -338,6 +367,7 @@ void AppListView::InitContents(gfx::NativeView parent, int initial_apps_page) {
   // This will be added to the |search_box_widget_| after the app list widget is
   // initialized.
   search_box_view_ = new SearchBoxView(app_list_main_view_, delegate_);
+  search_box_view_->SetAppListView(this);
   search_box_view_->SetPaintToLayer();
   search_box_view_->layer()->SetFillsBoundsOpaquely(false);
   search_box_view_->layer()->SetMasksToBounds(true);
@@ -404,21 +434,36 @@ void AppListView::InitChildWidgets() {
 
 void AppListView::InitializeFullscreen(gfx::NativeView parent,
                                        int initial_apps_page) {
-  views::Widget* widget = new views::Widget;
+  ComputeConstants();

[vadimt, 2017/05/22 23:36:57]

Sounds strange... UpdateDimensions?

[newcomer, 2017/05/25 23:10:53]

Done.

+
+  gfx::Rect app_list_overlay_view_bounds(
+      display_work_area_bounds_.x(), default_peeking_launcher_y_,
+      display_work_area_bounds_.width(),
+      display_work_area_bounds_.height() + kShelfSize);
+
+  fullscreen_widget_ = new views::Widget;
   views::Widget::InitParams app_list_overlay_view_params(
       views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
 
+  app_list_overlay_view_params.name = "AppList";
   app_list_overlay_view_params.parent = parent;
   app_list_overlay_view_params.delegate = this;
   app_list_overlay_view_params.opacity =
       views::Widget::InitParams::TRANSLUCENT_WINDOW;
-  app_list_overlay_view_params.bounds =
-      display::Screen::GetScreen()->
-      GetDisplayNearestView(parent).work_area();
-  widget->Init(app_list_overlay_view_params);
-  widget->GetLayer()->SetBackgroundBlur(10);
+  app_list_overlay_view_params.bounds = app_list_overlay_view_bounds;
+  app_list_overlay_view_params.layer_type = ui::LAYER_SOLID_COLOR;
+  fullscreen_widget_->Init(app_list_overlay_view_params);
 
+  fullscreen_widget_bounds_ = fullscreen_widget_->GetWindowBoundsInScreen();
   overlay_view_ = new AppListOverlayView(0 /* no corners */);
+
+  // Launcher + shelf height, used in HandleDrag.
+  launcher_threshold_ =
+      (default_peeking_launcher_y_ + fullscreen_widget_bounds_.height() -
+       fullscreen_widget_bounds_.y()) /
+      3;

[vadimt, 2017/05/22 23:36:57]

Should have a constant for '3'?

[newcomer, 2017/05/25 23:10:51]

Done.

+
+  display::Screen::GetScreen()->AddObserver(this);

[vadimt, 2017/05/22 23:36:57]

This line is paired with removing observer in destructor, but it's not clear
from the code, as this like isn't seemingly protected by 'IsFullscreen'
condition.
Could you do something to make it clearer?

[newcomer, 2017/05/25 23:10:52]

Done.

 }
 
 void AppListView::InitializeBubble(gfx::NativeView parent,
@@ -428,7 +473,7 @@ void AppListView::InitializeBubble(gfx::NativeView parent,
   set_shadow(views::BubbleBorder::NO_ASSETS);
 
   // This creates the app list widget. (Before this, child widgets cannot be
-  // created.)
+  // created.
   views::BubbleDialogDelegateView::CreateBubble(this);
 
   SetBubbleArrow(views::BubbleBorder::FLOAT);
@@ -476,6 +521,120 @@ void AppListView::GetWidgetHitTestMask(gfx::Path* mask) const {
   mask->addRect(gfx::RectToSkRect(GetBubbleFrameView()->GetContentsBounds()));
 }
 
+void AppListView::OnMouseEvent(ui::MouseEvent* event) {
+  if (features::IsFullscreenAppListEnabled()) {

[xiyuan, 2017/05/24 18:28:18]

nit: reverse condition and bail out early.

i.e.

  if (!features::IsFullscreenAppListEnabled())
    return;

  ...

[newcomer, 2017/05/25 23:10:51]

Done, also swapped if(features::IsFullscreenAppListEnabled()) for if
(launcher_background_shield_) because this gets called a lot.

+    if (event->type() == ui::ET_MOUSE_PRESSED ||

[vadimt, 2017/05/22 23:36:57]

I'd prefer switch statement.

[newcomer, 2017/05/25 23:10:52]

Done.

+        event->type() == ui::ET_MOUSE_DRAGGED ||
+        event->type() == ui::ET_MOUSE_RELEASED) {
+      HandleDrag(event->location(), event->type());
+      event->SetHandled();
+    }
+  }
+}
+
+void AppListView::OnGestureEvent(ui::GestureEvent* event) {
+  if (features::IsFullscreenAppListEnabled()) {

[xiyuan, 2017/05/24 18:28:18]

ditto

[newcomer, 2017/05/25 23:10:52]

Done.

+    if (event->type() == ui::ET_GESTURE_SCROLL_BEGIN ||
+        event->type() == ui::ET_GESTURE_SCROLL_UPDATE ||
+        event->type() == ui::ET_GESTURE_END) {
+      HandleDrag(event->location(), event->type());
+      event->SetHandled();
+      // Record the velocity so we can process it when the touch is released in
+      // HandleDrag.
+      last_fling_velocity_ = event->details().velocity_y();
+    }
+  }
+}
+
+void AppListView::HandleDrag(gfx::Point location, ui::EventType type) {
+  int new_y_position;
+  if (type != ui::ET_GESTURE_SCROLL_BEGIN && type != ui::ET_MOUSE_PRESSED)
+    // Calculate the new position of the launcher which may or may not be used.

[vadimt, 2017/05/22 23:36:57]

Use {}, as the statement in 'if' takes more than 1 line.

[newcomer, 2017/05/25 23:10:52]

Done.

+    new_y_position =

[vadimt, 2017/05/22 23:36:57]

'else' it will be uninitialized?

[newcomer, 2017/05/25 23:10:52]

Moved the initialization into the switch statement. The variable isn't used for
ET_GESTURE_SCROLL_BEGIN or ET_MOUSE_PRESSED.

+        location.y() - initial_drag_separation_ + fullscreen_widget_bounds_.y();
+
+  switch (type) {
+    case ui::ET_GESTURE_SCROLL_BEGIN:
+    case ui::ET_MOUSE_PRESSED: {
+      // Drag start, grab the initial_drag_separation_ to maintain the relative
+      // position of the top of the widget and the mouse/gesture.
+      initial_drag_separation_ = location.y();
+      fullscreen_widget_bounds_ = fullscreen_widget_->GetWindowBoundsInScreen();
+      return;
+    }
+    case ui::ET_GESTURE_SCROLL_UPDATE:
+    case ui::ET_MOUSE_DRAGGED: {
+      // Drag update, update the bounds of the widget while maintaining the
+      // relative position of the top of the widget and the mouse/gesture.
+      // Block drags north of 0 and recalculate the initial_drag_separation_.
+      if (new_y_position < 0) {
+        fullscreen_widget_bounds_.set_y(0);
+        initial_drag_separation_ = location.y();
+      } else
+        fullscreen_widget_bounds_.set_y(new_y_position);

[vadimt, 2017/05/22 23:36:57]

I assume you manually reproed cases when you need to go at all branches in the
new code? Also going to add tests for full coverage?

[newcomer, 2017/05/25 23:10:52]

Correct! The next CL will have full test coverage because it also includes
start_page_view state changes and the half launcher.

+
+      fullscreen_widget_->SetBounds(fullscreen_widget_bounds_);
+      return;

[vadimt, 2017/05/22 23:36:57]

Please use break as you don't use return as in early return pattern.

[newcomer, 2017/05/25 23:10:52]

Done.

+    }
+
+    case ui::ET_GESTURE_END:
+    case ui::ET_MOUSE_RELEASED: {
+      // Drag completion, either snap to fullscreen or close the App List based
+      // on where the drag ended. If fling velocity was over the threshold, snap
+      // to the next state in the direction of the fling.
+
+      if (std::abs(last_fling_velocity_) > 25) {

[vadimt, 2017/05/22 23:36:57]

Constant for 25?

[newcomer, 2017/05/25 23:10:52]

Done.

+        // If the user releases drag with velocity over the threshold, snap to
+        // the next state, ignoring the drag release position.
+

[vadimt, 2017/05/22 23:36:57]

Remove empty line.

[newcomer, 2017/05/25 23:10:51]

Done.

+        if (is_fullscreen_launcher_) {
+          if (last_fling_velocity_ > 0)
+            ToPeeking();
+        } else if (!is_fullscreen_launcher_) {

[vadimt, 2017/05/22 23:36:57]

No need to check !is_fullscreen_launcher_

[newcomer, 2017/05/25 23:10:52]

Done.

+          if (last_fling_velocity_ > 0) {
+            CloseAppList();
+          } else if (last_fling_velocity_ < 0) {
+            ToFullscreen();
+          }
+        }
+        last_fling_velocity_ = 0;
+        return;
+      } else {
+        // The drag release velocity was too low, so use the release point.
+        int launcher_snap_y =
+            is_fullscreen_launcher_ ? 0 : default_peeking_launcher_y_;
+        // If the user releases +/- 1/3 of launcher_threshold_ , snap to the
+        // next state.
+        if (std::abs(launcher_snap_y - new_y_position) <
+            (launcher_threshold_)) {
+          // If the drag released to the state above.
+          if (is_fullscreen_launcher_) {
+            ToFullscreen();
+          } else {
+            ToPeeking();
+          }
+        } else if ((launcher_snap_y + (launcher_threshold_)) < new_y_position) {
+          // If the user released to the state below.
+          if (is_fullscreen_launcher_) {
+            ToPeeking();
+          } else {
+            CloseAppList();
+          }
+
+        } else {
+          // if the user released to the state above, go to the state above.
+          ToFullscreen();
+        }
+
+        return;
+      }
+    }
+    default:
+      NOTREACHED();
+      return;
+  }
+}
+

[vadimt, 2017/05/22 23:36:57]

This method is over 100 lines; please split.

[newcomer, 2017/05/25 23:10:52]

It is now 81 lines.

 bool AppListView::AcceleratorPressed(const ui::Accelerator& accelerator) {
   DCHECK_EQ(ui::VKEY_ESCAPE, accelerator.key_code());
 
@@ -490,7 +649,31 @@ bool AppListView::AcceleratorPressed(const ui::Accelerator& accelerator) {
   return true;
 }
 
+void AppListView::ComputeConstants() {
+  display_work_area_bounds_ = display::Screen::GetScreen()
+                                  ->GetDisplayNearestView(parent_window())
+                                  .work_area();
+  default_peeking_launcher_y_ =
+      display_work_area_bounds_.height() + kShelfSize - kPeekingLauncherHeight;
+}
+
 void AppListView::Layout() {
+  if (features::IsFullscreenAppListEnabled()) {
+    ComputeConstants();
+
+    gfx::Rect recomputed_fullscreen_widget_bounds(
+        display_work_area_bounds_.x(), default_peeking_launcher_y_,
+        display_work_area_bounds_.width(),
+        display_work_area_bounds_.height() + kShelfSize);
+
+    fullscreen_widget_bounds_ = recomputed_fullscreen_widget_bounds;
+    // todo(newcomer): Support half launcher and all start_page_view states.
+    if (is_fullscreen_launcher_) {
+      ToFullscreen();
+    } else {
+      ToPeeking();
+    }
+  }
   const gfx::Rect contents_bounds = GetContentsBounds();
 
   // Make sure to layout |app_list_main_view_| and |speech_view_| at the center
@@ -511,6 +694,10 @@ void AppListView::Layout() {
     speech_bounds.Inset(-speech_view_->GetInsets());
     speech_view_->SetBoundsRect(speech_bounds);
   }
+  if (features::IsFullscreenAppListEnabled()) {
+    app_list_main_view_->contents_view()->start_page_view()->parent()->Layout();

[vadimt, 2017/05/22 23:36:57]

I'd explicitly plumb that view to here. Hierarchy may change.

[newcomer, 2017/05/25 23:10:53]

Done.

+    launcher_background_shield_->SetBoundsRect(contents_bounds);
+  }
 }
 
 void AppListView::SchedulePaintInRect(const gfx::Rect& rect) {
@@ -519,6 +706,27 @@ void AppListView::SchedulePaintInRect(const gfx::Rect& rect) {
     GetBubbleFrameView()->SchedulePaint();
 }
 
+void AppListView::ToFullscreen() {
+  fullscreen_widget_bounds_.set_y(0);
+  fullscreen_widget_->SetBounds(fullscreen_widget_bounds_);
+  is_fullscreen_launcher_ = true;
+  launcher_threshold_ = (fullscreen_widget_bounds_.height() + kShelfSize) / 3;

[vadimt, 2017/05/22 23:36:57]

3 again

[newcomer, 2017/05/25 23:10:52]

Done.

+}
+
+void AppListView::ToPeeking() {
+  fullscreen_widget_bounds_.set_y(default_peeking_launcher_y_);
+  fullscreen_widget_->SetBounds(fullscreen_widget_bounds_);
+  is_fullscreen_launcher_ = false;
+  // height is wrong

[vadimt, 2017/05/22 23:36:57]

Need a better comment.

[newcomer, 2017/05/25 23:10:51]

That was a prototyping comment. Removed!

+  launcher_threshold_ = (fullscreen_widget_bounds_.height() + kShelfSize -
+                         kPeekingLauncherHeight) /
+                        3;
+}
+
+bool AppListView::IsFullscreen() {

[vadimt, 2017/05/22 23:36:57]

Should be const method

[newcomer, 2017/05/25 23:10:52]

Done.

+  return is_fullscreen_launcher_;
+}
+
 void AppListView::OnWidgetDestroying(views::Widget* widget) {
   BubbleDialogDelegateView::OnWidgetDestroying(widget);
   if (delegate_ && widget == GetWidget())
@@ -603,4 +811,9 @@ void AppListView::OnSpeechRecognitionStateChanged(
   }
 }
 
+void AppListView::OnDisplayMetricsChanged(const display::Display& display,
+                                          uint32_t changed_metrics) {
+  Layout();
+}
+
 }  // namespace app_list