Commit 4cc695c4 authored by Tetsui Ohkubo's avatar Tetsui Ohkubo Committed by Commit Bot

message_center: Invert notification popup ordering.

In order to look visually consistent with UnifiedSystemTray,
notification popup ordering should be inverted on Chrome OS. To
implement that, this CL adds MOVE_UP_FOR_INVERSE animation state to
MessagePopupCollection.

TEST=MessagePopupCollectionTest
BUG=876769

Change-Id: I0dcb5c20c8bc3e76f44b32c989b7c935d0e2f676
Reviewed-on: https://chromium-review.googlesource.com/c/1290450
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: default avatarYoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602259}
parent 02aaf3b0
...@@ -74,6 +74,7 @@ UnifiedSystemTray::UiDelegate::UiDelegate(UnifiedSystemTray* owner) ...@@ -74,6 +74,7 @@ UnifiedSystemTray::UiDelegate::UiDelegate(UnifiedSystemTray* owner)
message_popup_collection_ = message_popup_collection_ =
std::make_unique<message_center::MessagePopupCollection>( std::make_unique<message_center::MessagePopupCollection>(
popup_alignment_delegate_.get()); popup_alignment_delegate_.get());
message_popup_collection_->set_inverse();
display::Screen* screen = display::Screen::GetScreen(); display::Screen* screen = display::Screen::GetScreen();
popup_alignment_delegate_->StartObserving( popup_alignment_delegate_->StartObserving(
screen, screen->GetDisplayNearestWindow( screen, screen->GetDisplayNearestWindow(
......
...@@ -67,7 +67,8 @@ void MessagePopupCollection::Update() { ...@@ -67,7 +67,8 @@ void MessagePopupCollection::Update() {
if (state_ != State::IDLE) { if (state_ != State::IDLE) {
// If not in IDLE state, start animation. // If not in IDLE state, start animation.
animation_->SetDuration(state_ == State::MOVE_DOWN animation_->SetDuration(state_ == State::MOVE_DOWN ||
state_ == State::MOVE_UP_FOR_INVERSE
? kMoveDownDuration ? kMoveDownDuration
: kFadeInFadeOutDuration); : kFadeInFadeOutDuration);
animation_->Start(); animation_->Start();
...@@ -216,16 +217,22 @@ void MessagePopupCollection::TransitionFromAnimation() { ...@@ -216,16 +217,22 @@ void MessagePopupCollection::TransitionFromAnimation() {
// If the animation is finished, transition to IDLE. // If the animation is finished, transition to IDLE.
state_ = State::IDLE; state_ = State::IDLE;
} else if (state_ == State::FADE_OUT && !popup_items_.empty()) { } else if (state_ == State::FADE_OUT && !popup_items_.empty()) {
// If FADE_OUT animation is finished and we still have remaining popups, if ((HasAddedPopup() && CollapseAllPopups()) || !inverse_) {
// we have to MOVE_DOWN them. // If FADE_OUT animation is finished and we still have remaining popups,
state_ = State::MOVE_DOWN; // we have to MOVE_DOWN them.
// If we're going to add a new popup after this MOVE_DOWN, do the collapse
// If we're going to add a new popup after this MOVE_DOWN, do the collapse // animation at the same time. Otherwise it will take another MOVE_DOWN.
// animation at the same time. Otherwise it will take another MOVE_DOWN. state_ = State::MOVE_DOWN;
if (HasAddedPopup()) MoveDownPopups();
CollapseAllPopups(); } else {
// If there's no collapsable popups and |inverse_| is on, there's nothing
MoveDownPopups(); // to do after FADE_OUT.
state_ = State::IDLE;
}
} else if (state_ == State::MOVE_UP_FOR_INVERSE) {
for (auto& item : popup_items_)
item.is_animating = item.will_fade_in;
state_ = State::FADE_IN;
} }
} }
...@@ -257,8 +264,16 @@ void MessagePopupCollection::TransitionToAnimation() { ...@@ -257,8 +264,16 @@ void MessagePopupCollection::TransitionToAnimation() {
MoveDownPopups(); MoveDownPopups();
return; return;
} else if (AddPopup()) { } else if (AddPopup()) {
// If a popup is actually added, show FADE_IN animation. // A popup is actually added.
state_ = State::FADE_IN; if (inverse_ && popup_items_.size() > 1) {
// If |inverse_| is on and there are existing notifications that have to
// be moved up (existing ones + new one, so > 1), transition to
// MOVE_UP_FOR_INVERSE.
state_ = State::MOVE_UP_FOR_INVERSE;
} else {
// Show FADE_IN animation.
state_ = State::FADE_IN;
}
return; return;
} }
} }
...@@ -300,7 +315,7 @@ void MessagePopupCollection::CalculateBounds() { ...@@ -300,7 +315,7 @@ void MessagePopupCollection::CalculateBounds() {
for (size_t i = 0; i < popup_items_.size(); ++i) { for (size_t i = 0; i < popup_items_.size(); ++i) {
gfx::Size preferred_size( gfx::Size preferred_size(
kNotificationWidth, kNotificationWidth,
popup_items_[i].popup->GetHeightForWidth(kNotificationWidth)); GetPopupItem(i)->popup->GetHeightForWidth(kNotificationWidth));
// Align the top of i-th popup to |hot_top_|. // Align the top of i-th popup to |hot_top_|.
if (is_hot_ && hot_index_ == i) { if (is_hot_ && hot_index_ == i) {
...@@ -316,8 +331,8 @@ void MessagePopupCollection::CalculateBounds() { ...@@ -316,8 +331,8 @@ void MessagePopupCollection::CalculateBounds() {
if (!alignment_delegate_->IsTopDown()) if (!alignment_delegate_->IsTopDown())
origin_y -= preferred_size.height(); origin_y -= preferred_size.height();
popup_items_[i].start_bounds = popup_items_[i].bounds; GetPopupItem(i)->start_bounds = GetPopupItem(i)->bounds;
popup_items_[i].bounds = GetPopupItem(i)->bounds =
gfx::Rect(gfx::Point(origin_x, origin_y), preferred_size); gfx::Rect(gfx::Point(origin_x, origin_y), preferred_size);
const int delta = preferred_size.height() + kMarginBetweenPopups; const int delta = preferred_size.height() + kMarginBetweenPopups;
...@@ -344,7 +359,8 @@ void MessagePopupCollection::UpdateByAnimation() { ...@@ -344,7 +359,8 @@ void MessagePopupCollection::UpdateByAnimation() {
else if (state_ == State::FADE_OUT) else if (state_ == State::FADE_OUT)
item.popup->SetOpacity(gfx::Tween::FloatValueBetween(value, 1.0f, 0.0f)); item.popup->SetOpacity(gfx::Tween::FloatValueBetween(value, 1.0f, 0.0f));
if (state_ == State::FADE_IN || state_ == State::MOVE_DOWN) { if (state_ == State::FADE_IN || state_ == State::MOVE_DOWN ||
state_ == State::MOVE_UP_FOR_INVERSE) {
item.popup->SetPopupBounds( item.popup->SetPopupBounds(
gfx::Tween::RectValueBetween(value, item.start_bounds, item.bounds)); gfx::Tween::RectValueBetween(value, item.start_bounds, item.bounds));
} }
...@@ -379,9 +395,11 @@ bool MessagePopupCollection::AddPopup() { ...@@ -379,9 +395,11 @@ bool MessagePopupCollection::AddPopup() {
if (!new_notification) if (!new_notification)
return false; return false;
// Reset animation flag of existing popups. // Reset animation flags of existing popups.
for (auto& item : popup_items_) for (auto& item : popup_items_) {
item.is_animating = false; item.is_animating = false;
item.will_fade_in = false;
}
{ {
PopupItem item; PopupItem item;
...@@ -398,6 +416,15 @@ bool MessagePopupCollection::AddPopup() { ...@@ -398,6 +416,15 @@ bool MessagePopupCollection::AddPopup() {
popup_items_.push_back(item); popup_items_.push_back(item);
} }
// There are existing notifications that have to be moved up (existing ones +
// new one, so > 1).
if (inverse_ && popup_items_.size() > 1) {
for (auto& item : popup_items_) {
item.will_fade_in = item.is_animating;
item.is_animating = !item.is_animating;
}
}
MessageCenter::Get()->DisplayedNotification(new_notification->id(), MessageCenter::Get()->DisplayedNotification(new_notification->id(),
DISPLAY_SOURCE_POPUP); DISPLAY_SOURCE_POPUP);
...@@ -453,10 +480,10 @@ bool MessagePopupCollection::IsNextEdgeOutsideWorkArea( ...@@ -453,10 +480,10 @@ bool MessagePopupCollection::IsNextEdgeOutsideWorkArea(
void MessagePopupCollection::StartHotMode() { void MessagePopupCollection::StartHotMode() {
for (size_t i = 0; i < popup_items_.size(); ++i) { for (size_t i = 0; i < popup_items_.size(); ++i) {
if (popup_items_[i].is_animating && popup_items_[i].popup->is_hovered()) { if (GetPopupItem(i)->is_animating && GetPopupItem(i)->popup->is_hovered()) {
is_hot_ = true; is_hot_ = true;
hot_index_ = i; hot_index_ = i;
hot_top_ = popup_items_[i].bounds.y(); hot_top_ = GetPopupItem(i)->bounds.y();
break; break;
} }
} }
...@@ -572,4 +599,11 @@ bool MessagePopupCollection::IsAnyPopupActive() const { ...@@ -572,4 +599,11 @@ bool MessagePopupCollection::IsAnyPopupActive() const {
return false; return false;
} }
MessagePopupCollection::PopupItem* MessagePopupCollection::GetPopupItem(
size_t index_from_top) {
DCHECK_LT(index_from_top, popup_items_.size());
return &popup_items_[inverse_ ? popup_items_.size() - index_from_top - 1
: index_from_top];
}
} // namespace message_center } // namespace message_center
...@@ -56,6 +56,8 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection ...@@ -56,6 +56,8 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection
void AnimationProgressed(const gfx::Animation* animation) override; void AnimationProgressed(const gfx::Animation* animation) override;
void AnimationCanceled(const gfx::Animation* animation) override; void AnimationCanceled(const gfx::Animation* animation) override;
void set_inverse() { inverse_ = true; }
protected: protected:
// TODO(tetsui): Merge PopupAlignmentDelegate into MessagePopupCollection and // TODO(tetsui): Merge PopupAlignmentDelegate into MessagePopupCollection and
// make subclasses e.g. DesktopMessagePopupCollection and // make subclasses e.g. DesktopMessagePopupCollection and
...@@ -87,7 +89,11 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection ...@@ -87,7 +89,11 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection
// Moving down notifications. Notification collapsing and resizing are also // Moving down notifications. Notification collapsing and resizing are also
// done in MOVE_DOWN. // done in MOVE_DOWN.
MOVE_DOWN MOVE_DOWN,
// Moving up notifications in order to show new one by FADE_IN. This is only
// used when |inverse_| is true.
MOVE_UP_FOR_INVERSE
}; };
// Stores animation related state of a popup. // Stores animation related state of a popup.
...@@ -103,6 +109,11 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection ...@@ -103,6 +109,11 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection
// The final bounds of the popup. // The final bounds of the popup.
gfx::Rect bounds; gfx::Rect bounds;
// The popup is waiting for MOVE_UP_FOR_INVERSE animation so that it can
// FADE_IN after that. The value is only used when the animation type is
// MOVE_UP_FOR_INVERSE.
bool will_fade_in = false;
// If the popup is animating. // If the popup is animating.
bool is_animating = false; bool is_animating = false;
...@@ -173,6 +184,10 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection ...@@ -173,6 +184,10 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection
// Return true if any popup is activated. // Return true if any popup is activated.
bool IsAnyPopupActive() const; bool IsAnyPopupActive() const;
// Returns the popup which is visually |index_from_top|-th from the top.
// When |inverse_| is false, it's same as popup_items_[i].
PopupItem* GetPopupItem(size_t index_from_top);
// Animation state. See the comment of State. // Animation state. See the comment of State.
State state_ = State::IDLE; State state_ = State::IDLE;
...@@ -211,6 +226,16 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection ...@@ -211,6 +226,16 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection
// |hot_index_| is aligned to |hot_top_|. Only valid if |is_hot_| is true. // |hot_index_| is aligned to |hot_top_|. Only valid if |is_hot_| is true.
int hot_top_ = 0; int hot_top_ = 0;
// Invert ordering of notification popups i.e. showing the latest notification
// at the top. It changes the state transition like this:
// Normal:
// * a new notification comes in: FADE_IN
// * a notification comes out: FADE_OUT -> MOVE_DOWN
// Inverted:
// * a new notification comes in: MOVE_UP_FOR_INVERSE -> FADE_IN
// * a notification comes out: FADE_OUT
bool inverse_ = false;
DISALLOW_COPY_AND_ASSIGN(MessagePopupCollection); DISALLOW_COPY_AND_ASSIGN(MessagePopupCollection);
}; };
......
...@@ -261,9 +261,15 @@ class MessagePopupCollectionTest : public views::ViewsTestBase, ...@@ -261,9 +261,15 @@ class MessagePopupCollectionTest : public views::ViewsTestBase,
popup_collection_->SetAnimationValue(1.0); popup_collection_->SetAnimationValue(1.0);
} }
void AnimateToMiddle() { popup_collection_->SetAnimationValue(0.5); } void AnimateToMiddle() {
EXPECT_TRUE(popup_collection_->IsAnimating());
popup_collection_->SetAnimationValue(0.5);
}
void AnimateToEnd() { popup_collection_->SetAnimationValue(1.0); } void AnimateToEnd() {
EXPECT_TRUE(popup_collection_->IsAnimating());
popup_collection_->SetAnimationValue(1.0);
}
MockMessagePopupView* GetPopup(const std::string& id) { MockMessagePopupView* GetPopup(const std::string& id) {
for (auto* popup : popup_collection_->popups()) { for (auto* popup : popup_collection_->popups()) {
...@@ -377,6 +383,43 @@ TEST_F(MessagePopupCollectionTest, FadeInMultipleNotifications) { ...@@ -377,6 +383,43 @@ TEST_F(MessagePopupCollectionTest, FadeInMultipleNotifications) {
} }
} }
TEST_F(MessagePopupCollectionTest, FadeInMultipleNotificationsInverse) {
popup_collection()->set_inverse();
std::vector<std::string> ids;
for (size_t i = 0; i < kMaxVisiblePopupNotifications; ++i)
ids.push_back(AddNotification());
for (size_t i = 0; i < ids.size(); ++i) {
EXPECT_EQ(ids[i], last_displayed_id());
EXPECT_EQ(i + 1, GetPopupCounts());
const int before_x = GetPopupAt(i)->GetBoundsInScreen().x();
AnimateToMiddle();
EXPECT_LT(0.0f, GetPopupAt(i)->GetOpacity());
EXPECT_GT(before_x, GetPopupAt(i)->GetBoundsInScreen().x());
AnimateToEnd();
EXPECT_EQ(1.0f, GetPopupAt(i)->GetOpacity());
EXPECT_TRUE(work_area().Contains(GetPopupAt(i)->GetBoundsInScreen()));
if (i + 1 < ids.size()) {
const int before_y = GetPopupAt(i)->GetBoundsInScreen().y();
AnimateToMiddle();
EXPECT_GT(before_y, GetPopupAt(i)->GetBoundsInScreen().y());
AnimateToEnd();
}
}
EXPECT_FALSE(IsAnimating());
EXPECT_EQ(kMaxVisiblePopupNotifications, GetPopupCounts());
for (size_t i = 0; i < ids.size(); ++i)
EXPECT_EQ(ids[i], GetPopupAt(i)->id());
for (size_t i = 0; i < ids.size() - 1; ++i) {
EXPECT_GT(GetPopupAt(i + 1)->GetBoundsInScreen().x(),
GetPopupAt(i)->GetBoundsInScreen().bottom());
}
}
TEST_F(MessagePopupCollectionTest, UpdateContents) { TEST_F(MessagePopupCollectionTest, UpdateContents) {
std::string id = AddNotification(); std::string id = AddNotification();
AnimateToEnd(); AnimateToEnd();
...@@ -502,6 +545,57 @@ TEST_F(MessagePopupCollectionTest, NotificationsMoveDown) { ...@@ -502,6 +545,57 @@ TEST_F(MessagePopupCollectionTest, NotificationsMoveDown) {
EXPECT_FALSE(IsAnimating()); EXPECT_FALSE(IsAnimating());
} }
TEST_F(MessagePopupCollectionTest, NotificationsMoveUpForInverse) {
popup_collection()->set_inverse();
std::vector<std::string> ids;
for (size_t i = 0; i < kMaxVisiblePopupNotifications + 1; ++i)
ids.push_back(AddNotification());
AnimateUntilIdle();
EXPECT_EQ(kMaxVisiblePopupNotifications, GetPopupCounts());
EXPECT_FALSE(IsAnimating());
gfx::Rect dismissed = GetPopup(ids.front())->GetBoundsInScreen();
MessageCenter::Get()->MarkSinglePopupAsShown(ids.front(), false);
EXPECT_TRUE(IsAnimating());
// FADE_OUT
AnimateToMiddle();
EXPECT_GT(1.0f, GetPopup(ids[0])->GetOpacity());
EXPECT_EQ(ids[0], GetPopup(ids[0])->id());
AnimateToEnd();
EXPECT_EQ(ids[1], GetPopup(ids[1])->id());
EXPECT_TRUE(IsAnimating());
gfx::Rect before = GetPopup(ids[1])->GetBoundsInScreen();
// MOVE_UP_FOR_INVERSE
AnimateToMiddle();
gfx::Rect moving = GetPopup(ids[1])->GetBoundsInScreen();
EXPECT_LT(moving.bottom(), before.bottom());
EXPECT_LT(dismissed.bottom(), moving.bottom());
AnimateToEnd();
gfx::Rect after = GetPopup(ids[1])->GetBoundsInScreen();
EXPECT_EQ(dismissed, after);
EXPECT_EQ(kMaxVisiblePopupNotifications, GetPopupCounts());
EXPECT_TRUE(IsAnimating());
EXPECT_EQ(0.f, GetPopup(ids.back())->GetOpacity());
// FADE_IN
AnimateToMiddle();
EXPECT_LT(0.0f, GetPopup(ids.back())->GetOpacity());
AnimateToEnd();
EXPECT_EQ(1.0f, GetPopup(ids.back())->GetOpacity());
EXPECT_FALSE(IsAnimating());
}
TEST_F(MessagePopupCollectionTest, PopupResized) { TEST_F(MessagePopupCollectionTest, PopupResized) {
std::vector<std::string> ids; std::vector<std::string> ids;
for (size_t i = 0; i < kMaxVisiblePopupNotifications; ++i) for (size_t i = 0; i < kMaxVisiblePopupNotifications; ++i)
...@@ -829,6 +923,38 @@ TEST_F(MessagePopupCollectionTest, DefaultPositioning) { ...@@ -829,6 +923,38 @@ TEST_F(MessagePopupCollectionTest, DefaultPositioning) {
EXPECT_EQ(r1.x(), r2.x()); EXPECT_EQ(r1.x(), r2.x());
} }
TEST_F(MessagePopupCollectionTest, DefaultPositioningInverse) {
popup_collection()->set_inverse();
std::string id0 = AddNotification();
std::string id1 = AddNotification();
std::string id2 = AddNotification();
std::string id3 = AddNotification();
AnimateUntilIdle();
// This part is inverted.
gfx::Rect r0 = GetPopup(id2)->GetBoundsInScreen();
gfx::Rect r1 = GetPopup(id1)->GetBoundsInScreen();
gfx::Rect r2 = GetPopup(id0)->GetBoundsInScreen();
// The 4th toast is not shown yet.
EXPECT_FALSE(GetPopup(id3));
// 3 toasts are shown, equal size, vertical stack.
EXPECT_EQ(r0.width(), r1.width());
EXPECT_EQ(r1.width(), r2.width());
EXPECT_EQ(r0.height(), r1.height());
EXPECT_EQ(r1.height(), r2.height());
EXPECT_GT(r0.y(), r1.y());
EXPECT_GT(r1.y(), r2.y());
EXPECT_EQ(r0.x(), r1.x());
EXPECT_EQ(r1.x(), r2.x());
}
TEST_F(MessagePopupCollectionTest, DefaultPositioningWithRightTaskbar) { TEST_F(MessagePopupCollectionTest, DefaultPositioningWithRightTaskbar) {
// If taskbar is on the right we show the toasts bottom to top as usual. // If taskbar is on the right we show the toasts bottom to top as usual.
......
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