Merge lp:~brandontschaefer/unity/lp.1163616-fix into lp:unity

Proposed by Brandon Schaefer
Status: Merged
Approved by: Andrea Azzarone
Approved revision: no longer in the source branch.
Merged at revision: 3270
Proposed branch: lp:~brandontschaefer/unity/lp.1163616-fix
Merge into: lp:unity
Diff against target: 84 lines (+24/-7)
2 files modified
tests/test_overlay_scrollbar.cpp (+19/-0)
unity-shared/PlacesOverlayVScrollBar.cpp (+5/-7)
To merge this branch: bzr merge lp:~brandontschaefer/unity/lp.1163616-fix
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Andrea Azzarone (community) Approve
Review via email: mp+156721@code.launchpad.net

This proposal supersedes a proposal from 2013-01-16.

Commit message

Allows dragging of the normal scrollbar if the overlay scrollbars have reached the top/bottom.

Description of the change

Nick mentioned that it is very hard to use the scroll bar to drag all the way down. It is very noticeable in the previews for an application. I think it would be better if it would stop scrolling when the scroll bar it self hit the bottom/top not the overlay thumb.

Here is what it looks like in unity right now:
http://ubuntuone.com/1jc4PcPg8dsFH85r0AoO0U

As you can see, it is hard to scroll through the text, as the thumb it self is larger then the scrollbar, so you will have to scroll down, then move up and scroll down....slightly annoying.

**Note** John (Design) has approved of this change.

To post a comment you must log in.
Revision history for this message
Andrea Azzarone (azzar1) wrote :

LGTM.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'tests/test_overlay_scrollbar.cpp'
--- tests/test_overlay_scrollbar.cpp 2013-01-17 16:11:14 +0000
+++ tests/test_overlay_scrollbar.cpp 2013-04-02 23:43:24 +0000
@@ -64,6 +64,7 @@
64 : PlacesOverlayVScrollBar(NUX_FILE_LINE_PARAM)64 : PlacesOverlayVScrollBar(NUX_FILE_LINE_PARAM)
65 , scroll_tick_(1000 * 401)65 , scroll_tick_(1000 * 401)
66 , scroll_dy_(0)66 , scroll_dy_(0)
67 , thumbs_height_(overlay_window_->GetThumbGeometry().height)
67 , scroll_up_signal_(false)68 , scroll_up_signal_(false)
68 , scroll_down_signal_(false)69 , scroll_down_signal_(false)
69 {70 {
@@ -182,6 +183,11 @@
182 tick_source_.tick(scroll_tick_);183 tick_source_.tick(scroll_tick_);
183 }184 }
184185
186 void FakeDragDown()
187 {
188 OnMouseDrag(0, overlay_window_->GetThumbOffsetY() + 1, 0, 5, 0, 0);
189 }
190
185 nux::NuxTimerTickSource tick_source_;191 nux::NuxTimerTickSource tick_source_;
186192
187 using PlacesOverlayVScrollBar::connector_height_;193 using PlacesOverlayVScrollBar::connector_height_;
@@ -189,6 +195,7 @@
189195
190 int scroll_tick_;196 int scroll_tick_;
191 int scroll_dy_;197 int scroll_dy_;
198 int thumbs_height_;
192 bool scroll_up_signal_;199 bool scroll_up_signal_;
193 bool scroll_down_signal_;200 bool scroll_down_signal_;
194};201};
@@ -419,5 +426,17 @@
419 EXPECT_EQ(connector_height, 0);426 EXPECT_EQ(connector_height, 0);
420}427}
421428
429
430TEST_F(TestOverlayVScrollBar, TestAllowDragIfOverlayIsAtMaximum)
431{
432 // Offset that sets the thumb at the bottom of the scrollbar
433 int thumb_offset = scroll_view_->scroll_bar_->GetHeight() -
434 scroll_view_->scroll_bar_->thumbs_height_;
435
436 scroll_view_->scroll_bar_->SetThumbOffset(thumb_offset);
437 scroll_view_->scroll_bar_->FakeDragDown();
438 EXPECT_TRUE(scroll_view_->scroll_bar_->scroll_down_signal_);
439}
440
422}441}
423442
424443
=== modified file 'unity-shared/PlacesOverlayVScrollBar.cpp'
--- unity-shared/PlacesOverlayVScrollBar.cpp 2012-12-13 14:26:56 +0000
+++ unity-shared/PlacesOverlayVScrollBar.cpp 2013-04-02 23:43:24 +0000
@@ -85,7 +85,7 @@
85 if (!sensitive)85 if (!sensitive)
86 {86 {
87 overlay_window_->ResetStates();87 overlay_window_->ResetStates();
88 ResetConnector(); 88 ResetConnector();
89 }89 }
90}90}
9191
@@ -353,17 +353,15 @@
353 MouseDraggingOverlay(y, dy);353 MouseDraggingOverlay(y, dy);
354}354}
355355
356void PlacesOverlayVScrollBar::MouseDraggingOverlay(int y, int dys)356void PlacesOverlayVScrollBar::MouseDraggingOverlay(int y, int dy)
357{357{
358 int const dy = y - overlay_window_->GetThumbOffsetY() - mouse_down_offset_;358 int const thumb_offset = overlay_window_->GetThumbOffsetY() + mouse_down_offset_;
359 int const at_min = overlay_window_->GetThumbOffsetY() <= 0;
360 int const at_max = overlay_window_->GetThumbOffsetY() + overlay_window_->GetThumbHeight() >= _track->GetBaseHeight();
361359
362 if (dy < 0 && !at_min)360 if (dy < 0 && !AtMinimum() && y <= thumb_offset)
363 {361 {
364 OnScrollUp.emit(stepY, abs(dy));362 OnScrollUp.emit(stepY, abs(dy));
365 }363 }
366 else if (dy > 0 && !at_max)364 else if (dy > 0 && !AtMaximum() && y >= thumb_offset)
367 {365 {
368 OnScrollDown.emit(stepY, dy);366 OnScrollDown.emit(stepY, dy);
369 }367 }