Merge lp:~faenil/ubuntu-ui-toolkit/scrollbar_moreHoverFixesAndTests into lp:ubuntu-ui-toolkit/staging

Proposed by Andrea Bernabei on 2016-08-26
Status: Merged
Merge reported by: Christian Dywan
Merged at revision: not available
Proposed branch: lp:~faenil/ubuntu-ui-toolkit/scrollbar_moreHoverFixesAndTests
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 662 lines (+290/-154)
3 files modified
src/Ubuntu/Components/Themes/Ambiance/1.3/ScrollbarStyle.qml (+49/-28)
tests/unit/visual/ScrollbarTestCase13.qml (+3/-0)
tests/unit/visual/tst_scrollbar.13.qml (+238/-126)
To merge this branch: bzr merge lp:~faenil/ubuntu-ui-toolkit/scrollbar_moreHoverFixesAndTests
Reviewer Review Type Date Requested Status
ubuntu-sdk-build-bot continuous-integration Needs Fixing on 2016-08-29
Christian Dywan (community) 2016-08-26 Approve on 2016-08-29
Review via email: mp+304139@code.launchpad.net

Commit message

Scrollbar: more hover related bugfixes and unit tests.

Description of the change

Fixes bug #1616926 and bug #1616868

More details on the problems and their solutions can be found on the bug threads

To post a comment you must log in.
Christian Dywan (kalikiana) wrote :

This indeed resolves the two linked bugs (which I only realized existed once I paid very close attention to the different shades of gray). I evaluated the changes with respect to the necessary implications of the behavior causing the bugs - strictly speaking it's really bad to have to deal with such counter-intuitive details of mouse area behavior so I won't say that it "makes sense".

review: Approve
Timo Jyrinki (timo-jyrinki) wrote :

Phones down, re-topapproving.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Ubuntu/Components/Themes/Ambiance/1.3/ScrollbarStyle.qml'
2--- src/Ubuntu/Components/Themes/Ambiance/1.3/ScrollbarStyle.qml 2016-08-02 11:18:34 +0000
3+++ src/Ubuntu/Components/Themes/Ambiance/1.3/ScrollbarStyle.qml 2016-08-27 15:14:55 +0000
4@@ -316,7 +316,9 @@
5 property string propPosRatio: (isVertical) ? "yPosition" : "xPosition"
6 property string propSizeRatio: (isVertical) ? "heightRatio" : "widthRatio"
7 property string propCoordinate: (isVertical) ? "y" : "x"
8+ property string otherPropCoordinate: (isVertical) ? "x" : "y"
9 property string propSize: (isVertical) ? "height" : "width"
10+ property string otherPropSize: (isVertical) ? "width" : "height"
11 property string propAtBeginning: (isVertical) ? "atYBeginning" : "atXBeginning"
12 property string propAtEnd: (isVertical) ? "atYEnd" : "atXEnd"
13
14@@ -622,8 +624,6 @@
15 property bool lockDrag: false
16
17 property bool hoveringThumb: false
18- property string scrollingProp: isVertical ? "y" : "x"
19- property string sizeProp: isVertical ? "height" : "width"
20
21 function initDrag(isMouse) {
22 __disableStateBinding = true
23@@ -648,20 +648,29 @@
24 if (!thumbArea.pressed) return
25
26 var mouseScrollingProp = isVertical ? mouseY : mouseX
27- if (mouseScrollingProp < slider[scrollingProp]) {
28+ if (mouseScrollingProp < slider[scrollbarUtils.propCoordinate]) {
29 scrollBy(-pageSize*visuals.longScrollingRatio, true)
30- } else if (mouseScrollingProp > slider[scrollingProp] + slider[sizeProp]) {
31+ } else if (mouseScrollingProp > slider[scrollbarUtils.propCoordinate] + slider[scrollbarUtils.propSize]) {
32 scrollBy(pageSize*visuals.longScrollingRatio, true)
33 }
34 }
35
36+ //we consider hover if it's inside the TROUGH along the scrolling axis
37+ //and inside the THUMB along the non-scrolling axis
38+ //NOTE: mouseX and mouseY are assumed to be relative to thumbArea
39 function handleHover(mouseX, mouseY) {
40+ //NOTE: we're assuming thumbArea has same size/position as the trough.
41+ //Use mapToItem to map the coordinates if that assumption falls (i.e. to implement
42+ //a larger touch detection area around the thumb)
43 var mouseScrollingProp = isVertical ? mouseY : mouseX
44- //don't count as hover if the user is already press-and-holding the trough to
45- //scroll page by page
46+ var otherProp = isVertical ? mouseX : mouseY
47+
48+ //don't count as hover if the user is already press-and-holding
49+ //the trough to scroll page by page
50 hoveringThumb = !(pressHoldTimer.running && pressHoldTimer.startedBy === thumbArea)
51- && mouseScrollingProp >= slider[scrollingProp] &&
52- mouseScrollingProp <= slider[scrollingProp] + slider[sizeProp]
53+ && mouseScrollingProp >= slider[scrollbarUtils.propCoordinate] &&
54+ mouseScrollingProp <= slider[scrollbarUtils.propCoordinate] + slider[scrollbarUtils.propSize] &&
55+ otherProp >= 0 && otherProp <= trough[scrollbarUtils.otherPropSize]
56 }
57
58 function saveFlickableScrollingState() {
59@@ -691,14 +700,9 @@
60 previousY = mouse.y
61 }
62
63- anchors {
64- fill: trough
65- // set margins adding 2 dp for error area
66- leftMargin: (!isVertical || frontAligned) ? 0 : units.dp(-2)
67- rightMargin: (!isVertical || rearAligned) ? 0 : units.dp(-2)
68- topMargin: (isVertical || topAligned) ? 0 : units.dp(-2)
69- bottomMargin: (isVertical || bottomAligned) ? 0 : units.dp(-2)
70- }
71+ //NOTE: remember to update the handleHover check if you add anchor margins
72+ anchors.fill: trough
73+
74 enabled: isScrollable && interactive && __canFitSteppersAndShorterThumb
75 onPressed: {
76 cacheMousePosition(mouse)
77@@ -710,8 +714,8 @@
78 if (firstStepper.visible) {
79 var mouseScrollingProp = isVertical ? mouseY : mouseX
80 //don't start the press and hold timer to avoid conflicts with the drag
81- if (mouseScrollingProp < slider[scrollingProp] ||
82- mouseScrollingProp > (slider[scrollingProp] + slider[sizeProp])) {
83+ if (mouseScrollingProp < slider[scrollbarUtils.propCoordinate] ||
84+ mouseScrollingProp > (slider[scrollbarUtils.propCoordinate] + slider[scrollbarUtils.propSize])) {
85 handlePress(mouseX, mouseY)
86 pressHoldTimer.startTimer(thumbArea)
87 } else {
88@@ -761,7 +765,8 @@
89 pressHoldTimer.stop()
90 }
91 onReleased: {
92- handleHover(mouseX, mouseY)
93+ //don't call handleHover here as touch release also triggers this handler
94+ //see bug #1616868
95 resetDrag()
96 pressHoldTimer.stop()
97 }
98@@ -797,18 +802,29 @@
99
100 //logic to support different colour on hovering
101 hoverEnabled: true
102+ //This means this mouse filter will only handle real mouse events!
103+ //i.e. the synthesized mouse events created when you use
104+ //touchscreen will not trigger it! This way we can have logic that
105+ //will not trigger when using touch
106+ Mouse.ignoreSynthesizedEvents: true
107 Mouse.enabled: true
108- Mouse.ignoreSynthesizedEvents: true
109 Mouse.onEntered: {
110 hoveringThumb = false
111 handleHover(event.x, event.y)
112 }
113 Mouse.onPositionChanged: {
114+ //We need to update the hover state also onPosChanged because this area
115+ //covers the whole trough, not just the thumb, so entered/exited are not enough
116+ //e.g. when mouse moves from inside the thumb to inside the trough, or when you
117+ //click on the trough and the thumb scrolls and goes under the mouse cursor
118 handleHover(mouse.x, mouse.y)
119 }
120 Mouse.onExited: {
121 hoveringThumb = false
122 }
123+ Mouse.onReleased: {
124+ handleHover(mouse.x, mouse.y)
125+ }
126
127 Timer {
128 id: pressHoldTimer
129@@ -845,6 +861,7 @@
130
131 MouseArea {
132 id: steppersMouseArea
133+ objectName: "steppersMouseArea"
134 //size is handled by the states
135
136 property bool hoveringFirstStepper: false
137@@ -897,12 +914,7 @@
138 hoveringFirstStepper = false
139 hoveringSecondStepper = false
140 }
141-
142- //We don't change the size of the images because we let the image reader figure the size out,
143- //though that means we have to hide the images somehow while the mousearea is visible but has
144- //null size. We choose to enable clipping here instead of creating bindings on images' visible prop
145- clip: true
146- onPressed: {
147+ Mouse.onPressed: {
148 //This additional trigger of the hovering logic is useful especially in testing
149 //environments, where simulating a click on the first stepper will generate onEntered,
150 //but then clicking on the second one (without a mouseMove) won't, because they're
151@@ -910,7 +922,16 @@
152 //we ensure that the hovering logic is correct even when there are no mouse moves between
153 //clicks on different steppers (like it happens in tst_actionSteppers test).
154 handleHover(mouse)
155+ }
156+ Mouse.onReleased: {
157+ handleHover(mouse)
158+ }
159
160+ //We don't change the size of the images because we let the image reader figure the size out,
161+ //though that means we have to hide the images somehow while the mousearea is visible but has
162+ //null size. We choose to enable clipping here instead of creating bindings on images' visible prop
163+ clip: true
164+ onPressed: {
165 handlePress()
166 pressHoldTimer.startTimer(steppersMouseArea)
167 }
168@@ -952,7 +973,7 @@
169 anchors.centerIn: parent
170 width: __stepperAssetWidth
171 rotation: isVertical ? 180 : 90
172- //NOTE: Qt.resolvedUrl was removed because it does not seem to be needed and
173+ //NOTE: Qt.resolvedUrl was removed because it does not seem to be needed and
174 //the QML profiler showed it's relatively expensive
175 source: visible ? "../artwork/toolkit_scrollbar-stepper.svg" : ""
176 asynchronous: true
177@@ -998,7 +1019,7 @@
178 anchors.centerIn: parent
179 width: __stepperAssetWidth
180 rotation: isVertical ? 0 : -90
181- //NOTE: Qt.resolvedUrl was removed because it does not seem to be needed and
182+ //NOTE: Qt.resolvedUrl was removed because it does not seem to be needed and
183 //the QML profiler showed it's relatively expensive
184 source: visible ? "../artwork/toolkit_scrollbar-stepper.svg" : ""
185 asynchronous: true
186
187=== modified file 'tests/unit/visual/ScrollbarTestCase13.qml'
188--- tests/unit/visual/ScrollbarTestCase13.qml 2016-06-08 11:36:03 +0000
189+++ tests/unit/visual/ScrollbarTestCase13.qml 2016-08-27 15:14:55 +0000
190@@ -242,6 +242,9 @@
191 function getThumbArea(scrollbar) {
192 return findChildAndCheckValidInstance(scrollbar.__styleInstance, "thumbArea")
193 }
194+ function getSteppersMouseArea(scrollbar) {
195+ return findChildAndCheckValidInstance(scrollbar.__styleInstance, "steppersMouseArea")
196+ }
197 function getScrollAnimation(scrollbar) {
198 var anim = findInvisibleChild(scrollbar.__styleInstance, "scrollAnimation")
199 verify(anim !== null, "Returning a valid reference to scrollAnimation.")
200
201=== modified file 'tests/unit/visual/tst_scrollbar.13.qml'
202--- tests/unit/visual/tst_scrollbar.13.qml 2016-08-02 13:43:39 +0000
203+++ tests/unit/visual/tst_scrollbar.13.qml 2016-08-27 15:14:55 +0000
204@@ -250,7 +250,7 @@
205 nullFlickableScrollbar.destroy()
206 }
207
208- //no need to test the anchors values when flickable is not set because we already
209+ //no need to test the anchors values when flickable is not set because we already
210 //test that the scrollbar is hidden when there's no flickable set
211 function test_bottomAlign_anchors() {
212 compare(scrollbar_bottomAlign_anchors.flickableItem,
213@@ -438,40 +438,101 @@
214 }
215 }
216
217- function test_checkStepperStatesStyling(data) {
218- var freshTestItem = getFreshFlickable(data.alignment)
219- var flickable = freshTestItem.flickable
220- var scrollbar = freshTestItem.scrollbar
221- var style = scrollbar.__styleInstance
222+ function checkSteppersColour(scrollbar, firstStepperStateString, secondStepperStateString, testDescription) {
223 var firstStepper = getFirstStepper(scrollbar)
224 var secondStepper = getSecondStepper(scrollbar)
225-
226- triggerSteppersMode(scrollbar)
227-
228- //the following tests are assuming that the steppers are not disabled
229- compare(style.isScrollable, true, "Scrollbar is not scrollable, cannot test steppers hover/pressed state.")
230-
231 var firstStepperIcon = getFirstStepperIcon(scrollbar)
232 var secondStepperIcon = getSecondStepperIcon(scrollbar)
233+ var style = scrollbar.__styleInstance
234
235- //bg color on hover/pressed. Otherwise it should be transparent so that it shows the same bg as the trough
236 var stepperBgColorBase = style.stepperBgColor
237-
238 var stepperImgColor = style.sliderColor
239+
240+ var stepperBgColorDisabled = "transparent"
241+ var stepperImgColorDisabled = Qt.rgba(stepperImgColor.r, stepperImgColor.g, stepperImgColor.b,
242+ stepperImgColor.a * style.__stepperImgOpacityDisabled)
243+ var stepperBgColorNormal = "transparent"
244+ var stepperImgColorNormal = Qt.rgba(stepperImgColor.r, stepperImgColor.g, stepperImgColor.b,
245+ stepperImgColor.a * style.__stepperImgOpacityNormal)
246+ var stepperBgColorOnHover = Qt.rgba(stepperBgColorBase.r, stepperBgColorBase.g, stepperBgColorBase.b,
247+ stepperBgColorBase.a * style.__stepperBgOpacityOnHover)
248 var stepperImgColorOnHover = Qt.rgba(stepperImgColor.r, stepperImgColor.g, stepperImgColor.b,
249 stepperImgColor.a * style.__stepperImgOpacityOnHover)
250- var stepperBgColorOnHover = Qt.rgba(stepperBgColorBase.r, stepperBgColorBase.g, stepperBgColorBase.b,
251- stepperBgColorBase.a * style.__stepperBgOpacityOnHover)
252+ var stepperBgColorOnPressed = Qt.rgba(stepperBgColorBase.r, stepperBgColorBase.g, stepperBgColorBase.b,
253+ stepperBgColorBase.a * style.__stepperBgOpacityOnPressed)
254 var stepperImgColorOnPressed = Qt.rgba(stepperImgColor.r, stepperImgColor.g, stepperImgColor.b,
255 stepperImgColor.a * style.__stepperImgOpacityOnPressed)
256- var stepperBgColorOnPressed = Qt.rgba(stepperBgColorBase.r, stepperBgColorBase.g, stepperBgColorBase.b,
257- stepperBgColorBase.a * style.__stepperBgOpacityOnPressed)
258- var stepperBgColorNormal = "transparent"
259- var stepperImgColorNormal = Qt.rgba(stepperImgColor.r, stepperImgColor.g, stepperImgColor.b,
260- stepperImgColor.a * style.__stepperImgOpacityNormal)
261- var stepperBgColorDisabled = "transparent"
262- var stepperImgColorDisabled = Qt.rgba(stepperImgColor.r, stepperImgColor.g, stepperImgColor.b,
263- stepperImgColor.a * style.__stepperImgOpacityDisabled)
264+
265+ switch (firstStepperStateString) {
266+ case "disabled":
267+ var expectedFirstStepperColour = stepperBgColorDisabled
268+ var expectedFirstStepperIconColour = stepperImgColorDisabled
269+ break
270+ case "normal":
271+ var expectedFirstStepperColour = stepperBgColorNormal
272+ var expectedFirstStepperIconColour = stepperImgColorNormal
273+ break
274+ case "hovered":
275+ var expectedFirstStepperColour = stepperBgColorOnHover
276+ var expectedFirstStepperIconColour = stepperImgColorOnHover
277+ break
278+ case "pressed":
279+ var expectedFirstStepperColour = stepperBgColorOnPressed
280+ var expectedFirstStepperIconColour = stepperImgColorOnPressed
281+ break
282+ default:
283+ fail("Invalid stepper state string")
284+ }
285+
286+ switch (secondStepperStateString) {
287+ case "disabled":
288+ var expectedSecondStepperColour = stepperBgColorDisabled
289+ var expectedSecondStepperIconColour = stepperImgColorDisabled
290+ break
291+ case "normal":
292+ var expectedSecondStepperColour = stepperBgColorNormal
293+ var expectedSecondStepperIconColour = stepperImgColorNormal
294+ break
295+ case "hovered":
296+ var expectedSecondStepperColour = stepperBgColorOnHover
297+ var expectedSecondStepperIconColour = stepperImgColorOnHover
298+ break
299+ case "pressed":
300+ var expectedSecondStepperColour = stepperBgColorOnPressed
301+ var expectedSecondStepperIconColour = stepperImgColorOnPressed
302+ break
303+ default:
304+ fail("Invalid stepper state string")
305+ }
306+
307+ var msg = "First stepper expected state: " + firstStepperStateString + "."
308+ + " Second stepper expected state: " + secondStepperStateString + ". "
309+ + " Test description: " + testDescription + ". "
310+ + " Error encountered: "
311+
312+ //compare() returns pass on different colours, see https://bugreports.qt.io/browse/QTBUG-34878
313+ compare(Qt.colorEqual(firstStepper.color, expectedFirstStepperColour), true,
314+ msg + "wrong first stepper background color.")
315+ compare(Qt.colorEqual(firstStepperIcon.color, expectedFirstStepperIconColour), true,
316+ msg + "wrong first stepper icon color.")
317+ compare(Qt.colorEqual(secondStepper.color, expectedSecondStepperColour), true,
318+ msg + "wrong second stepper background color.")
319+ compare(Qt.colorEqual(secondStepperIcon.color, expectedSecondStepperIconColour), true,
320+ msg + "wrong second stepper icon color.")
321+ }
322+
323+ function test_checkStepperStatesStyling(data) {
324+ var freshTestItem = getFreshFlickable(data.alignment)
325+ var flickable = freshTestItem.flickable
326+ var scrollbar = freshTestItem.scrollbar
327+ var style = scrollbar.__styleInstance
328+ var firstStepper = getFirstStepper(scrollbar)
329+ var secondStepper = getSecondStepper(scrollbar)
330+
331+ triggerSteppersMode(scrollbar)
332+
333+ //the following tests are assuming that the steppers are not disabled
334+ compare(style.isScrollable, true, "Scrollbar is not scrollable, cannot test steppers hover/pressed state.")
335
336 if (style.isVertical) {
337 compare(style.flickableItem.atYBeginning, true, "View not scrolled to the beginning as expected.")
338@@ -482,38 +543,17 @@
339 }
340
341 //Check that the first stepper is disabled and the second one is in normal state
342- compare(Qt.colorEqual(firstStepper.color, stepperBgColorDisabled), true,
343- "Wrong first stepper bg color in disabled state.")
344- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorDisabled), true,
345- "Wrong first stepper img color in disabled state.")
346- compare(Qt.colorEqual(secondStepper.color, stepperBgColorNormal), true,
347- "Wrong second stepper bg color when it should be in normal state.")
348- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorNormal), true,
349- "Wrong second stepper img color when it should be in normal state.")
350+ checkSteppersColour(scrollbar, "disabled", "normal", "default state before moving mouse")
351+
352+ //Check that hovering on a disabled stepper does not change its colour
353+ mouseMove(firstStepper, firstStepper.width/2, firstStepper.height/2)
354+ checkSteppersColour(scrollbar, "disabled", "normal", "mouse hovered to disabled first stepper")
355
356 //Check that tapping on a disabled stepper does not change its colour
357 mousePress(firstStepper, firstStepper.width/2, firstStepper.height/2)
358- compare(Qt.colorEqual(firstStepper.color, stepperBgColorDisabled), true,
359- "Pressing first stepper changes its bg colour even when it's disabled.")
360- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorDisabled), true,
361- "Pressing first stepper changes its img colour even when it's disabled.")
362- compare(Qt.colorEqual(secondStepper.color, stepperBgColorNormal), true,
363- "Wrong second stepper bg color when it should be in normal state.")
364- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorNormal), true,
365- "Wrong second stepper img color when it should be in normal state.")
366+ checkSteppersColour(scrollbar, "disabled", "normal", "mouse pressed on disabled first stepper")
367 mouseRelease(firstStepper, firstStepper.width/2, firstStepper.height/2)
368
369- //Check that hovering on a disabled stepper does not change its colour
370- mouseMove(firstStepper, firstStepper.width/2, firstStepper.height/2)
371- compare(Qt.colorEqual(firstStepper.color, stepperBgColorDisabled), true,
372- "Hovering on first stepper changes its bg colour even when it's disabled.")
373- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorDisabled), true,
374- "Hovering on first stepper changes its img colour even when it's disabled.")
375- compare(Qt.colorEqual(secondStepper.color, stepperBgColorNormal), true,
376- "Wrong second stepper bg color when it should be in normal state.")
377- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorNormal), true,
378- "Wrong second stepper img color when it should be in normal state.")
379-
380 //move mouse away from the steppers
381 mouseMove(scrollbar, 0, 0)
382
383@@ -533,44 +573,23 @@
384 }
385
386
387- //Check that the first stepper is disabled and the second one is in normal state
388- compare(Qt.colorEqual(firstStepper.color, stepperBgColorNormal), true,
389- "Wrong first stepper bg color when it should be in normal state.")
390- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorNormal), true,
391- "Wrong first stepper img color when it should be in normal state.")
392- compare(Qt.colorEqual(secondStepper.color, stepperBgColorDisabled), true,
393- "Wrong second stepper bg color in disabled state.")
394- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorDisabled), true,
395- "Wrong second stepper img color in disabled state.")
396+ //Check that the first stepper is normal and the second one is in disable state
397+ checkSteppersColour(scrollbar, "normal", "disabled", "after thumb dragged to bottom and mouse moved away from steppers.")
398+
399+ //Check that hovering on a disabled stepper does not change its colour
400+ mouseMove(secondStepper, secondStepper.width/2, secondStepper.height/2)
401+ checkSteppersColour(scrollbar, "normal", "disabled", "thumb at the bottom, mouse hovered on disabled second stepper")
402
403 //Check that tapping on a disabled stepper does not change its colour
404 mousePress(secondStepper, secondStepper.width/2, secondStepper.height/2)
405- compare(Qt.colorEqual(firstStepper.color, stepperBgColorNormal), true,
406- "Wrong first stepper bg color when it should be in normal state.")
407- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorNormal), true,
408- "Wrong first stepper img color when it should be in normal state.")
409- compare(Qt.colorEqual(secondStepper.color, stepperBgColorDisabled), true,
410- "Pressing second stepper changes its bg colour even when it's disabled.")
411- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorDisabled), true,
412- "Pressing second stepper changes its img colour even when it's disabled.")
413+ checkSteppersColour(scrollbar, "normal", "disabled", "thumb at the bottom, mouse pressed on disabled second stepper")
414 mouseRelease(secondStepper, secondStepper.width/2, secondStepper.height/2)
415
416- //Check that hovering on a disabled stepper does not change its colour
417- mouseMove(secondStepper, secondStepper.width/2, secondStepper.height/2)
418- compare(Qt.colorEqual(firstStepper.color, stepperBgColorNormal), true,
419- "Wrong first stepper bg color when it should be in normal state.")
420- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorNormal), true,
421- "Wrong first stepper img color when it should be in normal state.")
422- compare(Qt.colorEqual(secondStepper.color, stepperBgColorDisabled), true,
423- "Hovering on second stepper changes its bg colour even when it's disabled.")
424- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorDisabled), true,
425- "Hovering on second stepper changes its img colour even when it's disabled.")
426-
427 //move mouse away from the steppers
428 mouseMove(scrollbar, 0, 0)
429
430- //trigger first stepper to scroll down a bit, so that they're both not disabled
431- //scrolling down with a stepper should not scroll to the top, unless there's a bug somewhere else
432+ //trigger first stepper to scroll up a bit, so that they're both not disabled
433+ //scrolling up with a stepper should not scroll to the top, unless there's a bug somewhere else
434 //or the flickable test item gets changed
435 clickInTheMiddle(firstStepper)
436 checkScrolling(flickable, style.flickableItem.contentX, style.flickableItem.contentY, style,
437@@ -589,65 +608,46 @@
438 mouseMove(scrollbar, 0, 0)
439
440 //Check colours in normal state
441- compare(Qt.colorEqual(firstStepper.color, stepperBgColorNormal), true,
442- "Wrong first stepper bg color in normal state.")
443- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorNormal), true,
444- "Wrong first stepper img color in normal state.")
445- compare(Qt.colorEqual(secondStepper.color, stepperBgColorNormal), true,
446- "Wrong second stepper bg color in normal state.")
447- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorNormal), true,
448- "Wrong second stepper img color in normal state.")
449+ checkSteppersColour(scrollbar, "normal", "normal", "mouse moved away from steppers")
450
451 //Hover on first stepper and check colour of both steppers
452 mouseMove(firstStepper, firstStepper.width/2, firstStepper.height/2)
453- //compare() returns pass on different colours, see https://bugreports.qt.io/browse/QTBUG-34878
454- compare(Qt.colorEqual(firstStepper.color, stepperBgColorOnHover), true,
455- "Wrong first stepper bg color on hover.")
456- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorOnHover), true,
457- "Wrong first stepper img color on hover.")
458- compare(Qt.colorEqual(secondStepper.color, stepperBgColorNormal), true,
459- "Wrong second stepper bg color when hovering on first stepper.")
460- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorNormal), true,
461- "Wrong second stepper img color when hovering on first stepper.")
462+ checkSteppersColour(scrollbar, "hovered", "normal", "mouse hovered on first stepper")
463
464 //Press on first stepper and check colour of both steppers
465 mousePress(firstStepper, firstStepper.width/2, firstStepper.height/2)
466- compare(Qt.colorEqual(firstStepper.color, stepperBgColorOnPressed), true,
467- "Wrong first stepper bg color on pressed.")
468- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorOnPressed), true,
469- "Wrong first stepper img color on pressed.")
470- compare(Qt.colorEqual(secondStepper.color, stepperBgColorNormal), true,
471- "Wrong second stepper bg color when pressing on first stepper.")
472- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorNormal), true,
473- "Wrong second stepper img color when pressing on first stepper.")
474+ checkSteppersColour(scrollbar, "pressed", "normal", "mouse pressed on first stepper")
475 mouseRelease(firstStepper, firstStepper.width/2, firstStepper.height/2)
476
477 //Hover on second stepper and check colour of both steppers
478 mouseMove(secondStepper, secondStepper.width/2, secondStepper.height/2)
479- compare(Qt.colorEqual(firstStepper.color, stepperBgColorNormal), true,
480- "Wrong first stepper bg color when hovering on second stepper.")
481- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorNormal), true,
482- "Wrong first stepper img color when hovering on second stepper.")
483- compare(Qt.colorEqual(secondStepper.color, stepperBgColorOnHover), true,
484- "Wrong second stepper bg color on hover.")
485- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorOnHover), true,
486- "Wrong second stepper img color on hover.")
487+ checkSteppersColour(scrollbar, "normal", "hovered", "mouse hovered on second stepper")
488
489 //Press on second stepper and check colour of both steppers
490 mousePress(secondStepper, secondStepper.width/2, secondStepper.height/2)
491- compare(Qt.colorEqual(firstStepper.color, stepperBgColorNormal), true,
492- "Wrong first stepper bg color when pressing on second stepper.")
493- compare(Qt.colorEqual(firstStepperIcon.color, stepperImgColorNormal), true,
494- "Wrong first stepper img color when pressing on second stepper.")
495- compare(Qt.colorEqual(secondStepper.color, stepperBgColorOnPressed), true,
496- "Wrong second stepper bg color on pressed.")
497- compare(Qt.colorEqual(secondStepperIcon.color, stepperImgColorOnPressed), true,
498- "Wrong second stepper img color on pressed.")
499- mouseRelease(secondStepper, secondStepper.width/2, secondStepper.height/2)
500+ checkSteppersColour(scrollbar, "normal", "pressed", "mouse pressed on second stepper")
501+ mouseRelease(secondStepper, secondStepper.width/2, secondStepper.height/2)
502+
503+ //Check that pressing on a stepper (mouse-only) will activate the hover state
504+ //even without a mouse move (i.e. without onEntered/onExited, which can often
505+ //happen in unit tests)
506+
507+ //Press on first stepper without first firing mouseMove and check colour of both steppers
508+ mousePress(firstStepper, firstStepper.width/2, firstStepper.height/2)
509+ checkSteppersColour(scrollbar, "pressed", "normal", "mouse pressed on first stepper without sending mouseMove before")
510+ mouseRelease(firstStepper, firstStepper.width/2, firstStepper.height/2)
511+ checkSteppersColour(scrollbar, "hovered", "normal", "after mouse released while on first stepper")
512+
513+ //Press on second stepper without first firing mouseMove and check colour of both steppers
514+ mousePress(secondStepper, secondStepper.width/2, secondStepper.height/2)
515+ checkSteppersColour(scrollbar, "normal", "pressed", "mouse pressed on second stepper without sending mouseMove before")
516+ mouseRelease(secondStepper, secondStepper.width/2, secondStepper.height/2)
517+ checkSteppersColour(scrollbar, "normal", "hovered", "after mouse released while on second stepper")
518 }
519
520- //test dragging the thumb and relative visual changes due to hover/pressed states
521- function test_dragThumbAndCheckStyling(data) {
522+
523+ //test that moving the mouse inside and outside any of the hover area borders has the expected effect
524+ function test_thumbHoverArea(data) {
525 var freshTestItem = getFreshFlickable(data.alignment)
526 var flickable = freshTestItem.flickable
527 var scrollbar = freshTestItem.scrollbar
528@@ -673,7 +673,7 @@
529 //check colour of the thumb in normal state
530 compare(Qt.colorEqual(thumb.color, thumbNormalColor), true, "Wrong thumb color in normal state.")
531
532- //check hovered colour
533+ //hover on the middle
534 mouseMove(thumb, thumb.width/2, thumb.height/2)
535 compare(Qt.colorEqual(thumb.color, thumbHoveredColor), true, "Wrong thumb color in hover state.")
536
537@@ -695,6 +695,102 @@
538 compare(Qt.colorEqual(thumb.color, thumbHoveredColor), true,
539 "Thumb does not show as hovered after mouse press-release inside it.")
540
541+ //depending on how the implementation of the scrollbar evolves, the input area
542+ //may become bigger than the trough on one or both sides, so we map the coords
543+ //to know where to send events relative to the thumb so that they hit the trough
544+ //(which is part of the hover area, along the scrolling axis, i.e. x-axis for vert.scrollbar)
545+ var mappedCoords = thumb.mapToItem(trough, 0, 0)
546+
547+ //mouseMove seem to deliver events at pixel bound, so we have to take the floor of the sizes
548+ //otherwise if thumb has width 50.6, mouseMove(thumb, 50.6, 0) will send an event to x==51 and that
549+ //will cause the logic to assume the mouse is outside the hover area while it's on the border
550+ var floorThumbWidth = Math.floor(thumb.width)
551+ var floorThumbHeight = Math.floor(thumb.height)
552+ var floorTroughWidth = Math.floor(trough.width)
553+ var floorTroughHeight = Math.floor(trough.height)
554+
555+ //top-left and bottom-right coords of the rectangle defining the thumb hover area
556+ //we need different handling of vertical/horizontal because the hover area is defined
557+ //as the whole trough along the scrolling axis (e.g. whole trough width for the vert. scrollbar)
558+ //and exactly the thumb along the non-scrolling axis (e.g. thumb's height for vert. scrollbar
559+ if (style.isVertical) {
560+ var insideTopLeft = [-mappedCoords.x , 0 ]
561+ var insideBottomRight = [-mappedCoords.x + floorTroughWidth, floorThumbHeight]
562+ } else {
563+ var insideTopLeft = [0 , -mappedCoords.y ]
564+ var insideBottomRight = [floorThumbWidth, -mappedCoords.y + floorTroughHeight]
565+ }
566+ mouseMove(thumb, insideTopLeft[0], insideTopLeft[1])
567+ compare(Qt.colorEqual(thumb.color, thumbHoveredColor), true,
568+ "Thumb does not show as hovered after mouse moved to the top-left corner of the hover area.")
569+ mouseMove(thumb, insideBottomRight[0], insideBottomRight[1])
570+ compare(Qt.colorEqual(thumb.color, thumbHoveredColor), true,
571+ "Thumb does not show as hovered after mouse moved to the bottom-right corner of the hover area.")
572+
573+ //right outside the hover area
574+ var outsideTopBorder = [insideTopLeft[0] , insideTopLeft[1] - 1] //move 1 up
575+ var outsideLeftBorder = [insideTopLeft[0] - 1 , insideTopLeft[1]] //move 1 left
576+ var outsideBottomBorder = [insideBottomRight[0] , insideBottomRight[1] + 1] //move 1 down
577+ var outsideRightBorder = [insideBottomRight[0] + 1 , insideBottomRight[1]] //move 1 right
578+
579+ //check that when the mouse is right outside the borders of the hover area the thumb will use the non-hover colour
580+ mouseMove(thumb, outsideTopBorder[0], outsideTopBorder[1])
581+ compare(Qt.colorEqual(thumb.color, thumbNormalColor), true,
582+ "Thumb does not show as NOT-hovered after mouse moved outside the top border of the hover area.")
583+ mouseMove(thumb, outsideBottomBorder[0], outsideBottomBorder[1])
584+ compare(Qt.colorEqual(thumb.color, thumbNormalColor), true,
585+ "Thumb does not show as NOT-hovered after mouse moved outside the bottom border of the hover area.")
586+ mouseMove(thumb, outsideLeftBorder[0], outsideLeftBorder[1])
587+ compare(Qt.colorEqual(thumb.color, thumbNormalColor), true,
588+ "Thumb does not show as NOT-hovered after mouse moved outside the left border of the hover area.")
589+ mouseMove(thumb, outsideRightBorder[0], outsideRightBorder[1])
590+ compare(Qt.colorEqual(thumb.color, thumbNormalColor), true,
591+ "Thumb does not show as NOT-hovered after mouse moved outside the right border of the hover area.")
592+
593+ //BUG #1616926
594+ //check pressind on the THUMB and releasing mouse outside the TROUGH resets it to normal state
595+ //press thumb, move mouse outside the trough and check that the thumb is not showing as hovered
596+ //The broken logic was just checking if mouse was inside the thumb
597+ //on the same axis as the scrolling one (so y for vertical scrollbar, x for horizontal) so we move
598+ //mouse in the opposite axis (i.e. along x if vertical scrollbar) to check that the bug is fixed
599+ mousePress(thumb, thumb.width/2, thumb.height/2)
600+ if (style.isVertical) {
601+ mouseMove(thumb, outsideLeftBorder[0], outsideLeftBorder[1])
602+ compare(Qt.colorEqual(thumb.color, thumbPressedColor), true, "Wrong THUMB color in pressed state.")
603+ mouseRelease(thumb, outsideLeftBorder[0], outsideLeftBorder[1])
604+ compare(Qt.colorEqual(thumb.color, thumbNormalColor), true, "Wrong THUMB color after releasing mouse with x outside TROUGH.")
605+ } else {
606+ mouseMove(thumb, outsideTopBorder[0], outsideTopBorder[1])
607+ compare(Qt.colorEqual(thumb.color, thumbPressedColor), true, "Wrong THUMB color in pressed state.")
608+ mouseRelease(thumb, outsideTopBorder[0], outsideTopBorder[1])
609+ compare(Qt.colorEqual(thumb.color, thumbNormalColor), true, "Wrong THUMB color after releasing mouse with y outside TROUGH.")
610+ }
611+ }
612+
613+ //test dragging the thumb and relative visual changes due to hover/pressed states
614+ function test_dragThumbAndCheckStyling(data) {
615+ var freshTestItem = getFreshFlickable(data.alignment)
616+ var flickable = freshTestItem.flickable
617+ var scrollbar = freshTestItem.scrollbar
618+ var thumb = getThumb(scrollbar)
619+ var thumbArea = getThumbArea(scrollbar)
620+ var trough = getTrough(scrollbar)
621+ var style = freshTestItem.scrollbar.__styleInstance
622+ var scrollbarUtils = getScrollbarUtils(scrollbar)
623+ var secondStepper = getSecondStepper(scrollbar)
624+ var thumbNormalColor = Qt.rgba(style.sliderColor.r, style.sliderColor.g, style.sliderColor.b,
625+ style.sliderColor.a * 0.4)
626+ var thumbHoveredColor = Qt.rgba(style.sliderColor.r, style.sliderColor.g, style.sliderColor.b,
627+ style.sliderColor.a * 0.7)
628+ var thumbPressedColor = Qt.rgba(style.sliderColor.r, style.sliderColor.g, style.sliderColor.b,
629+ style.sliderColor.a * 1.0)
630+
631+ addContentMargins(flickable)
632+
633+ setContentPositionToTopLeft(flickable)
634+
635+ triggerSteppersMode(scrollbar)
636+
637 if (style.isVertical) {
638 mouseDrag(thumb, thumb.width/2, thumb.height/2, 0, trough.height)
639 compare(flickable[scrollbarUtils.propContent],
640@@ -1072,6 +1168,22 @@
641
642 }
643
644+ //check that the mouse filters handling the interactions with the thumb and steppers
645+ //ignore the synthesized events. This is required to have the correct hover handling,
646+ //because it's the way we make sure that the hover logic is only called for real mouse
647+ //events, and not touch-to-mouse synthesized events
648+ function test_ignoreSynthesizedEventsInMouseFilters() {
649+ var scrollbar = defaultValuesScrollbar
650+ var style = scrollbar.__styleInstance
651+ var thumbArea = getThumbArea(scrollbar)
652+ var steppersMouseArea = getSteppersMouseArea(scrollbar)
653+
654+ compare(thumbArea.Mouse.ignoreSynthesizedEvents, true,
655+ "The mouse filter in the thumb MouseArea does not ignore synthesized events. Hover state logic may be affected.")
656+ compare(steppersMouseArea.Mouse.ignoreSynthesizedEvents, true,
657+ "The mouse filter in the steppers MouseArea does not ignore synthesized events. Hover state logic may be affected.")
658+ }
659+
660 function test_defaultStylingValues() {
661 var scrollbar = defaultValuesScrollbar
662 var style = scrollbar.__styleInstance

Subscribers

People subscribed via source and target branches