Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1922)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: ui/views/controls/scroll_view_unittest.cc

Issue 2555213004: Improving the appearance of overlay scrollbars (Closed)
Patch Set: attempt to fix mac Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« ui/views/controls/scroll_view.cc ('K') | « ui/views/controls/scroll_view.cc ('k') | ui/views/controls/scrollbar/base_scroll_bar.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ui/views/controls/scroll_view_unittest.cc
diff --git a/ui/views/controls/scroll_view_unittest.cc b/ui/views/controls/scroll_view_unittest.cc
index c4a5ca7a113e2e57dbf6818529af58591afd501c..1aa29e15974dce512c18bd8211dcda8c2211250e 100644
--- a/ui/views/controls/scroll_view_unittest.cc
+++ b/ui/views/controls/scroll_view_unittest.cc
@@ -167,6 +167,14 @@ class ScrollViewTest : public ViewsTestBase {
protected:
#endif
+ int VerticalScrollBarWidth() {
+ return scroll_view_.vertical_scroll_bar()->GetThickness();
+ }
+
+ int HorizontalScrollBarHeight() {
+ return scroll_view_.horizontal_scroll_bar()->GetThickness();
+ }
+
ScrollView scroll_view_;
private:
@@ -309,7 +317,7 @@ TEST_F(ScrollViewTest, ScrollBars) {
// Size the contents such that vertical scrollbar is needed.
contents->SetBounds(0, 0, 50, 400);
scroll_view_.Layout();
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth(),
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth(),
contents->parent()->width());
EXPECT_EQ(100, contents->parent()->height());
CheckScrollbarVisibility(scroll_view_, VERTICAL, true);
@@ -323,7 +331,7 @@ TEST_F(ScrollViewTest, ScrollBars) {
contents->SetBounds(0, 0, 400, 50);
scroll_view_.Layout();
EXPECT_EQ(100, contents->parent()->width());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarHeight(),
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutHeight(),
contents->parent()->height());
CheckScrollbarVisibility(scroll_view_, VERTICAL, false);
CheckScrollbarVisibility(scroll_view_, HORIZONTAL, true);
@@ -331,9 +339,9 @@ TEST_F(ScrollViewTest, ScrollBars) {
// Both horizontal and vertical.
contents->SetBounds(0, 0, 300, 400);
scroll_view_.Layout();
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth(),
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth(),
contents->parent()->width());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarHeight(),
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutHeight(),
contents->parent()->height());
CheckScrollbarVisibility(scroll_view_, VERTICAL, true);
CheckScrollbarVisibility(scroll_view_, HORIZONTAL, true);
@@ -347,16 +355,16 @@ TEST_F(ScrollViewTest, ScrollBars) {
kBottomPadding, kRightPadding));
contents->SetBounds(0, 0, 50, 400);
scroll_view_.Layout();
- EXPECT_EQ(
- 100 - scroll_view_.GetScrollBarWidth() - kLeftPadding - kRightPadding,
- contents->parent()->width());
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth() - kLeftPadding -
+ kRightPadding,
+ contents->parent()->width());
EXPECT_EQ(100 - kTopPadding - kBottomPadding, contents->parent()->height());
EXPECT_TRUE(!scroll_view_.horizontal_scroll_bar() ||
!scroll_view_.horizontal_scroll_bar()->visible());
ASSERT_TRUE(scroll_view_.vertical_scroll_bar() != NULL);
EXPECT_TRUE(scroll_view_.vertical_scroll_bar()->visible());
gfx::Rect bounds = scroll_view_.vertical_scroll_bar()->bounds();
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth() - kRightPadding, bounds.x());
+ EXPECT_EQ(100 - VerticalScrollBarWidth() - kRightPadding, bounds.x());
EXPECT_EQ(100 - kRightPadding, bounds.right());
EXPECT_EQ(kTopPadding, bounds.y());
EXPECT_EQ(100 - kBottomPadding, bounds.bottom());
@@ -365,9 +373,9 @@ TEST_F(ScrollViewTest, ScrollBars) {
contents->SetBounds(0, 0, 400, 50);
scroll_view_.Layout();
EXPECT_EQ(100 - kLeftPadding - kRightPadding, contents->parent()->width());
- EXPECT_EQ(
- 100 - scroll_view_.GetScrollBarHeight() - kTopPadding - kBottomPadding,
- contents->parent()->height());
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutHeight() - kTopPadding -
+ kBottomPadding,
+ contents->parent()->height());
ASSERT_TRUE(scroll_view_.horizontal_scroll_bar() != NULL);
EXPECT_TRUE(scroll_view_.horizontal_scroll_bar()->visible());
EXPECT_TRUE(!scroll_view_.vertical_scroll_bar() ||
@@ -375,38 +383,35 @@ TEST_F(ScrollViewTest, ScrollBars) {
bounds = scroll_view_.horizontal_scroll_bar()->bounds();
EXPECT_EQ(kLeftPadding, bounds.x());
EXPECT_EQ(100 - kRightPadding, bounds.right());
- EXPECT_EQ(100 - kBottomPadding - scroll_view_.GetScrollBarHeight(),
- bounds.y());
+ EXPECT_EQ(100 - kBottomPadding - HorizontalScrollBarHeight(), bounds.y());
EXPECT_EQ(100 - kBottomPadding, bounds.bottom());
// Both horizontal and vertical with border.
contents->SetBounds(0, 0, 300, 400);
scroll_view_.Layout();
- EXPECT_EQ(
- 100 - scroll_view_.GetScrollBarWidth() - kLeftPadding - kRightPadding,
- contents->parent()->width());
- EXPECT_EQ(
- 100 - scroll_view_.GetScrollBarHeight() - kTopPadding - kBottomPadding,
- contents->parent()->height());
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth() - kLeftPadding -
+ kRightPadding,
+ contents->parent()->width());
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutHeight() - kTopPadding -
+ kBottomPadding,
+ contents->parent()->height());
bounds = scroll_view_.horizontal_scroll_bar()->bounds();
// Check horiz.
ASSERT_TRUE(scroll_view_.horizontal_scroll_bar() != NULL);
EXPECT_TRUE(scroll_view_.horizontal_scroll_bar()->visible());
bounds = scroll_view_.horizontal_scroll_bar()->bounds();
EXPECT_EQ(kLeftPadding, bounds.x());
- EXPECT_EQ(100 - kRightPadding - scroll_view_.GetScrollBarWidth(),
- bounds.right());
- EXPECT_EQ(100 - kBottomPadding - scroll_view_.GetScrollBarHeight(),
- bounds.y());
+ EXPECT_EQ(100 - kRightPadding - VerticalScrollBarWidth(), bounds.right());
+ EXPECT_EQ(100 - kBottomPadding - HorizontalScrollBarHeight(), bounds.y());
EXPECT_EQ(100 - kBottomPadding, bounds.bottom());
// Check vert.
ASSERT_TRUE(scroll_view_.vertical_scroll_bar() != NULL);
EXPECT_TRUE(scroll_view_.vertical_scroll_bar()->visible());
bounds = scroll_view_.vertical_scroll_bar()->bounds();
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth() - kRightPadding, bounds.x());
+ EXPECT_EQ(100 - VerticalScrollBarWidth() - kRightPadding, bounds.x());
EXPECT_EQ(100 - kRightPadding, bounds.right());
EXPECT_EQ(kTopPadding, bounds.y());
- EXPECT_EQ(100 - kBottomPadding - scroll_view_.GetScrollBarHeight(),
+ EXPECT_EQ(100 - kBottomPadding - HorizontalScrollBarHeight(),
bounds.bottom());
}
@@ -464,19 +469,25 @@ TEST_F(ScrollViewTest, ScrollBarsWithHeader) {
scroll_view_.Layout();
EXPECT_EQ(0, contents->parent()->x());
EXPECT_EQ(20, contents->parent()->y());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth(),
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth(),
contents->parent()->width());
EXPECT_EQ(80, contents->parent()->height());
EXPECT_EQ(0, header->parent()->x());
EXPECT_EQ(0, header->parent()->y());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth(), header->parent()->width());
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth(),
+ header->parent()->width());
EXPECT_EQ(20, header->parent()->height());
EXPECT_TRUE(!scroll_view_.horizontal_scroll_bar() ||
!scroll_view_.horizontal_scroll_bar()->visible());
ASSERT_TRUE(scroll_view_.vertical_scroll_bar() != NULL);
EXPECT_TRUE(scroll_view_.vertical_scroll_bar()->visible());
- // Make sure the vertical scrollbar overlaps the header.
- EXPECT_EQ(header->y(), scroll_view_.vertical_scroll_bar()->y());
+ // Make sure the vertical scrollbar overlaps the header for traditional
+ // scrollbars and doesn't overlap the header for overlay scrollbars.
+ const int expected_scrollbar_y =
+ scroll_view_.vertical_scroll_bar()->OverlapsContent()
+ ? header->bounds().bottom()
+ : header->y();
+ EXPECT_EQ(expected_scrollbar_y, scroll_view_.vertical_scroll_bar()->y());
EXPECT_EQ(header->y(), contents->y());
// Size the contents such that horizontal scrollbar is needed.
@@ -485,7 +496,7 @@ TEST_F(ScrollViewTest, ScrollBarsWithHeader) {
EXPECT_EQ(0, contents->parent()->x());
EXPECT_EQ(20, contents->parent()->y());
EXPECT_EQ(100, contents->parent()->width());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarHeight() - 20,
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutHeight() - 20,
contents->parent()->height());
EXPECT_EQ(0, header->parent()->x());
EXPECT_EQ(0, header->parent()->y());
@@ -501,13 +512,14 @@ TEST_F(ScrollViewTest, ScrollBarsWithHeader) {
scroll_view_.Layout();
EXPECT_EQ(0, contents->parent()->x());
EXPECT_EQ(20, contents->parent()->y());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth(),
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth(),
contents->parent()->width());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarHeight() - 20,
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutHeight() - 20,
contents->parent()->height());
EXPECT_EQ(0, header->parent()->x());
EXPECT_EQ(0, header->parent()->y());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth(), header->parent()->width());
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutWidth(),
+ header->parent()->width());
EXPECT_EQ(20, header->parent()->height());
ASSERT_TRUE(scroll_view_.horizontal_scroll_bar() != NULL);
EXPECT_TRUE(scroll_view_.horizontal_scroll_bar()->visible());
@@ -560,7 +572,7 @@ TEST_F(ScrollViewTest, ScrollRectToVisible) {
const int viewport_height = test_api.contents_viewport()->height();
// Expect there to be a horizontal scrollbar, making the viewport shorter.
- EXPECT_LT(viewport_height, 100);
+ EXPECT_EQ(100 - scroll_view_.GetScrollBarLayoutHeight(), viewport_height);
gfx::ScrollOffset offset = test_api.CurrentOffset();
EXPECT_EQ(415 - viewport_height, offset.y());
@@ -618,10 +630,6 @@ TEST_F(ScrollViewTest, ClipHeightToShortContentHeight) {
// Verifies ClipHeightTo() uses the maximum height when the content is longer
// thamn the maximum height value.
TEST_F(ScrollViewTest, ClipHeightToTallContentHeight) {
- // Use a scrollbar that is disabled by default, so the width of the content is
- // not affected.
- scroll_view_.SetVerticalScrollBar(new views::OverlayScrollBar(false));
-
scroll_view_.ClipHeightTo(kMinHeight, kMaxHeight);
const int kTallContentHeight = 1000;
@@ -633,8 +641,9 @@ TEST_F(ScrollViewTest, ClipHeightToTallContentHeight) {
scroll_view_.SizeToPreferredSize();
scroll_view_.Layout();
- EXPECT_EQ(gfx::Size(kWidth, kTallContentHeight),
- scroll_view_.contents()->size());
+ // The width may be less than kWidth if the scroll bar takes up some width.
+ EXPECT_GE(kWidth, scroll_view_.contents()->width());
+ EXPECT_EQ(kTallContentHeight, scroll_view_.contents()->height());
EXPECT_EQ(gfx::Size(kWidth, kMaxHeight), scroll_view_.size());
}
@@ -654,8 +663,7 @@ TEST_F(ScrollViewTest, ClipHeightToScrollbarUsesWidth) {
scroll_view_.SetSize(new_size);
scroll_view_.Layout();
- int scroll_bar_width = scroll_view_.GetScrollBarWidth();
- int expected_width = kWidth - scroll_bar_width;
+ int expected_width = kWidth - scroll_view_.GetScrollBarLayoutWidth();
EXPECT_EQ(scroll_view_.contents()->size().width(), expected_width);
EXPECT_EQ(scroll_view_.contents()->size().height(), 1000 * expected_width);
EXPECT_EQ(gfx::Size(kWidth, kMaxHeight), scroll_view_.size());
@@ -665,17 +673,24 @@ TEST_F(ScrollViewTest, CornerViewVisibility) {
View* contents = InstallContents();
View* corner_view = ScrollViewTestApi(&scroll_view_).corner_view();
- // Corner view should be visible when both scrollbars are visible.
contents->SetBounds(0, 0, 200, 200);
scroll_view_.Layout();
+
+ // Corner view should not exist if using overlay scrollbars.
+ if (scroll_view_.vertical_scroll_bar()->OverlapsContent()) {
+ EXPECT_FALSE(corner_view->parent());
+ return;
+ }
+
+ // Corner view should be visible when both scrollbars are visible.
EXPECT_EQ(&scroll_view_, corner_view->parent());
EXPECT_TRUE(corner_view->visible());
// Corner view should be aligned to the scrollbars.
EXPECT_EQ(scroll_view_.vertical_scroll_bar()->x(), corner_view->x());
EXPECT_EQ(scroll_view_.horizontal_scroll_bar()->y(), corner_view->y());
- EXPECT_EQ(scroll_view_.GetScrollBarWidth(), corner_view->width());
- EXPECT_EQ(scroll_view_.GetScrollBarHeight(), corner_view->height());
+ EXPECT_EQ(scroll_view_.GetScrollBarLayoutWidth(), corner_view->width());
+ EXPECT_EQ(scroll_view_.GetScrollBarLayoutHeight(), corner_view->height());
// Corner view should be removed when only the vertical scrollbar is visible.
contents->SetBounds(0, 0, 50, 200);
@@ -712,7 +727,7 @@ TEST_F(ScrollViewTest, CocoaOverlayScrollBars) {
scroll_view_.Layout();
EXPECT_EQ(100, contents->parent()->width());
EXPECT_EQ(100, contents->parent()->height());
- EXPECT_EQ(0, scroll_view_.GetScrollBarWidth());
+ EXPECT_EQ(0, scroll_view_.GetScrollBarLayoutWidth());
CheckScrollbarVisibility(scroll_view_, VERTICAL, true);
CheckScrollbarVisibility(scroll_view_, HORIZONTAL, false);
@@ -721,7 +736,7 @@ TEST_F(ScrollViewTest, CocoaOverlayScrollBars) {
scroll_view_.Layout();
EXPECT_EQ(100, contents->parent()->width());
EXPECT_EQ(100, contents->parent()->height());
- EXPECT_EQ(0, scroll_view_.GetScrollBarHeight());
+ EXPECT_EQ(0, scroll_view_.GetScrollBarLayoutHeight());
CheckScrollbarVisibility(scroll_view_, VERTICAL, false);
CheckScrollbarVisibility(scroll_view_, HORIZONTAL, true);
@@ -730,8 +745,8 @@ TEST_F(ScrollViewTest, CocoaOverlayScrollBars) {
scroll_view_.Layout();
EXPECT_EQ(100, contents->parent()->width());
EXPECT_EQ(100, contents->parent()->height());
- EXPECT_EQ(0, scroll_view_.GetScrollBarWidth());
- EXPECT_EQ(0, scroll_view_.GetScrollBarHeight());
+ EXPECT_EQ(0, scroll_view_.GetScrollBarLayoutWidth());
+ EXPECT_EQ(0, scroll_view_.GetScrollBarLayoutHeight());
CheckScrollbarVisibility(scroll_view_, VERTICAL, true);
CheckScrollbarVisibility(scroll_view_, HORIZONTAL, true);
@@ -744,12 +759,10 @@ TEST_F(ScrollViewTest, CocoaOverlayScrollBars) {
// Switch to the non-overlay style and check that the ViewPort is now sized
// to be smaller, and ScrollbarWidth and ScrollbarHeight are non-zero.
SetOverlayScrollersEnabled(false);
- EXPECT_EQ(100 - scroll_view_.GetScrollBarWidth(),
- contents->parent()->width());
- EXPECT_EQ(100 - scroll_view_.GetScrollBarHeight(),
- contents->parent()->height());
- EXPECT_NE(0, scroll_view_.GetScrollBarWidth());
- EXPECT_NE(0, scroll_view_.GetScrollBarHeight());
+ EXPECT_EQ(100 - VerticalScrollBarWidth(), contents->parent()->width());
+ EXPECT_EQ(100 - HorizontalScrollBarHeight(), contents->parent()->height());
+ EXPECT_NE(0, VerticalScrollBarWidth());
+ EXPECT_NE(0, HorizontalScrollBarHeight());
}
// Test overlay scrollbar behavior when just resting fingers on the trackpad.
« ui/views/controls/scroll_view.cc ('K') | « ui/views/controls/scroll_view.cc ('k') | ui/views/controls/scrollbar/base_scroll_bar.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698