Skip to content

Commit

Permalink
Update hotseat widget to handle gesture events with its bounds.
Browse files Browse the repository at this point in the history
This CL also adds RTL for shelf view and scrollable shelf view unittests.

Note that there are 7 tests in shelf_view_unittest and 6 tests in
scrollable_shelf_view_unittest that have not been converted because
their behavior is broken for RTL. Those fixes will be landed in follow
up CLs.

Bug: 1188738
Change-Id: I5524b8596dd0665c1e465627af01292925904d90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2800409
Commit-Queue: Yulun Wu <yulunwu@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#869298}
  • Loading branch information
Yulun Wu authored and Chromium LUCI CQ committed Apr 5, 2021
1 parent 83940a2 commit abf1709
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 67 deletions.
2 changes: 1 addition & 1 deletion ash/shelf/hotseat_widget.cc
Expand Up @@ -717,7 +717,7 @@ void HotseatWidget::OnGestureEvent(ui::GestureEvent* event) {
// context menu starts capturing events. Ignore events not interesting to the
// shelf app button in this state.
ShelfAppButton* item_with_context_menu =
scrollable_shelf_view_->shelf_view()->GetShelfItemViewWithContextMenu();
GetShelfView()->GetShelfItemViewWithContextMenu();
if (item_with_context_menu &&
!ShelfAppButton::ShouldHandleEventFromContextMenu(event)) {
event->SetHandled();
Expand Down
4 changes: 3 additions & 1 deletion ash/shelf/scrollable_shelf_view.cc
Expand Up @@ -332,7 +332,9 @@ bool ScrollableShelfContainerView::DoesIntersectRect(
// This view's layer is clipped. So the view should only handle the events
// within the area after cilp.

gfx::RectF bounds = gfx::RectF(scrollable_shelf_view_->visible_space());
// Note that |rect| is not mirrored under RTL while |visible_space_| has been
// mirrored.
gfx::RectF bounds(GetMirroredRect(scrollable_shelf_view_->visible_space()));
views::View::ConvertRectToTarget(scrollable_shelf_view_, this, &bounds);
return ToEnclosedRect(bounds).Contains(rect);
}
Expand Down
63 changes: 32 additions & 31 deletions ash/shelf/scrollable_shelf_view_unittest.cc
Expand Up @@ -226,6 +226,19 @@ class ScrollableShelfViewTest : public AshTestBase {
int id_ = 0;
};

// Tests scrollable shelf's features under both LTR and RTL.
class ScrollableShelfViewRTLTest : public ScrollableShelfViewTest,
public testing::WithParamInterface<bool> {
public:
ScrollableShelfViewRTLTest() : scoped_locale_(GetParam() ? "ar" : "") {}
~ScrollableShelfViewRTLTest() override = default;

private:
base::test::ScopedRestoreICUDefaultLocale scoped_locale_;
};

INSTANTIATE_TEST_SUITE_P(RTL, ScrollableShelfViewRTLTest, testing::Bool());

// Verifies that the display rotation from the short side to the long side
// should not break the scrollable shelf's UI
// behavior(https://crbug.com/1000764).
Expand Down Expand Up @@ -286,7 +299,7 @@ TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotationShortToLong) {
// Verifies that the display rotation from the long side to the short side
// should not break the scrollable shelf's UI behavior
// (https://crbug.com/1000764).
TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotationLongToShort) {
TEST_P(ScrollableShelfViewRTLTest, CorrectUIAfterDisplayRotationLongToShort) {
// Changes the display setting in order that the display's width is greater
// than the height.
UpdateDisplay("600x300");
Expand Down Expand Up @@ -320,7 +333,7 @@ TEST_F(ScrollableShelfViewTest, CorrectUIAfterDisplayRotationLongToShort) {

// Verifies that the mask layer gradient shader is not applied when no arrow
// button shows.
TEST_F(ScrollableShelfViewTest, VerifyApplyMaskGradientShaderWhenNeeded) {
TEST_P(ScrollableShelfViewRTLTest, VerifyApplyMaskGradientShaderWhenNeeded) {
AddAppShortcut();
ASSERT_EQ(ScrollableShelfView::LayoutStrategy::kNotShowArrowButtons,
scrollable_shelf_view_->layout_strategy_for_test());
Expand All @@ -334,7 +347,7 @@ TEST_F(ScrollableShelfViewTest, VerifyApplyMaskGradientShaderWhenNeeded) {

// When hovering mouse on a shelf icon, the tooltip only shows for the visible
// icon (see https://crbug.com/997807).
TEST_F(ScrollableShelfViewTest, NotShowTooltipForHiddenIcons) {
TEST_P(ScrollableShelfViewRTLTest, NotShowTooltipForHiddenIcons) {
AddAppShortcutsUntilOverflow();

ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
Expand Down Expand Up @@ -371,7 +384,7 @@ TEST_F(ScrollableShelfViewTest, NotShowTooltipForHiddenIcons) {

// Test that tapping near the scroll arrow button triggers scrolling. (see
// https://crbug.com/1004998)
TEST_F(ScrollableShelfViewTest, ScrollAfterTappingNearScrollArrow) {
TEST_P(ScrollableShelfViewRTLTest, ScrollAfterTappingNearScrollArrow) {
AddAppShortcutsUntilOverflow();

ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
Expand Down Expand Up @@ -414,7 +427,7 @@ TEST_F(ScrollableShelfViewTest, ScrollAfterTappingNearScrollArrow) {
// Verifies that in overflow mode, the app icons indexed by
// |first_tappable_app_index_| and |last_tappable_app_index_| are completely
// shown (https://crbug.com/1013811).
TEST_F(ScrollableShelfViewTest, VerifyTappableAppIndices) {
TEST_P(ScrollableShelfViewRTLTest, VerifyTappableAppIndices) {
AddAppShortcutsUntilOverflow();

// Checks bounds when the layout strategy is kShowRightArrowButton.
Expand Down Expand Up @@ -444,7 +457,7 @@ TEST_F(ScrollableShelfViewTest, VerifyTappableAppIndices) {
CheckFirstAndLastTappableIconsBounds();
}

TEST_F(ScrollableShelfViewTest, ShowTooltipForArrowButtons) {
TEST_P(ScrollableShelfViewRTLTest, ShowTooltipForArrowButtons) {
AddAppShortcutsUntilOverflow();
ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
scrollable_shelf_view_->layout_strategy_for_test());
Expand Down Expand Up @@ -480,7 +493,7 @@ TEST_F(ScrollableShelfViewTest, ShowTooltipForArrowButtons) {
// Verifies that dragging an app icon to a new shelf page works well. In
// addition, the dragged icon moves with mouse before mouse release (see
// https://crbug.com/1031367).
TEST_F(ScrollableShelfViewTest, DragIconToNewPage) {
TEST_P(ScrollableShelfViewRTLTest, DragIconToNewPage) {
scrollable_shelf_view_->set_page_flip_time_threshold(
base::TimeDelta::FromMilliseconds(10));

Expand Down Expand Up @@ -540,7 +553,7 @@ TEST_F(ScrollableShelfViewTest, DragIconToNewPage) {
// Verifies that after adding the second display, shelf icons showing on
// the primary display are also visible on the second display
// (https://crbug.com/1035596).
TEST_F(ScrollableShelfViewTest, CheckTappableIndicesOnSecondDisplay) {
TEST_P(ScrollableShelfViewRTLTest, CheckTappableIndicesOnSecondDisplay) {
constexpr int icon_number = 5;
for (int i = 0; i < icon_number; i++)
AddAppShortcut();
Expand Down Expand Up @@ -592,7 +605,7 @@ TEST_F(ScrollableShelfViewTest, CorrectUIAfterSwitchingToTablet) {

// Verifies that activating a shelf icon's ripple ring does not bring crash
// on an extremely small display. It is an edge case detected by fuzz tests.
TEST_F(ScrollableShelfViewTest, VerifyActivateIconRippleOnVerySmallDisplay) {
TEST_P(ScrollableShelfViewRTLTest, VerifyActivateIconRippleOnVerySmallDisplay) {
AddAppShortcut();

// Resize the display to ensure that no shelf icon is visible.
Expand Down Expand Up @@ -633,7 +646,7 @@ TEST_F(ScrollableShelfViewTest, CorrectUIInTabletWithoutOverflow) {

// Verifies that the scrollable shelf without overflow has the correct layout in
// tablet mode.
TEST_F(ScrollableShelfViewTest, CheckRoundedCornersSetForInkDrop) {
TEST_P(ScrollableShelfViewRTLTest, CheckRoundedCornersSetForInkDrop) {
Shell::Get()->tablet_mode_controller()->SetEnabledForTest(true);
AddAppShortcutsUntilOverflow();
ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
Expand Down Expand Up @@ -682,7 +695,8 @@ TEST_F(ScrollableShelfViewTest, CheckRoundedCornersSetForInkDrop) {

// Verify that the rounded corners work as expected after transition from
// clamshell mode to tablet mode (https://crbug.com/1086484).
TEST_F(ScrollableShelfViewTest, CheckRoundedCornersAfterTabletStateTransition) {
TEST_P(ScrollableShelfViewRTLTest,
CheckRoundedCornersAfterTabletStateTransition) {
ui::ScopedAnimationDurationScaleMode regular_animations(
ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
PopulateAppShortcut(1);
Expand Down Expand Up @@ -821,7 +835,7 @@ TEST_F(ScrollableShelfViewTest, CheckRoundedCornersAfterLongPress) {

// Verifies that doing a mousewheel scroll on the scrollable shelf does scroll
// forward.
TEST_F(ScrollableShelfViewTest, ScrollWithMouseWheel) {
TEST_P(ScrollableShelfViewRTLTest, ScrollWithMouseWheel) {
// The scroll threshold. Taken from |KScrollOffsetThreshold| in
// scrollable_shelf_view.cc.
constexpr int scroll_threshold = 20;
Expand Down Expand Up @@ -858,7 +872,7 @@ TEST_F(ScrollableShelfViewTest, ScrollWithMouseWheel) {

// Verifies that removing a shelf icon by mouse works as expected on scrollable
// shelf (see https://crbug.com/1033967).
TEST_F(ScrollableShelfViewTest, RipOffShelfItem) {
TEST_P(ScrollableShelfViewRTLTest, RipOffShelfItem) {
AddAppShortcutsUntilOverflow();
ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
scrollable_shelf_view_->layout_strategy_for_test());
Expand Down Expand Up @@ -886,7 +900,7 @@ TEST_F(ScrollableShelfViewTest, RipOffShelfItem) {
}

// Verifies that the scrollable shelf handles the mouse wheel event as expected.
TEST_F(ScrollableShelfViewTest, MouseWheelOnEmptyShelfShouldExpandAppList) {
TEST_P(ScrollableShelfViewRTLTest, MouseWheelOnEmptyShelfShouldExpandAppList) {
// First mouse wheel over apps and then over empty shelf. When apps are not
// overflowing, and we mouse wheel over the app icons, the launcher should
// stay hidden. When we mouse wheel over the empty area of the shelf, the
Expand Down Expand Up @@ -935,7 +949,7 @@ TEST_F(ScrollableShelfViewTest, MouseWheelOnEmptyShelfShouldExpandAppList) {
->app_list_state());
}

TEST_F(ScrollableShelfViewTest, ScrollsByMouseWheelEvent) {
TEST_P(ScrollableShelfViewRTLTest, ScrollsByMouseWheelEvent) {
AddAppShortcutsUntilOverflow();
ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
scrollable_shelf_view_->layout_strategy_for_test());
Expand Down Expand Up @@ -963,7 +977,7 @@ TEST_F(ScrollableShelfViewTest, ScrollsByMouseWheelEvent) {

// Verifies that the scrollable shelf handles the scroll event (usually
// generated by the touchpad scroll) as expected.
TEST_F(ScrollableShelfViewTest, VerifyScrollEvent) {
TEST_P(ScrollableShelfViewRTLTest, VerifyScrollEvent) {
AddAppShortcutsUntilOverflow();

// Checks the default state of the scrollable shelf and the launcher.
Expand Down Expand Up @@ -1004,7 +1018,7 @@ TEST_F(ScrollableShelfViewTest, VerifyScrollEvent) {

// Verify that the ripple ring of the first/last app icon is fully shown
// (https://crbug.com/1057710).
TEST_F(ScrollableShelfViewTest, CheckInkDropRippleOfEdgeIcons) {
TEST_P(ScrollableShelfViewRTLTest, CheckInkDropRippleOfEdgeIcons) {
AddAppShortcutsUntilOverflow();
ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
scrollable_shelf_view_->layout_strategy_for_test());
Expand All @@ -1028,7 +1042,7 @@ TEST_F(ScrollableShelfViewTest, CheckInkDropRippleOfEdgeIcons) {

// Verifies that right-click on the last shelf icon should open the icon's
// context menu instead of the shelf's (https://crbug.com/1041702).
TEST_F(ScrollableShelfViewTest, ClickAtLastIcon) {
TEST_P(ScrollableShelfViewRTLTest, ClickAtLastIcon) {
AddAppShortcutsUntilOverflow();
ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
scrollable_shelf_view_->layout_strategy_for_test());
Expand Down Expand Up @@ -1139,19 +1153,6 @@ TEST_F(ScrollableShelfViewTest, PresentationTimeMetricsForGestureScroll) {
check_bucket_size(1, 1);
}

// Tests scrollable shelf's features under both LTR and RTL.
class ScrollableShelfViewRTLTest : public ScrollableShelfViewTest,
public testing::WithParamInterface<bool> {
public:
ScrollableShelfViewRTLTest() : scoped_locale_(GetParam() ? "ar" : "") {}
~ScrollableShelfViewRTLTest() override = default;

private:
base::test::ScopedRestoreICUDefaultLocale scoped_locale_;
};

INSTANTIATE_TEST_SUITE_P(LtrRTL, ScrollableShelfViewRTLTest, testing::Bool());

// Verifies that the shelf is scrolled to show the pinned app after pinning.
TEST_P(ScrollableShelfViewRTLTest, FeedbackForAppPinning) {
AddAppShortcutsUntilOverflow();
Expand Down

0 comments on commit abf1709

Please sign in to comment.