Merge lp:~aacid/unity8/quicklist_selection_tuning into lp:unity8
- quicklist_selection_tuning
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | MichaĆ Sawicz |
Approved revision: | 2871 |
Merged at revision: | 2893 |
Proposed branch: | lp:~aacid/unity8/quicklist_selection_tuning |
Merge into: | lp:unity8 |
Diff against target: |
130 lines (+56/-12) 2 files modified
qml/Launcher/LauncherPanel.qml (+24/-6) tests/qmltests/Launcher/tst_Launcher.qml (+32/-6) |
To merge this branch: | bzr merge lp:~aacid/unity8/quicklist_selection_tuning |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot | continuous-integration | Approve | |
Michael Zanetti (community) | Approve | ||
Review via email: mp+320226@code.launchpad.net |
Commit message
Tune quicklist item selection
Mouse on hover selects items
Keyboard selection skips non-clickable items
Description of the change
* Are there any related MPs required for this MP to build/function as expected?
No
* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
* If you changed the UI, has there been a design review?
N/A
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
Michael Zanetti (mzanetti) wrote : | # |
works mostly good. one thing:
if there is a disabled entry, and you hover through the list, once you are on the disabled one, the previously one will stay highlighted while there should be no highlight at that point
Albert Astals Cid (aacid) wrote : | # |
> works mostly good. one thing:
>
> if there is a disabled entry, and you hover through the list, once you are on
> the disabled one, the previously one will stay highlighted while there should
> be no highlight at that point
Agreed, fixed.
Michael Zanetti (mzanetti) wrote : | # |
code looks good. tested it, works fine. waiting on CI to test the latest commit for top approval
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2870
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2871
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2871
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2871
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2871
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2871
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'qml/Launcher/LauncherPanel.qml' |
2 | --- qml/Launcher/LauncherPanel.qml 2017-03-16 10:46:15 +0000 |
3 | +++ qml/Launcher/LauncherPanel.qml 2017-03-24 08:28:31 +0000 |
4 | @@ -751,16 +751,18 @@ |
5 | Keys.onPressed: { |
6 | switch (event.key) { |
7 | case Qt.Key_Down: |
8 | - selectedIndex++; |
9 | - if (selectedIndex >= popoverRepeater.count) { |
10 | - selectedIndex = 0; |
11 | + var prevIndex = selectedIndex; |
12 | + selectedIndex = (selectedIndex + 1 < popoverRepeater.count) ? selectedIndex + 1 : 0; |
13 | + while (!popoverRepeater.itemAt(selectedIndex).clickable && selectedIndex != prevIndex) { |
14 | + selectedIndex = (selectedIndex + 1 < popoverRepeater.count) ? selectedIndex + 1 : 0; |
15 | } |
16 | event.accepted = true; |
17 | break; |
18 | case Qt.Key_Up: |
19 | - selectedIndex--; |
20 | - if (selectedIndex < 0) { |
21 | - selectedIndex = popoverRepeater.count - 1; |
22 | + var prevIndex = selectedIndex; |
23 | + selectedIndex = (selectedIndex > 0) ? selectedIndex - 1 : popoverRepeater.count - 1; |
24 | + while (!popoverRepeater.itemAt(selectedIndex).clickable && selectedIndex != prevIndex) { |
25 | + selectedIndex = (selectedIndex > 0) ? selectedIndex - 1 : popoverRepeater.count - 1; |
26 | } |
27 | event.accepted = true; |
28 | break; |
29 | @@ -806,6 +808,19 @@ |
30 | width: parent.width |
31 | height: quickListColumn.height |
32 | |
33 | + MouseArea { |
34 | + anchors.fill: parent |
35 | + hoverEnabled: true |
36 | + onPositionChanged: { |
37 | + var item = quickListColumn.childAt(mouseX, mouseY); |
38 | + if (item.clickable) { |
39 | + quickList.selectedIndex = item.index; |
40 | + } else { |
41 | + quickList.selectedIndex = -1; |
42 | + } |
43 | + } |
44 | + } |
45 | + |
46 | Column { |
47 | id: quickListColumn |
48 | width: parent.width |
49 | @@ -820,6 +835,9 @@ |
50 | } |
51 | |
52 | ListItem { |
53 | + readonly property bool clickable: model.clickable |
54 | + readonly property int index: model.index |
55 | + |
56 | objectName: "quickListEntry" + index |
57 | selected: index === quickList.selectedIndex |
58 | height: label.implicitHeight + label.anchors.topMargin + label.anchors.bottomMargin |
59 | |
60 | === modified file 'tests/qmltests/Launcher/tst_Launcher.qml' |
61 | --- tests/qmltests/Launcher/tst_Launcher.qml 2017-03-14 10:38:52 +0000 |
62 | +++ tests/qmltests/Launcher/tst_Launcher.qml 2017-03-24 08:28:31 +0000 |
63 | @@ -1254,14 +1254,14 @@ |
64 | tryCompare(quickList, "selectedIndex", 0) |
65 | |
66 | // Down should move down the quicklist |
67 | + // Because item 1 is not selectable |
68 | keyClick(Qt.Key_Down); |
69 | - tryCompare(quickList, "selectedIndex", 1) |
70 | + tryCompare(quickList, "selectedIndex", 2) |
71 | |
72 | // The quicklist should wrap around too |
73 | keyClick(Qt.Key_Down); |
74 | keyClick(Qt.Key_Down); |
75 | keyClick(Qt.Key_Down); |
76 | - keyClick(Qt.Key_Down); |
77 | tryCompare(quickList, "selectedIndex", 0) |
78 | |
79 | // Left gets us back to the launcher |
80 | @@ -1286,14 +1286,14 @@ |
81 | |
82 | keyClick(Qt.Key_Down); // Down to launcher item 0 |
83 | keyClick(Qt.Key_Down); // Down to launcher item 1 |
84 | - keyClick(Qt.Key_Right); // Into quicklist |
85 | - keyClick(Qt.Key_Down); // Down to quicklist item 1 |
86 | - keyClick(Qt.Key_Down); // Down to quicklist item 2 |
87 | + keyClick(Qt.Key_Right); // Into quicklist, item 0 is selected |
88 | + keyClick(Qt.Key_Down); // Down to quicklist item 2 (because 1 is not selectable) |
89 | + keyClick(Qt.Key_Down); // Down to quicklist item 3 |
90 | keyClick(Qt.Key_Enter); // Trigger it |
91 | |
92 | compare(signalSpy.count, 1, "Quicklist signal wasn't triggered") |
93 | compare(signalSpy.signalArguments[0][0], LauncherModel.get(1).appId) |
94 | - compare(signalSpy.signalArguments[0][1], 2) |
95 | + compare(signalSpy.signalArguments[0][1], 3) |
96 | assertFocusOnIndex(-2); |
97 | } |
98 | |
99 | @@ -1542,5 +1542,31 @@ |
100 | tryCompare(launcher, "maxPanelX", 0); |
101 | launcher.panelWidth = oldSize; |
102 | } |
103 | + |
104 | + function test_mouseHoverSelectQuickList() { |
105 | + dragLauncherIntoView(); |
106 | + var clickedItem = findChild(launcher, "launcherDelegate5") |
107 | + var quickList = findChild(launcher, "quickList") |
108 | + |
109 | + // Initial state |
110 | + tryCompare(quickList, "state", "") |
111 | + |
112 | + // Open quickList |
113 | + mouseClick(clickedItem, clickedItem.width / 2, clickedItem.height / 2, Qt.RightButton) |
114 | + verify(quickList, "state", "open") |
115 | + compare(quickList.selectedIndex, -1) |
116 | + |
117 | + var qEntry = findChild(launcher, "quickListEntry0"); |
118 | + mouseMove(qEntry, qEntry.width / 2 , qEntry.height / 2, 10); |
119 | + mouseMove(qEntry, qEntry.width / 2 + 1, qEntry.height / 2, 10); |
120 | + |
121 | + tryCompare(quickList, "selectedIndex", 0) |
122 | + |
123 | + qEntry = findChild(launcher, "quickListEntry1"); |
124 | + mouseMove(qEntry, qEntry.width / 2 , qEntry.height / 2, 10); |
125 | + mouseMove(qEntry, qEntry.width / 2 + 1, qEntry.height / 2, 10); |
126 | + |
127 | + tryCompare(quickList, "selectedIndex", -1) |
128 | + } |
129 | } |
130 | } |
PASSED: Continuous integration, rev:2869 /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3450/ /unity8- jenkins. ubuntu. com/job/ build/4544 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= xenial+ overlay, testname= qmluitests. sh/2736 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= zesty,testname= qmluitests. sh/2736 /unity8- jenkins. ubuntu. com/job/ build-0- fetch/4572 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4399 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4399/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4399 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4399/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4399 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4399/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4399 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4399/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4399 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4399/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4399 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4399/ artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3450/ rebuild
https:/