Unity

Merge lp:~aacid/unity/hud-result-highlighting into lp:unity/phablet

  1. hud-result-highlighting
  2. Merge into phablet
Proposed by Albert Astals Cid
Status: Merged
Approved by: Gerry Boland
Approved revision: no longer in the source branch.
Merged at revision: 667
Proposed branch: lp:~aacid/unity/hud-result-highlighting
Merge into: lp:unity/phablet
Prerequisite: lp:~aacid/unity/qpa_502
Diff against target: 206 lines (+138/-5)
4 files modified
Hud/Result.qml (+36/-4)
Hud/ResultList.qml (+3/-1)
tests/qmltests/CMakeLists.txt (+1/-0)
tests/qmltests/Hud/tst_Result.qml (+98/-0)
To merge this branch: bzr merge lp:~aacid/unity/hud-result-highlighting
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Gerry Boland (community) Approve
Daniel d'Andrada (community) Approve
Review via email: mp+163115@code.launchpad.net

This proposal supersedes a proposal from 2013-04-19.

Commit message

Hud Result highlighting

Description of the change

Hud Result highlighting

You need qtcore5 >= 5.0.2+dfsg1-3ubuntu1~raring1~test2 to properly see it working

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:665
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~aacid/unity/hud-result-highlighting/+merge/159841/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/877/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/806/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/753/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/757/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/877/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:665
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/878/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/807/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/754/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/758/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/878/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:665
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/881/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/810/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/757/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/761/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/881/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:666
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/882/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/811/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/758/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/762/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/882/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:666
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/883/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/812/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/759/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/763/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/883/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:666
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/884/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/813/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/760/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/764/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/884/rebuild

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

PASSED: Continuous integration, rev:667
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/886/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/815
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/762
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/762/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/766

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/886/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

161 + {text: "Pastel", highlights: [1, 1, 3, 4], result: "P<font color=\"#ffffff\">a</font>s<font color=\"#ffffff\">te</font>l"},
162 + {text: "Pastel", highlights: [1, 1, 3, 5], result: "P<font color=\"#ffffff\">a</font>s<font color=\"#ffffff\">tel</font>"},
163 + {text: "Pastel", highlights: [0, 0, 3, 5], result: "<font color=\"#ffffff\">P</font>as<font color=\"#ffffff\">tel</font>"},
164 + {text: "Pastel", highlights: [5, 5], result: "Paste<font color=\"#ffffff\">l</font>"},
165 + {text: "Two Words", highlights: [1, 2, 5, 5], result: "T<font color=\"#ffffff\">wo</font> W<font color=\"#ffffff\">o</font>rds"}

That's too long even to launchpad's diff viewer

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:668
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/889/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/820/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/765/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/769/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/889/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

19 + for (var i = 0; i < highlights.length; i += 2) {

Even though you have a preceding "if" checking for a even-numbered length, I think it would be safer if you had it like that instead:
    for (var i = 0; (i + 1) < highlights.length; i += 2) {
or
    for (var i = 0; i < highlights.length - 1; i += 2) {

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:670
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/892/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/823/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/768
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/768/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/772

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/892/rebuild

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

PASSED: Continuous integration, rev:670
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/893/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/824
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/769
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/769/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/773

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/893/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

in Hud/ResultList.qml:

78 + internalModel.append({"name": itemData.column_1, "highlights": itemData.column_2, "context": itemData.column_3, "contextHighlights": itemData.column_4})

That's ridiculously long.

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Otherwise looks good, even though I wasn't able to try it out (don't have that needed qt lib version)

review: Approve
Revision history for this message
Albert Astals Cid (aacid) wrote :

Qt is at https://launchpad.net/~canonical-qt5-edgers/+archive/qt5-proper

Revision history for this message
Gerry Boland (gerboland) wrote :

Functional test checks out, code looks fine too.

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
1=== modified file 'Hud/Result.qml'
2--- Hud/Result.qml 2013-02-12 11:54:44 +0000
3+++ Hud/Result.qml 2013-05-09 14:58:29 +0000
4@@ -18,8 +18,38 @@
5 import Ubuntu.Components 0.1
6
7 Item {
8- property alias contextSnippetText: contextSnippetLabel.text
9- property alias nameText: actionLabel.text
10+ property string contextSnippetText
11+ property variant contextSnippetHighlights
12+ property string nameText
13+ property variant nameHighlights
14+
15+ function highlightedText(text, highlights) {
16+ var hText = "";
17+ var nextIndexToProcess = 0;
18+ if (highlights && highlights.length % 2 == 0) {
19+ for (var i = 0; i < highlights.length - 1; i += 2) {
20+ var highlightStart = highlights[i];
21+ var highlightEnd = highlights[i + 1];
22+ if (highlightEnd < highlightStart)
23+ continue;
24+ if (highlightStart < nextIndexToProcess)
25+ continue;
26+ if (highlightStart != nextIndexToProcess) {
27+ // Prev non marked text
28+ hText += text.substr(nextIndexToProcess, highlightStart - nextIndexToProcess);
29+ }
30+
31+ // Marked text
32+ hText += "<font color=\"#ffffff\">" + text.substr(highlightStart, highlightEnd - highlightStart + 1) + "</font>";
33+ nextIndexToProcess = highlightEnd + 1;
34+ }
35+ }
36+ if (nextIndexToProcess != text.length) {
37+ // End non marked text
38+ hText += text.substr(nextIndexToProcess);
39+ }
40+ return hText;
41+ }
42
43 Label {
44 id: actionLabel
45@@ -29,9 +59,10 @@
46 anchors.verticalCenter: parent.verticalCenter
47 width: parent.width / 2
48 fontSize: "medium"
49- color: "white"
50 elide: Text.ElideRight
51 maximumLineCount: 1
52+ text: highlightedText(nameText, nameHighlights)
53+ color: "#80ffffff"
54 }
55
56 Label {
57@@ -44,9 +75,10 @@
58 anchors.rightMargin: units.gu(1)
59 anchors.verticalCenter: parent.verticalCenter
60 fontSize: "small"
61- color: "white"
62 opacity: 0.5
63 elide: Text.ElideRight
64 maximumLineCount: 2
65+ text: highlightedText(contextSnippetText, contextSnippetHighlights)
66+ color: "#80ffffff"
67 }
68 }
69
70=== modified file 'Hud/ResultList.qml'
71--- Hud/ResultList.qml 2013-04-05 11:03:11 +0000
72+++ Hud/ResultList.qml 2013-05-09 14:58:29 +0000
73@@ -49,7 +49,7 @@
74 internalModel.clear()
75 for (var i = 0; i < 5 && i < model.count; ++i) {
76 var itemData = model.get(i)
77- internalModel.append({"name": itemData.column_1, "context": itemData.column_3})
78+ internalModel.append({"name": itemData.column_1, "highlights": itemData.column_2, "context": itemData.column_3, "contextHighlights": itemData.column_4})
79 }
80 }
81
82@@ -87,7 +87,9 @@
83 anchors.right: parent.right
84
85 nameText: name
86+ nameHighlights: highlights
87 contextSnippetText: context
88+ contextSnippetHighlights: contextHighlights
89 }
90
91 BorderImage {
92
93=== modified file 'tests/qmltests/CMakeLists.txt'
94--- tests/qmltests/CMakeLists.txt 2013-05-01 19:30:22 +0000
95+++ tests/qmltests/CMakeLists.txt 2013-05-09 14:58:29 +0000
96@@ -51,6 +51,7 @@
97 add_qml_test(Dash/People PeopleFilterGrid IMPORT_PATHS ${CMAKE_BINARY_DIR}/plugins ${CMAKE_CURRENT_SOURCE_DIR}/plugins ${qmltest_DEFAULT_IMPORT_PATHS})
98 add_qml_test(Greeter Greeter)
99 add_qml_test(Hud Hud)
100+add_qml_test(Hud Result)
101 add_qml_test(Launcher Launcher)
102 add_qml_test(Panel IndicatorRow)
103 add_qml_test(Panel Indicators)
104
105=== added file 'tests/qmltests/Hud/tst_Result.qml'
106--- tests/qmltests/Hud/tst_Result.qml 1970-01-01 00:00:00 +0000
107+++ tests/qmltests/Hud/tst_Result.qml 2013-05-09 14:58:29 +0000
108@@ -0,0 +1,98 @@
109+/*
110+ * Copyright 2013 Canonical Ltd.
111+ *
112+ * This program is free software; you can redistribute it and/or modify
113+ * it under the terms of the GNU General Public License as published by
114+ * the Free Software Foundation; version 3.
115+ *
116+ * This program is distributed in the hope that it will be useful,
117+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
118+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
119+ * GNU General Public License for more details.
120+ *
121+ * You should have received a copy of the GNU General Public License
122+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
123+ */
124+
125+import QtQuick 2.0
126+import QtTest 1.0
127+import ".."
128+import "../../../Hud"
129+import Unity.Test 0.1 as UT
130+
131+Rectangle {
132+ width: 600
133+ height: 600
134+ color: "black"
135+ Column {
136+ anchors.fill: parent
137+ Repeater {
138+ model: test.test_highlightedText_data().length
139+ Result {
140+ width: parent.width
141+ height: units.gu(4)
142+ nameText: test.test_highlightedText_data()[index].text
143+ nameHighlights: test.test_highlightedText_data()[index].highlights
144+ }
145+ }
146+ }
147+
148+ UT.UnityTestCase {
149+ name: "Result"
150+ id: test
151+
152+ Result {
153+ id: result
154+ }
155+
156+ function test_highlightedText_data() {
157+ return [
158+ {
159+ text: "Copy",
160+ highlights: [],
161+ result: "Copy"
162+ },
163+ {
164+ text: "Copy",
165+ highlights: [0, 0],
166+ result: "<font color=\"#ffffff\">C</font>opy"
167+ },
168+ {
169+ text: "Copy",
170+ highlights: [1, 1],
171+ result: "C<font color=\"#ffffff\">o</font>py"
172+ },
173+ {
174+ text: "Pastel",
175+ highlights: [1, 1, 3, 4],
176+ result: "P<font color=\"#ffffff\">a</font>s<font color=\"#ffffff\">te</font>l"
177+ },
178+ {
179+ text: "Pastel",
180+ highlights: [1, 1, 3, 5],
181+ result: "P<font color=\"#ffffff\">a</font>s<font color=\"#ffffff\">tel</font>"
182+ },
183+ {
184+ text: "Pastel",
185+ highlights: [0, 0, 3, 5],
186+ result: "<font color=\"#ffffff\">P</font>as<font color=\"#ffffff\">tel</font>"
187+ },
188+ {
189+ text: "Pastel",
190+ highlights: [5, 5],
191+ result: "Paste<font color=\"#ffffff\">l</font>"
192+ },
193+ {
194+ text: "Two Words",
195+ highlights: [1, 2, 5, 5],
196+ result: "T<font color=\"#ffffff\">wo</font> W<font color=\"#ffffff\">o</font>rds"
197+ }
198+ ]
199+ }
200+
201+ function test_highlightedText(data) {
202+ var hText = result.highlightedText(data.text, data.highlights);
203+ compare(hText, data.result);
204+ }
205+ }
206+}

Subscribers

People subscribed via source and target branches