Commit cec3dc4c authored by xiyuan@chromium.org's avatar xiyuan@chromium.org

ash: Fix launcher icon overlaps with status.

- Make LauncherView::CalculateIdealBounds to return last visible index;
- In LauncherView::LauncherItemAdded, use the last visible index to determine if
  we need the animation;

BUG=122482
TEST=Verify fix for issue 122482.


Review URL: http://codereview.chromium.org/10068027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133603 0039d316-1c4b-4281-b951-d872f2087c98
parent 6e8ac9af
......@@ -346,9 +346,11 @@
'launcher/launcher_view_unittest.cc',
'monitor/multi_monitor_manager_unittest.cc',
'shell_unittest.cc',
'test/ash_unittests.cc',
'test/ash_test_base.cc',
'test/ash_test_base.h',
'test/ash_unittests.cc',
'test/launcher_view_test_api.cc',
'test/launcher_view_test_api.h',
'test/test_activation_delegate.cc',
'test/test_activation_delegate.h',
'test/test_launcher_delegate.cc',
......
......@@ -9,6 +9,7 @@
#include "ash/shell.h"
#include "ash/test/ash_test_base.h"
#include "ash/test/launcher_view_test_api.h"
#include "ui/views/view.h"
#include "ui/views/widget/widget.h"
......@@ -36,7 +37,7 @@ TEST_F(LauncherTest, OpenBrowser) {
Launcher* launcher = Shell::GetInstance()->launcher();
ASSERT_TRUE(launcher);
LauncherView* launcher_view = launcher->GetLauncherViewForTest();
LauncherView::TestAPI test(launcher_view);
test::LauncherViewTestAPI test(launcher_view);
LauncherModel* model = launcher->model();
// Initially we have the app list and chrome icon.
......
......@@ -182,6 +182,7 @@ void ReflectItemStatus(const ash::LauncherItem& item,
break;
}
}
} // namespace
// AnimationDelegate used when inserting a new item. This steadily decreased the
......@@ -226,7 +227,6 @@ class LauncherView::StartFadeAnimationDelegate :
// AnimationDelegate overrides:
virtual void AnimationEnded(const Animation* animation) OVERRIDE {
view_->SetVisible(true);
launcher_view_->FadeIn(view_);
}
virtual void AnimationCanceled(const Animation* animation) OVERRIDE {
......@@ -240,18 +240,6 @@ class LauncherView::StartFadeAnimationDelegate :
DISALLOW_COPY_AND_ASSIGN(StartFadeAnimationDelegate);
};
int LauncherView::TestAPI::GetButtonCount() {
return launcher_view_->view_model_->view_size();
}
LauncherButton* LauncherView::TestAPI::GetButton(int index) {
if (index == 0)
return NULL;
return static_cast<LauncherButton*>(
launcher_view_->view_model_->view_at(index));
}
LauncherView::LauncherView(LauncherModel* model, LauncherDelegate* delegate)
: model_(model),
delegate_(delegate),
......@@ -369,20 +357,14 @@ void LauncherView::CalculateIdealBounds(IdealBounds* bounds) {
last_visible_index_ = DetermineLastVisibleIndex(
available_width - kLeadingInset - bounds->overflow_bounds.width() -
kButtonSpacing - kButtonWidth);
bool show_overflow =
(last_visible_index_ + 1 != view_model_->view_size());
int app_list_index = view_model_->view_size() - 1;
if (overflow_button_->visible() != show_overflow) {
// Only change visibility of the views if the visibility of the overflow
// button changes. Otherwise we'll effect the insertion animation, which
// changes the visibility.
for (int i = 0; i <= last_visible_index_; ++i)
view_model_->view_at(i)->SetVisible(true);
for (int i = last_visible_index_ + 1; i < view_model_->view_size(); ++i) {
if (i != app_list_index)
view_model_->view_at(i)->SetVisible(false);
}
bool show_overflow = (last_visible_index_ + 1 < app_list_index);
for (int i = 0; i < view_model_->view_size(); ++i) {
view_model_->view_at(i)->SetVisible(
i == app_list_index || i <= last_visible_index_);
}
overflow_button_->SetVisible(show_overflow);
if (show_overflow) {
DCHECK_NE(0, view_model_->view_size());
......@@ -688,11 +670,13 @@ void LauncherView::LauncherItemAdded(int model_index) {
views::View* view = CreateViewForItem(model_->items()[model_index]);
AddChildView(view);
// Hide the view, it'll be made visible when the animation is done.
view->SetVisible(false);
// Hide the view, it'll be made visible when the animation is done. Using
// opacity 0 here to avoid messing with CalculateIdealBounds which touches
// the view's visibility.
view->layer()->SetOpacity(0);
view_model_->Add(view, model_index);
// Give the button it's ideal bounds. That way if we end up animating the
// Give the button its ideal bounds. That way if we end up animating the
// button before this animation completes it doesn't appear at some random
// spot (because it was in the middle of animating from 0,0 0x0 to its
// target).
......@@ -700,13 +684,16 @@ void LauncherView::LauncherItemAdded(int model_index) {
CalculateIdealBounds(&ideal_bounds);
view->SetBoundsRect(view_model_->ideal_bounds(model_index));
// The first animation moves all the views to their target position. |view| is
// hidden, so it visually appears as though we are providing space for
// The first animation moves all the views to their target position. |view|
// is hidden, so it visually appears as though we are providing space for
// it. When done we'll fade the view in.
AnimateToIdealBounds();
if (!overflow_button_->visible()) {
if (model_index <= last_visible_index_) {
bounds_animator_->SetAnimationDelegate(
view, new StartFadeAnimationDelegate(this, view), true);
} else {
// Undo the hiding if animation does not run.
view->layer()->SetOpacity(1.0f);
}
FOR_EACH_OBSERVER(LauncherIconObserver, observers_,
......
......@@ -26,6 +26,10 @@ class ViewModel;
namespace ash {
namespace test {
class LauncherViewTestAPI;
}
class LauncherDelegate;
struct LauncherItem;
class LauncherIconObserver;
......@@ -42,23 +46,6 @@ class ASH_EXPORT LauncherView : public views::View,
public views::ContextMenuController,
public views::FocusTraversable {
public:
// Use the api in this class for testing only.
class ASH_EXPORT TestAPI {
public:
explicit TestAPI(LauncherView* launcher_view)
: launcher_view_(launcher_view) {
}
// Number of icons displayed.
int GetButtonCount();
// Retrieve the button at |index|.
LauncherButton* GetButton(int index);
private:
LauncherView* launcher_view_;
DISALLOW_COPY_AND_ASSIGN(TestAPI);
};
LauncherView(LauncherModel* model, LauncherDelegate* delegate);
virtual ~LauncherView();
......@@ -80,6 +67,8 @@ class ASH_EXPORT LauncherView : public views::View,
virtual View* GetFocusTraversableParentView() OVERRIDE;
private:
friend class ash::test::LauncherViewTestAPI;
class FadeOutAnimationDelegate;
class StartFadeAnimationDelegate;
......
......@@ -4,23 +4,28 @@
#include "ash/launcher/launcher_view.h"
#include "ash/ash_switches.h"
#include "ash/launcher/launcher.h"
#include "ash/launcher/launcher_button.h"
#include "ash/launcher/launcher_model.h"
#include "ash/launcher/launcher_icon_observer.h"
#include "ash/shell.h"
#include "ash/shell_window_ids.h"
#include "ash/test/ash_test_base.h"
#include "ash/test/launcher_view_test_api.h"
#include "ash/test/test_launcher_delegate.h"
#include "base/basictypes.h"
#include "base/command_line.h"
#include "base/compiler_specific.h"
#include "base/memory/scoped_ptr.h"
#include "ui/aura/window.h"
#include "ui/aura/test/aura_test_base.h"
#include "ui/gfx/compositor/layer.h"
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_delegate.h"
namespace ash {
namespace test {
namespace {
////////////////////////////////////////////////////////////////////////////////
// LauncherIconObserver tests.
class TestLauncherIconObserver : public LauncherIconObserver {
public:
......@@ -34,7 +39,7 @@ class TestLauncherIconObserver : public LauncherIconObserver {
// LauncherIconObserver implementation.
void OnLauncherIconPositionsChanged() OVERRIDE {
count_++;
++count_;
}
int count() const { return count_; }
......@@ -46,10 +51,10 @@ class TestLauncherIconObserver : public LauncherIconObserver {
DISALLOW_COPY_AND_ASSIGN(TestLauncherIconObserver);
};
class LauncherViewTest : public ash::test::AshTestBase {
class LauncherViewIconObserverTest : public ash::test::AshTestBase {
public:
LauncherViewTest() {}
virtual ~LauncherViewTest() {}
LauncherViewIconObserverTest() {}
virtual ~LauncherViewIconObserverTest() {}
virtual void SetUp() OVERRIDE {
AshTestBase::SetUp();
......@@ -66,10 +71,10 @@ class LauncherViewTest : public ash::test::AshTestBase {
private:
scoped_ptr<TestLauncherIconObserver> observer_;
DISALLOW_COPY_AND_ASSIGN(LauncherViewTest);
DISALLOW_COPY_AND_ASSIGN(LauncherViewIconObserverTest);
};
TEST_F(LauncherViewTest, AddRemove) {
TEST_F(LauncherViewIconObserverTest, AddRemove) {
ash::test::TestLauncherDelegate* launcher_delegate =
ash::test::TestLauncherDelegate::instance();
ASSERT_TRUE(launcher_delegate);
......@@ -90,7 +95,7 @@ TEST_F(LauncherViewTest, AddRemove) {
observer()->Reset();
}
TEST_F(LauncherViewTest, BoundsChanged) {
TEST_F(LauncherViewIconObserverTest, BoundsChanged) {
Launcher* launcher = Shell::GetInstance()->launcher();
int total_width = launcher->widget()->GetWindowScreenBounds().width();
ASSERT_GT(total_width, 0);
......@@ -99,6 +104,156 @@ TEST_F(LauncherViewTest, BoundsChanged) {
observer()->Reset();
}
} // namespace
////////////////////////////////////////////////////////////////////////////////
// LauncherView tests.
class LauncherViewTest : public aura::test::AuraTestBase {
public:
LauncherViewTest() {}
virtual ~LauncherViewTest() {}
virtual void SetUp() OVERRIDE {
aura::test::AuraTestBase::SetUp();
model_.reset(new LauncherModel);
launcher_view_.reset(new internal::LauncherView(model_.get(), NULL));
launcher_view_->Init();
// The bounds should be big enough for 4 buttons + overflow chevron.
launcher_view_->SetBounds(0, 0, 500, 50);
test_api_.reset(new LauncherViewTestAPI(launcher_view_.get()));
test_api_->SetAnimationDuration(1); // Speeds up animation for test.
}
protected:
LauncherID AddAppShortcut() {
LauncherItem item;
item.type = TYPE_APP_SHORTCUT;
item.status = STATUS_CLOSED;
int id = model_->next_id();
model_->Add(item);
test_api_->RunMessageLoopUntilAnimationsDone();
return id;
}
LauncherID AddTabbedBrowser() {
LauncherItem item;
item.type = TYPE_TABBED;
item.status = STATUS_RUNNING;
int id = model_->next_id();
model_->Add(item);
test_api_->RunMessageLoopUntilAnimationsDone();
return id;
}
void RemoveByID(LauncherID id) {
model_->RemoveItemAt(model_->ItemIndexByID(id));
test_api_->RunMessageLoopUntilAnimationsDone();
}
internal::LauncherButton* GetButtonByID(LauncherID id) {
int index = model_->ItemIndexByID(id);
return test_api_->GetButton(index);
}
scoped_ptr<LauncherModel> model_;
scoped_ptr<internal::LauncherView> launcher_view_;
scoped_ptr<LauncherViewTestAPI> test_api_;
private:
DISALLOW_COPY_AND_ASSIGN(LauncherViewTest);
};
// Adds browser button until overflow and verifies that the last added browser
// button is hidden.
TEST_F(LauncherViewTest, AddBrowserUntilOverflow) {
// All buttons should be visible.
ASSERT_EQ(test_api_->GetLastVisibleIndex() + 1,
test_api_->GetButtonCount());
// Add tabbed browser until overflow.
LauncherID last_added = AddTabbedBrowser();
while (!test_api_->IsOverflowButtonVisible()) {
// Added button is visible after animation while in this loop.
EXPECT_TRUE(GetButtonByID(last_added)->visible());
last_added = AddTabbedBrowser();
}
// The last added button should be invisible.
EXPECT_FALSE(GetButtonByID(last_added)->visible());
}
// Adds one browser button then adds app shortcut until overflow. Verifies that
// the browser button gets hidden on overflow and last added app shortcut is
// still visible.
TEST_F(LauncherViewTest, AddAppShortcutWithBrowserButtonUntilOverflow) {
// All buttons should be visible.
ASSERT_EQ(test_api_->GetLastVisibleIndex() + 1,
test_api_->GetButtonCount());
LauncherID browser_button_id = AddTabbedBrowser();
// Add app shortcut until overflow.
LauncherID last_added = AddAppShortcut();
while (!test_api_->IsOverflowButtonVisible()) {
// Added button is visible after animation while in this loop.
EXPECT_TRUE(GetButtonByID(last_added)->visible());
last_added = AddAppShortcut();
}
// The last added app short button should be visible.
EXPECT_TRUE(GetButtonByID(last_added)->visible());
// And the browser button is invisible.
EXPECT_FALSE(GetButtonByID(browser_button_id)->visible());
}
// Adds button until overflow then removes first added one. Verifies that
// the last added one changes from invisible to visible and overflow
// chevron is gone.
TEST_F(LauncherViewTest, RemoveButtonRevealsOverflowed) {
// All buttons should be visible.
ASSERT_EQ(test_api_->GetLastVisibleIndex() + 1,
test_api_->GetButtonCount());
// Add tabbed browser until overflow.
LauncherID first_added= AddTabbedBrowser();
LauncherID last_added = first_added;
while (!test_api_->IsOverflowButtonVisible())
last_added = AddTabbedBrowser();
// Expect add more than 1 button. First added is visible and last is not.
EXPECT_NE(first_added, last_added);
EXPECT_TRUE(GetButtonByID(first_added)->visible());
EXPECT_FALSE(GetButtonByID(last_added)->visible());
// Remove first added.
RemoveByID(first_added);
// Last added button becomes visible and overflow chevron is gone.
EXPECT_TRUE(GetButtonByID(last_added)->visible());
EXPECT_EQ(1.0f, GetButtonByID(last_added)->layer()->opacity());
EXPECT_FALSE(test_api_->IsOverflowButtonVisible());
}
// Verifies that remove last overflowed button should hide overflow chevron.
TEST_F(LauncherViewTest, RemoveLastOverflowed) {
// All buttons should be visible.
ASSERT_EQ(test_api_->GetLastVisibleIndex() + 1,
test_api_->GetButtonCount());
// Add tabbed browser until overflow.
LauncherID last_added= AddTabbedBrowser();
while (!test_api_->IsOverflowButtonVisible())
last_added = AddTabbedBrowser();
RemoveByID(last_added);
EXPECT_FALSE(test_api_->IsOverflowButtonVisible());
}
} // namespace test
} // namespace ash
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "ash/test/launcher_view_test_api.h"
#include "ash/launcher/launcher_button.h"
#include "ash/launcher/launcher_view.h"
#include "base/message_loop.h"
#include "ui/views/animation/bounds_animator.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/view_model.h"
namespace {
// A class used to wait for animations.
class TestAPIAnimationObserver : public views::BoundsAnimatorObserver {
public:
TestAPIAnimationObserver() {}
virtual ~TestAPIAnimationObserver() {}
// views::BoundsAnimatorObserver overrides:
virtual void OnBoundsAnimatorDone(views::BoundsAnimator* animator) OVERRIDE {
MessageLoop::current()->Quit();
}
private:
DISALLOW_COPY_AND_ASSIGN(TestAPIAnimationObserver);
};
} // namespace
namespace ash {
namespace test {
LauncherViewTestAPI::LauncherViewTestAPI(internal::LauncherView* launcher_view)
: launcher_view_(launcher_view) {
}
LauncherViewTestAPI::~LauncherViewTestAPI() {
}
int LauncherViewTestAPI::GetButtonCount() {
return launcher_view_->view_model_->view_size();
}
internal::LauncherButton* LauncherViewTestAPI::GetButton(int index) {
// App list button is not a LauncherButton.
if (index == GetButtonCount() - 1)
return NULL;
return static_cast<internal::LauncherButton*>(
launcher_view_->view_model_->view_at(index));
}
int LauncherViewTestAPI::GetLastVisibleIndex() {
return launcher_view_->last_visible_index_;
}
bool LauncherViewTestAPI::IsOverflowButtonVisible() {
return launcher_view_->overflow_button_->visible();
}
void LauncherViewTestAPI::SetAnimationDuration(int duration_ms) {
launcher_view_->bounds_animator_->SetAnimationDuration(duration_ms);
}
void LauncherViewTestAPI::RunMessageLoopUntilAnimationsDone() {
if (!launcher_view_->bounds_animator_->IsAnimating())
return;
scoped_ptr<TestAPIAnimationObserver> observer(new TestAPIAnimationObserver());
launcher_view_->bounds_animator_->AddObserver(observer.get());
// This nested loop will quit when TestAPIAnimationObserver's
// OnBoundsAnimatorDone is called.
MessageLoop::current()->Run();
launcher_view_->bounds_animator_->RemoveObserver(observer.get());
}
} // namespace test
} // namespace ash
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef ASH_TEST_LAUNCHER_VIEW_TEST_API_H_
#define ASH_TEST_LAUNCHER_VIEW_TEST_API_H_
#pragma once
#include "base/basictypes.h"
namespace ash {
namespace internal {
class LauncherButton;
class LauncherView;
}
namespace test {
// Use the api in this class to test LauncherView.
class LauncherViewTestAPI {
public:
explicit LauncherViewTestAPI(internal::LauncherView* launcher_view);
~LauncherViewTestAPI();
// Number of icons displayed.
int GetButtonCount();
// Retrieve the button at |index|.
internal::LauncherButton* GetButton(int index);
// Last visible button index.
int GetLastVisibleIndex();
// Returns true if overflow button is visible.
bool IsOverflowButtonVisible();
// Sets animation duration in milliseconds for test.
void SetAnimationDuration(int duration_ms);
// Runs message loop and waits until all add/remove animations are done.
void RunMessageLoopUntilAnimationsDone();
private:
internal::LauncherView* launcher_view_;
DISALLOW_COPY_AND_ASSIGN(LauncherViewTestAPI);
};
} // namespace test
} // namespace ash
#endif // ASH_TEST_LAUNCHER_VIEW_TEST_API_H_
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
......@@ -10,7 +10,7 @@
#include "ui/views/view.h"
// Duration in milliseconds for animations.
static const int kAnimationDuration = 200;
static const int kDefaultAnimationDuration = 200;
using ui::Animation;
using ui::AnimationContainer;
......@@ -21,8 +21,8 @@ namespace views {
BoundsAnimator::BoundsAnimator(View* parent)
: parent_(parent),
observer_(NULL),
container_(new AnimationContainer()) {
container_(new AnimationContainer()),
animation_duration_ms_(kDefaultAnimationDuration) {
container_->set_observer(this);
}
......@@ -137,10 +137,22 @@ void BoundsAnimator::Cancel() {
AnimationContainerProgressed(container_.get());
}
void BoundsAnimator::SetAnimationDuration(int duration_ms) {
animation_duration_ms_ = duration_ms;
}
void BoundsAnimator::AddObserver(BoundsAnimatorObserver* observer) {
observers_.AddObserver(observer);
}
void BoundsAnimator::RemoveObserver(BoundsAnimatorObserver* observer) {
observers_.RemoveObserver(observer);
}
SlideAnimation* BoundsAnimator::CreateAnimation() {
SlideAnimation* animation = new SlideAnimation(this);
animation->SetContainer(container_.get());
animation->SetSlideDuration(kAnimationDuration);
animation->SetSlideDuration(animation_duration_ms_);
animation->SetTweenType(Tween::EASE_OUT);
return animation;
}
......@@ -249,10 +261,12 @@ void BoundsAnimator::AnimationContainerProgressed(
repaint_bounds_.SetRect(0, 0, 0, 0);
}
if (observer_ && !IsAnimating()) {
if (!IsAnimating()) {
// Notify here rather than from AnimationXXX to avoid deleting the animation
// while the animation is calling us.
observer_->OnBoundsAnimatorDone(this);
FOR_EACH_OBSERVER(BoundsAnimatorObserver,
observers_,
OnBoundsAnimatorDone(this));
}
}
......
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
......@@ -10,6 +10,7 @@
#include "base/compiler_specific.h"
#include "base/memory/ref_counted.h"
#include "base/observer_list.h"
#include "ui/base/animation/animation_container_observer.h"
#include "ui/base/animation/animation_delegate.h"
#include "ui/gfx/rect.h"
......@@ -24,7 +25,7 @@ namespace views {
class BoundsAnimator;
class View;
class BoundsAnimatorObserver {
class VIEWS_EXPORT BoundsAnimatorObserver {
public:
// Invoked when all animations are complete.
virtual void OnBoundsAnimatorDone(BoundsAnimator* animator) = 0;
......@@ -92,9 +93,12 @@ class VIEWS_EXPORT BoundsAnimator : public ui::AnimationDelegate,
// size. Any views marked for deletion are deleted.
void Cancel();
void set_observer(BoundsAnimatorObserver* observer) {
observer_ = observer;
}
// Overrides default animation duration. |duration_ms| is the new duration in
// milliseconds.
void SetAnimationDuration(int duration_ms);
void AddObserver(BoundsAnimatorObserver* observer);
void RemoveObserver(BoundsAnimatorObserver* observer);
protected:
// Creates the animation to use for animating views.
......@@ -165,7 +169,7 @@ class VIEWS_EXPORT BoundsAnimator : public ui::AnimationDelegate,
// Parent of all views being animated.
View* parent_;
BoundsAnimatorObserver* observer_;
ObserverList<BoundsAnimatorObserver> observers_;
// All animations we create up with the same container.
scoped_refptr<ui::AnimationContainer> container_;
......@@ -182,6 +186,8 @@ class VIEWS_EXPORT BoundsAnimator : public ui::AnimationDelegate,
// to repaint these bounds.
gfx::Rect repaint_bounds_;
int animation_duration_ms_;
DISALLOW_COPY_AND_ASSIGN(BoundsAnimator);
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment