Animate in/out placeholder and disable section animations on init.

Section animations are disabled on initialization as holding space
child bubbles will be animated in with their contents already loaded.
Note that the child bubble animation has not yet been implemented.

Also note that this CL also moves animation logic from
HoldingSpaceItemView into HoldingSpaceItemViewsSection since a refactor
has given that class visibility to the parent container of item views.

Bug: 1154998
Change-Id: I32e311f8193da87b5f42e2b4a3941a22bc8a1d2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599447
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839118}

ash/system/holding_space/holding_space_item_view.cc

@@ -15,10 +15,6 @@
 #include "base/bind.h"
 #include "ui/base/class_property.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
-#include "ui/compositor/layer_animation_element.h"
-#include "ui/compositor/layer_animation_observer.h"
-#include "ui/compositor/layer_animation_sequence.h"
-#include "ui/compositor/layer_animator.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/paint_vector_icon.h"
@@ -37,11 +33,6 @@ namespace ash {
 
 namespace {
 
-// Animation.
-constexpr base::TimeDelta kAnimationDuration =
-    base::TimeDelta::FromMilliseconds(167);
-constexpr SkScalar kAnimationTranslationY = 20;
-
 // A UI class property used to identify if a view is an instance of
 // `HoldingSpaceItemView`. Class name is not an adequate identifier as it may be
 // overridden by subclasses.
@@ -49,24 +40,6 @@ DEFINE_UI_CLASS_PROPERTY_KEY(bool, kIsHoldingSpaceItemViewProperty, false)
 
 // Helpers ---------------------------------------------------------------------
 
-// Creates a `ui::LayerAnimationSequence` for the specified `element` observed
-// by the specified `observer`.
-std::unique_ptr<ui::LayerAnimationSequence> CreateObservedSequence(
-    std::unique_ptr<ui::LayerAnimationElement> element,
-    ui::LayerAnimationObserver* observer) {
-  auto sequence = std::make_unique<ui::LayerAnimationSequence>();
-  sequence->AddElement(std::move(element));
-  sequence->AddObserver(observer);
-  return sequence;
-}
-
-// Creates a `gfx:Transform` for the specified `x` and `y` offsets.
-gfx::Transform CreateTransformFromOffset(SkScalar x, SkScalar y) {
-  gfx::Transform transform;
-  transform.Translate(x, y);
-  return transform;
-}
-
 // Schedules repaint of `layer`.
 void InvalidateLayer(ui::Layer* layer) {
   layer->SchedulePaint(gfx::Rect(layer->size()));
@@ -134,15 +107,6 @@ HoldingSpaceItemView::HoldingSpaceItemView(
   SetPaintToLayer();
   layer()->SetFillsBoundsOpaquely(false);
 
-  // This view will be animated in. Set initial opacity and transform so that
-  // the next call to `AnimateIn()` will fade in and translate the view into
-  // place. Note that preemption strategy is also set to cause any calls to
-  // `AnimateIn()`/`AnimateOut()` to preempt any in-progress animation.
-  layer()->SetOpacity(0.f);
-  layer()->SetTransform(CreateTransformFromOffset(0, kAnimationTranslationY));
-  layer()->GetAnimator()->set_preemption_strategy(
-      ui::LayerAnimator::PreemptionStrategy::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
-
   // Focus.
   SetFocusBehavior(FocusBehavior::ALWAYS);
   focused_layer_owner_ =
@@ -261,16 +225,6 @@ void HoldingSpaceItemView::OnHoldingSpaceItemUpdated(
     GetViewAccessibility().OverrideName(item->text());
 }
 
-void HoldingSpaceItemView::AnimateIn(ui::LayerAnimationObserver* observer) {
-  AnimateImmediatelyTo(/*opacity=*/1.f, gfx::Transform(), observer);
-}
-
-void HoldingSpaceItemView::AnimateOut(ui::LayerAnimationObserver* observer) {
-  AnimateImmediatelyTo(/*opacity=*/0.f,
-                       CreateTransformFromOffset(0, -kAnimationTranslationY),
-                       observer);
-}
-
 void HoldingSpaceItemView::StartDrag(const ui::LocatedEvent& event,
                                      ui::mojom::DragEventSource source) {
   int drag_operations = GetDragOperations(event.location());
@@ -403,28 +357,6 @@ void HoldingSpaceItemView::UpdatePin() {
   OnPinVisiblityChanged(true);
 }
 
-void HoldingSpaceItemView::AnimateImmediatelyTo(
-    float opacity,
-    const gfx::Transform& transform,
-    ui::LayerAnimationObserver* observer) {
-  // Opacity animation.
-  auto opacity_element = ui::LayerAnimationElement::CreateOpacityElement(
-      opacity, kAnimationDuration);
-  opacity_element->set_tween_type(gfx::Tween::Type::LINEAR);
-
-  // Transform animation.
-  auto transform_element = ui::LayerAnimationElement::CreateTransformElement(
-      transform, kAnimationDuration);
-  transform_element->set_tween_type(gfx::Tween::Type::EASE_OUT_3);
-
-  // Note that the `ui::LayerAnimator` takes ownership of any animation
-  // sequences so they need to be released.
-  layer()->GetAnimator()->StartTogether(
-      {CreateObservedSequence(std::move(opacity_element), observer).release(),
-       CreateObservedSequence(std::move(transform_element), observer)
-           .release()});
-}
-
 BEGIN_METADATA(HoldingSpaceItemView, views::InkDropHostView)
 END_METADATA
 

ash/system/holding_space/holding_space_item_view.h

@@ -14,10 +14,6 @@
 #include "ui/views/animation/ink_drop_host_view.h"
 #include "ui/views/metadata/metadata_header_macros.h"
 
-namespace ui {
-class LayerAnimationObserver;
-}  // namespace ui
-
 namespace views {
 class InkDropContainerView;
 class ToggleImageButton;
@@ -66,11 +62,6 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView,
   // HoldingSpaceModelObserver:
   void OnHoldingSpaceItemUpdated(const HoldingSpaceItem* item) override;
 
-  // Invoked to initiate animate in/out of this view. Any animations created
-  // will be associated with the specified `observer`.
-  void AnimateIn(ui::LayerAnimationObserver* observer);
-  void AnimateOut(ui::LayerAnimationObserver* observer);
-
   // Starts a drag from this view at the location specified by the given `event`
   // and with the specified `source`. Note that this method copies the logic of
   // `views::View::DoDrag()` as a workaround to that API being private.
@@ -93,13 +84,6 @@ class ASH_EXPORT HoldingSpaceItemView : public views::InkDropHostView,
   void OnPinPressed();
   void UpdatePin();
 
-  // Animates this view to the specified `opacity` and `transform`, preempting
-  // any in-progress animations. Any animations created will be associated with
-  // the specified `observer`.
-  void AnimateImmediatelyTo(float opacity,
-                            const gfx::Transform& transform,
-                            ui::LayerAnimationObserver* observer);
-
   HoldingSpaceItemViewDelegate* const delegate_;
   const HoldingSpaceItem* const item_;
 

ash/system/holding_space/holding_space_item_views_section.cc

@@ -7,7 +7,12 @@
 #include "ash/public/cpp/holding_space/holding_space_constants.h"
 #include "ash/system/holding_space/holding_space_item_view.h"
 #include "ash/system/holding_space/holding_space_item_view_delegate.h"
+#include "base/auto_reset.h"
 #include "ui/compositor/callback_layer_animation_observer.h"
+#include "ui/compositor/layer_animation_element.h"
+#include "ui/compositor/layer_animation_observer.h"
+#include "ui/compositor/layer_animation_sequence.h"
+#include "ui/compositor/layer_animator.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/style/platform_style.h"
@@ -16,10 +21,90 @@ namespace ash {
 
 namespace {
 
+// Animation.
+constexpr base::TimeDelta kAnimationDuration =
+    base::TimeDelta::FromMilliseconds(167);
+constexpr SkScalar kAnimationTranslationY = 20;
+
 // Value returned during notification of animation completion events in order to
 // delete the observer which provided notification.
 constexpr bool kDeleteObserver = true;
 
+// Helpers ---------------------------------------------------------------------
+
+// Initializes the layer for the specified `view` for animations.
+void InitLayerForAnimations(views::View* view) {
+  view->SetPaintToLayer();
+  view->layer()->SetFillsBoundsOpaquely(false);
+  view->layer()->GetAnimator()->set_preemption_strategy(
+      ui::LayerAnimator::PreemptionStrategy::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+}
+
+// Creates a `ui::LayerAnimationSequence` for the specified `element` observed
+// by the specified `observer`.
+std::unique_ptr<ui::LayerAnimationSequence> CreateObservedSequence(
+    std::unique_ptr<ui::LayerAnimationElement> element,
+    ui::LayerAnimationObserver* observer) {
+  auto sequence = std::make_unique<ui::LayerAnimationSequence>();
+  sequence->AddElement(std::move(element));
+  sequence->AddObserver(observer);
+  return sequence;
+}
+
+// Creates a `gfx:Transform` for the specified `x` and `y` offsets.
+gfx::Transform CreateTransformFromOffset(SkScalar x, SkScalar y) {
+  gfx::Transform transform;
+  transform.Translate(x, y);
+  return transform;
+}
+
+// Animates the specified `view` to a target `opacity` and `transform` with the
+// specified `duration`, associating `observer` with the created animation
+// sequences.
+void DoAnimateTo(views::View* view,
+                 float opacity,
+                 const gfx::Transform& transform,
+                 base::TimeDelta duration,
+                 ui::LayerAnimationObserver* observer) {
+  // Opacity animation.
+  auto opacity_element =
+      ui::LayerAnimationElement::CreateOpacityElement(opacity, duration);
+  opacity_element->set_tween_type(gfx::Tween::Type::LINEAR);
+
+  // Transform animation.
+  auto transform_element =
+      ui::LayerAnimationElement::CreateTransformElement(transform, duration);
+  transform_element->set_tween_type(gfx::Tween::Type::EASE_OUT_3);
+
+  // Note that the `ui::LayerAnimator` takes ownership of any animation
+  // sequences so they need to be released.
+  view->layer()->GetAnimator()->StartTogether(
+      {CreateObservedSequence(std::move(opacity_element), observer).release(),
+       CreateObservedSequence(std::move(transform_element), observer)
+           .release()});
+}
+
+// Animates in the specified `view` with the specified `duration`, associating
+// `observer` with the created animation sequences.
+void DoAnimateIn(views::View* view,
+                 base::TimeDelta duration,
+                 ui::LayerAnimationObserver* observer) {
+  view->layer()->SetOpacity(0.f);
+  view->layer()->SetTransform(
+      CreateTransformFromOffset(0, kAnimationTranslationY));
+  DoAnimateTo(view, /*opacity=*/1.f, gfx::Transform(), duration, observer);
+}
+
+// Animates out the specified `view` with the specified `duration, associating
+// `observer` with the created animation sequences.
+void DoAnimateOut(views::View* view,
+                  base::TimeDelta duration,
+                  ui::LayerAnimationObserver* observer) {
+  DoAnimateTo(view, /*opacity=*/0.f,
+              CreateTransformFromOffset(0, -kAnimationTranslationY), duration,
+              observer);
+}
+
 // HoldingSpaceScrollView ------------------------------------------------------
 
 class HoldingSpaceScrollView : public views::ScrollView,
@@ -109,28 +194,34 @@ void HoldingSpaceItemViewsSection::Init() {
   // access to all contained item views.
   if (max_count_.has_value()) {
     container_ = AddChildView(CreateContainer());
-    container_->SetVisible(false);
   } else {
     auto* scroll = AddChildView(std::make_unique<HoldingSpaceScrollView>());
     scroll->SetBackgroundColor(base::nullopt);
     scroll->ClipHeightTo(0, INT_MAX);
     scroll->SetDrawOverflowIndicator(false);
     container_ = scroll->SetContents(CreateContainer());
-    container_->SetVisible(false);
   }
 
+  InitLayerForAnimations(container_);
+  container_->SetVisible(false);
+
   // Placeholder.
   auto placeholder = CreatePlaceholder();
   if (placeholder) {
     placeholder_ = AddChildView(std::move(placeholder));
+    InitLayerForAnimations(placeholder_);
     placeholder_->SetVisible(true);
     header_->SetVisible(true);
   }
 
   // Views.
   HoldingSpaceModel* model = HoldingSpaceController::Get()->model();
-  if (model)
+  if (model) {
+    // Sections are not animated during initialization as their respective
+    // bubbles will be animated in instead.
+    base::AutoReset<bool> scoped_disable_animations(&disable_animations_, true);
     OnHoldingSpaceModelAttached(model);
+  }
 }
 
 void HoldingSpaceItemViewsSection::Reset() {
@@ -266,29 +357,29 @@ void HoldingSpaceItemViewsSection::MaybeAnimateOut() {
   animate_out_observer->SetActive();
 }
 
-// TODO(dmblack): Handle animate in of `placeholder_`.
 // TODO(dmblack): Handle grow/shrink of container.
 void HoldingSpaceItemViewsSection::AnimateIn(
     ui::LayerAnimationObserver* observer) {
+  const base::TimeDelta animation_duration =
+      disable_animations_ ? base::TimeDelta() : kAnimationDuration;
   if (views_by_item_id_.empty() && placeholder_) {
-    DCHECK(placeholder_->GetVisible());
+    DoAnimateIn(placeholder_, animation_duration, observer);
     return;
   }
-  for (auto& view_by_item_id : views_by_item_id_)
-    view_by_item_id.second->AnimateIn(observer);
+  DoAnimateIn(container_, animation_duration, observer);
 }
 
-// TODO(dmblack): Handle animate out of `placeholder_`.
 // TODO(dmblack): Handle animate out of `header_` if this section is leaving.
 void HoldingSpaceItemViewsSection::AnimateOut(
     ui::LayerAnimationObserver* observer) {
+  const base::TimeDelta animation_duration =
+      disable_animations_ ? base::TimeDelta() : kAnimationDuration;
   if (placeholder_ && placeholder_->GetVisible()) {
     DCHECK(views_by_item_id_.empty());
-    placeholder_->SetVisible(false);
+    DoAnimateOut(placeholder_, animation_duration, observer);
     return;
   }
-  for (auto& view_by_item_id : views_by_item_id_)
-    view_by_item_id.second->AnimateOut(observer);
+  DoAnimateOut(container_, animation_duration, observer);
 }
 
 bool HoldingSpaceItemViewsSection::OnAnimateInCompleted(

ash/system/holding_space/holding_space_item_views_section.h

@@ -133,6 +133,10 @@ class HoldingSpaceItemViewsSection : public views::View,
   // animation is preempting another.
   uint32_t animation_state_ = AnimationState::kNotAnimating;
 
+  // Whether or not animations are disabled. Animations are only disabled during
+  // initialization as holding space child bubbles are animated in instead.
+  bool disable_animations_ = false;
+
   base::ScopedObservation<HoldingSpaceController,
                           HoldingSpaceControllerObserver>
       controller_observer_{this};