Merge lp:~tiagosh/unity-2d/unity-2d-shell-homelens into lp:unity-2d
- unity-2d-shell-homelens
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Albert Astals Cid |
Approved revision: | 952 |
Merged at revision: | 910 |
Proposed branch: | lp:~tiagosh/unity-2d/unity-2d-shell-homelens |
Merge into: | lp:unity-2d |
Diff against target: |
341 lines (+92/-64) 9 files modified
libunity-2d-private/src/lens.cpp (+15/-3) libunity-2d-private/src/lenses.cpp (+8/-5) libunity-2d-private/src/lenses.h (+2/-0) shell/app/shelldeclarativeview.cpp (+7/-0) shell/app/shelldeclarativeview.h (+2/-0) shell/dash/Dash.qml (+25/-17) shell/dash/LensBar.qml (+20/-19) shell/dash/LensView.qml (+7/-6) shell/dash/RendererGrid.qml (+6/-14) |
To merge this branch: | bzr merge lp:~tiagosh/unity-2d/unity-2d-shell-homelens |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Albert Astals Cid (community) | Approve | ||
Gerry Boland | Pending | ||
Michał Sawicz | Pending | ||
Review via email: mp+92571@code.launchpad.net |
This proposal supersedes a proposal from 2012-01-31.
Commit message
Description of the change
This MR replaces the old home lens by the new one provided by unity.
The following libqtdee branch must be installed in order to compile this branch: lp:~tiagosh/dee-qt/dee-qt-local-models .
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal | # |
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal | # |
Hey, this also looks like it could go into lp:unity-2d almost without modification. As it's a goal to have as small a diff between trunk and shell when we get to merging it, could you please have a MR against lp:unity-2d for this?
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal | # |
As discussed, let's wait with this until shell gets merged into trunk.
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal | # |
Hey,
this is great work, thank you Tiago. It requires some fixes though:
- first, your patch to dee-qt may need fixing. I'm blaming it for these problems:
- on first run, all the icons were ? icons with no text.
- bug I can reproduce:
1. I hope dash. I search for something, I get correct results
2. if I empty search box, everything is still good
3. I switch to applications lens, everything good
4. I switch back to home lens, I get many ? icons everywhere with no text (just like first run)
But the bug may be here. I've not tried to dig yet.
Here's screengrab to show you: https:/
- possibly as a consequence, I get these unity-2d errors when viewing Home lens
[WARNING] shell/dash/
[WARNING] shell/dash/
[WARNING] shell/dash/
[WARNING] shell/dash/
[WARNING] shell/dash/
<repeated many times>
[WARNING] shell/dash/
<repeated many times>
But your code looks clean, nothing obvious appears wrong to me.
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal | # |
Doesn't merge cleanly with lp:~unity-2d-team/unity-2d/unity-2d-shell Can you remerge it?
Albert Astals Cid (aacid) : Posted in a previous version of this proposal | # |
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal | # |
With fun i mean "Functional" :/
I still have to look at the code, there is also
unity-2d-shell: [CRITICAL] dee: dee_model_
unity-2d-shell: [CRITICAL] dee: dee_model_
unity-2d-shell: [CRITICAL] dee: dee_model_
unity-2d-shell: [CRITICAL] dee: dee_model_
unity-2d-shell: [CRITICAL] dee: dee_model_
unity-2d-shell: [CRITICAL] dee: dee_model_
unity-2d-shell: [CRITICAL] dee: dee_model_
unity-2d-shell: [CRITICAL] dee: dee_model_
That thiago says that can't remove but are somehow scary for my pov
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal | # |
And with Thiago i mean Tiago
Florian Boucault (fboucault) wrote : Posted in a previous version of this proposal | # |
Please resubmit the MR against lp:unity-2d
Albert Astals Cid (aacid) wrote : | # |
Had a more thorough look at those warnings and they really seem to come from unity libraries themselves without much interaction in our side.
So Approve it is
Preview Diff
1 | === modified file 'libunity-2d-private/src/lens.cpp' |
2 | --- libunity-2d-private/src/lens.cpp 2011-12-14 22:45:04 +0000 |
3 | +++ libunity-2d-private/src/lens.cpp 2012-02-10 20:42:23 +0000 |
4 | @@ -224,9 +224,21 @@ |
5 | |
6 | m_filters = new Filters(m_unityLens->filters, this); |
7 | |
8 | - m_results->setName(QString::fromStdString(m_unityLens->results()->swarm_name)); |
9 | - m_globalResults->setName(QString::fromStdString(m_unityLens->global_results()->swarm_name)); |
10 | - m_categories->setName(QString::fromStdString(m_unityLens->categories()->swarm_name)); |
11 | + if (QString::fromStdString(m_unityLens->results()->swarm_name) == QString(":local")) { |
12 | + m_results->setModel(m_unityLens->results()->model()); |
13 | + } else { |
14 | + m_results->setName(QString::fromStdString(m_unityLens->results()->swarm_name)); |
15 | + } |
16 | + if (QString::fromStdString(m_unityLens->global_results()->swarm_name) == QString(":local")) { |
17 | + m_globalResults->setModel(m_unityLens->global_results()->model()); |
18 | + } else { |
19 | + m_globalResults->setName(QString::fromStdString(m_unityLens->global_results()->swarm_name)); |
20 | + } |
21 | + if (QString::fromStdString(m_unityLens->categories()->swarm_name) == QString(":local")) { |
22 | + m_categories->setModel(m_unityLens->categories()->model()); |
23 | + } else { |
24 | + m_categories->setName(QString::fromStdString(m_unityLens->categories()->swarm_name)); |
25 | + } |
26 | |
27 | /* Property change signals */ |
28 | m_unityLens->id.changed.connect(sigc::mem_fun(this, &Lens::idChanged)); |
29 | |
30 | === modified file 'libunity-2d-private/src/lenses.cpp' |
31 | --- libunity-2d-private/src/lenses.cpp 2011-09-09 10:15:14 +0000 |
32 | +++ libunity-2d-private/src/lenses.cpp 2012-02-10 20:42:23 +0000 |
33 | @@ -27,6 +27,8 @@ |
34 | |
35 | // libunity-core |
36 | #include <UnityCore/FilesystemLenses.h> |
37 | +#include <UnityCore/HomeLens.h> |
38 | +#include <unity2dtr.h> |
39 | |
40 | Lenses::Lenses(QObject *parent) : |
41 | QAbstractListModel(parent) |
42 | @@ -36,17 +38,18 @@ |
43 | roles[Lenses::RoleVisible] = "visible"; |
44 | setRoleNames(roles); |
45 | |
46 | + m_homeLens = new unity::dash::HomeLens(u2dTr("Home").toStdString(), u2dTr("Home screen").toStdString(), u2dTr("Search").toStdString()); |
47 | m_unityLenses = new unity::dash::FilesystemLenses("/usr/share/unity/lenses"); |
48 | - for (unsigned int i=0; i<m_unityLenses->count(); i++) { |
49 | - unity::dash::Lens::Ptr unityLens = m_unityLenses->GetLensAtIndex(i); |
50 | - addUnityLens(unityLens, i); |
51 | - } |
52 | - m_unityLenses->lens_added.connect(sigc::mem_fun(this, &Lenses::onLensAdded)); |
53 | + m_homeLens->AddLenses(*m_unityLenses); |
54 | + m_homeLens->lens_added.connect(sigc::mem_fun(this, &Lenses::onLensAdded)); |
55 | + unity::dash::HomeLens::Ptr homeLensPtr(m_homeLens); |
56 | + addUnityLens(homeLensPtr, 0); |
57 | } |
58 | |
59 | Lenses::~Lenses() |
60 | { |
61 | delete m_unityLenses; |
62 | + delete m_homeLens; |
63 | } |
64 | |
65 | int Lenses::rowCount(const QModelIndex& parent) const |
66 | |
67 | === modified file 'libunity-2d-private/src/lenses.h' |
68 | --- libunity-2d-private/src/lenses.h 2011-08-11 14:38:51 +0000 |
69 | +++ libunity-2d-private/src/lenses.h 2012-02-10 20:42:23 +0000 |
70 | @@ -26,6 +26,7 @@ |
71 | |
72 | // libunity-core |
73 | #include <UnityCore/Lens.h> |
74 | +#include <UnityCore/HomeLens.h> |
75 | |
76 | namespace unity |
77 | { |
78 | @@ -64,6 +65,7 @@ |
79 | |
80 | private: |
81 | unity::dash::Lenses* m_unityLenses; |
82 | + unity::dash::HomeLens* m_homeLens; |
83 | QList<Lens*> m_lenses; |
84 | |
85 | void addUnityLens(unity::dash::Lens::Ptr unity_lens, int index); |
86 | |
87 | === modified file 'shell/app/shelldeclarativeview.cpp' |
88 | --- shell/app/shelldeclarativeview.cpp 2012-02-09 12:27:35 +0000 |
89 | +++ shell/app/shelldeclarativeview.cpp 2012-02-10 20:42:23 +0000 |
90 | @@ -41,6 +41,7 @@ |
91 | #include <QtDBus/QDBusInterface> |
92 | #include <QX11Info> |
93 | #include <QGraphicsObject> |
94 | +#include <QFileInfo> |
95 | |
96 | // X11 |
97 | #include <X11/Xlib.h> |
98 | @@ -188,6 +189,12 @@ |
99 | return m_active; |
100 | } |
101 | |
102 | +bool |
103 | +ShellDeclarativeView::haveCustomHomeShortcuts() const |
104 | +{ |
105 | + return QFileInfo(unity2dDirectory() + "/shell/dash/HomeShortcutsCustomized.qml").exists(); |
106 | +} |
107 | + |
108 | void |
109 | ShellDeclarativeView::setDashMode(ShellDeclarativeView::DashMode mode) |
110 | { |
111 | |
112 | === modified file 'shell/app/shelldeclarativeview.h' |
113 | --- shell/app/shelldeclarativeview.h 2012-02-09 02:22:19 +0000 |
114 | +++ shell/app/shelldeclarativeview.h 2012-02-10 20:42:23 +0000 |
115 | @@ -39,6 +39,7 @@ |
116 | Q_PROPERTY(QString activeLens READ activeLens WRITE setActiveLens NOTIFY activeLensChanged) |
117 | Q_PROPERTY(bool focus READ hasFocus NOTIFY focusChanged) // overridden to add notify |
118 | Q_PROPERTY(bool superKeyHeld READ superKeyHeld NOTIFY superKeyHeldChanged) |
119 | + Q_PROPERTY(bool haveCustomHomeShortcuts READ haveCustomHomeShortcuts) |
120 | |
121 | /* These two properties and mouse movement tracking on the widget are added here only because |
122 | we need to detect when the mouse is inside the area occupied by the lancher. This should |
123 | @@ -59,6 +60,7 @@ |
124 | |
125 | /* getters */ |
126 | bool dashActive() const; |
127 | + bool haveCustomHomeShortcuts() const; |
128 | DashMode dashMode() const; |
129 | const QString& activeLens() const; |
130 | bool expanded() const; |
131 | |
132 | === modified file 'shell/dash/Dash.qml' |
133 | --- shell/dash/Dash.qml 2012-02-09 11:22:43 +0000 |
134 | +++ shell/dash/Dash.qml 2012-02-10 20:42:23 +0000 |
135 | @@ -106,6 +106,7 @@ |
136 | for (var i=0; i<lenses.rowCount(); i++) { |
137 | lenses.get(i).viewType = Lens.Hidden |
138 | } |
139 | + declarativeView.activeLens = "" |
140 | } |
141 | |
142 | SpreadMonitor { |
143 | @@ -133,11 +134,19 @@ |
144 | return |
145 | } |
146 | |
147 | - /* To activate lens, we set its viewType to LensView, and then set all |
148 | - other lenses to Hidden */ |
149 | for (var i=0; i<lenses.rowCount(); i++) { |
150 | var thislens = lenses.get(i) |
151 | - thislens.viewType = (lens == thislens) ? Lens.LensView : Lens.Hidden |
152 | + if (lensId == "home.lens") { |
153 | + if (thislens.id == lensId) { |
154 | + thislens.viewType = Lens.LensView |
155 | + continue |
156 | + } |
157 | + /* When Home is shown, need to notify all other lenses. Those in the global view |
158 | + (in home search results page) are set to HomeView, all others to Hidden */ |
159 | + thislens.viewType = (thislens.searchInGlobal) ? Lens.HomeView : Lens.Hidden |
160 | + } else { |
161 | + thislens.viewType = (lens == thislens) ? Lens.LensView : Lens.Hidden |
162 | + } |
163 | } |
164 | |
165 | buildLensPage(lens) |
166 | @@ -147,20 +156,19 @@ |
167 | |
168 | function activateHome() { |
169 | if (spreadMonitor.shown) return |
170 | - |
171 | - /* When Home is shown, need to notify all other lenses. Those in the global view |
172 | - (in home search results page) are set to HomeView, all others to Hidden */ |
173 | - for (var i=0; i<lenses.rowCount(); i++) { |
174 | - var thislens = lenses.get(i) |
175 | - thislens.viewType = (thislens.searchInGlobal) ? Lens.HomeView : Lens.Hidden |
176 | + if (declarativeView.haveCustomHomeShortcuts) { |
177 | + for (var i=0; i<lenses.rowCount(); i++) { |
178 | + lenses.get(i).viewType = Lens.Hidden |
179 | + } |
180 | + pageLoader.setSource("Home.qml") |
181 | + /* Take advantage of the fact that the loaded qml is local and setting |
182 | + the source loads it immediately making pageLoader.item valid */ |
183 | + activatePage(pageLoader.item) |
184 | + declarativeView.activeLens = "" |
185 | + dash.active = true |
186 | + } else { |
187 | + activateLens("home.lens") |
188 | } |
189 | - |
190 | - pageLoader.setSource("Home.qml") |
191 | - /* Take advantage of the fact that the loaded qml is local and setting |
192 | - the source loads it immediately making pageLoader.item valid */ |
193 | - activatePage(pageLoader.item) |
194 | - declarativeView.activeLens = "" |
195 | - dash.active = true |
196 | } |
197 | |
198 | function activateLensWithOptionFilter(lensId, filterId, optionId) { |
199 | @@ -304,7 +312,7 @@ |
200 | KeyNavigation.left: search_entry |
201 | |
202 | /* FilterPane is only to be displayed for lenses, not in the home page or Alt+F2 Run page */ |
203 | - visible: declarativeView.activeLens != "" && declarativeView.activeLens != "commands.lens" |
204 | + visible: declarativeView.activeLens != "home.lens" && declarativeView.activeLens != "" && declarativeView.activeLens != "commands.lens" |
205 | lens: visible && currentPage != undefined ? currentPage.model : undefined |
206 | |
207 | anchors.top: search_entry.anchors.top |
208 | |
209 | === modified file 'shell/dash/LensBar.qml' |
210 | --- shell/dash/LensBar.qml 2012-01-10 13:07:48 +0000 |
211 | +++ shell/dash/LensBar.qml 2012-02-10 20:42:23 +0000 |
212 | @@ -93,22 +93,6 @@ |
213 | return undefined |
214 | } |
215 | |
216 | - /* Need to manually include the Home lens */ |
217 | - LensButton { |
218 | - id: homeLens |
219 | - |
220 | - Accessible.name: u2d.tr("home") |
221 | - |
222 | - focus: true |
223 | - icon: "artwork/lens-nav-home.svg" |
224 | - onClicked: dash.activateHome() |
225 | - active: ( declarativeView.activeLens == "" ) |
226 | - iconWidth: lensBar.iconWidth |
227 | - iconSpacing: lensBar.iconSpacing |
228 | - width: iconWidth+iconSpacing |
229 | - height: lensContainer.height |
230 | - } |
231 | - |
232 | /* Now fetch all other lenses and display */ |
233 | Repeater{ |
234 | id: repeater |
235 | @@ -118,9 +102,26 @@ |
236 | Accessible.name: u2d.tr(item.name) |
237 | |
238 | /* Heuristic: if iconHint does not contain a '/' then it is an icon name */ |
239 | - icon: item.iconHint.indexOf("/") == -1 ? "image://icons/" + item.iconHint : item.iconHint |
240 | - active: item.viewType == Lens.LensView |
241 | - onClicked: dash.activateLens(item.id) |
242 | + icon: { |
243 | + if (item.id == "home.lens") { |
244 | + return "artwork/lens-nav-home.svg" |
245 | + } |
246 | + item.iconHint.indexOf("/") == -1 ? "image://icons/" + item.iconHint : item.iconHint |
247 | + } |
248 | + active: { |
249 | + /* we need this in order to activate the arrow when using a custom shortcuts file */ |
250 | + if (item.id == "home.lens" && declarativeView.activeLens == "") { |
251 | + return true |
252 | + } |
253 | + return item.viewType == Lens.LensView |
254 | + } |
255 | + onClicked: { |
256 | + if (item.id == "home.lens") { |
257 | + dash.activateHome() |
258 | + } else { |
259 | + dash.activateLens(item.id) |
260 | + } |
261 | + } |
262 | iconWidth: lensBar.iconWidth |
263 | iconSpacing: lensBar.iconSpacing |
264 | width: iconWidth+iconSpacing |
265 | |
266 | === modified file 'shell/dash/LensView.qml' |
267 | --- shell/dash/LensView.qml 2012-01-10 10:42:46 +0000 |
268 | +++ shell/dash/LensView.qml 2012-02-10 20:42:23 +0000 |
269 | @@ -81,7 +81,7 @@ |
270 | bodyDelegate: Loader { |
271 | visible: category_model.count > 0 |
272 | width: parent.width |
273 | - height: visible ? item.contentHeight : 0 |
274 | + height: item ? visible ? item.contentHeight : 0 : 0 |
275 | |
276 | property string name: model.column_0 |
277 | property string iconHint: model.column_1 |
278 | @@ -119,18 +119,19 @@ |
279 | Binding { target: item; property: "categoryId"; value: categoryId } |
280 | Binding { target: item; property: "category_model"; value: category_model } |
281 | Binding { target: item; property: "lens"; value: lensView.model } |
282 | + Binding { target: item; property: "lensId"; value: lensView.model.id } |
283 | |
284 | onLoaded: item.focus = true |
285 | } |
286 | |
287 | headerDelegate: CategoryHeader { |
288 | - visible: body.item.needHeader && body.visible |
289 | + visible: body.item ? body.item.needHeader && body.visible : false |
290 | height: visible ? 32 : 0 |
291 | |
292 | - property bool foldable: body.item.folded != undefined |
293 | - availableCount: foldable ? body.category_model.count - body.item.cellsPerRow : 0 |
294 | - folded: foldable ? body.item.folded : false |
295 | - onClicked: if(foldable) body.item.folded = !body.item.folded |
296 | + property bool foldable: body.item ? body.item.folded != undefined : false |
297 | + availableCount: body.item ? foldable ? body.category_model.count - body.item.cellsPerRow : 0 : 0 |
298 | + folded: body.item ? foldable ? body.item.folded : false : false |
299 | + onClicked: if(foldable && body.item) body.item.folded = !body.item.folded |
300 | moving: flickerMoving |
301 | |
302 | icon: body.iconHint |
303 | |
304 | === modified file 'shell/dash/RendererGrid.qml' |
305 | --- shell/dash/RendererGrid.qml 2011-11-23 10:27:05 +0000 |
306 | +++ shell/dash/RendererGrid.qml 2012-02-10 20:42:23 +0000 |
307 | @@ -77,14 +77,6 @@ |
308 | |
309 | FocusPath.index: index |
310 | |
311 | - property string uri: column_0 |
312 | - property string iconHint: column_1 |
313 | - property string categoryId: column_2 // FIXME: rename to categoryIndex |
314 | - property string mimetype: column_3 |
315 | - property string displayName: column_4 // FIXME: rename to name |
316 | - property string comment: column_5 |
317 | - property string dndUri: column_6 |
318 | - |
319 | Loader { |
320 | id: loader |
321 | |
322 | @@ -96,13 +88,13 @@ |
323 | |
324 | sourceComponent: cellRenderer |
325 | onLoaded: { |
326 | - item.uri = uri |
327 | - item.iconHint = iconHint |
328 | - item.mimetype = mimetype |
329 | - item.displayName = displayName |
330 | - item.comment = comment |
331 | + item.uri = column_0 |
332 | + item.iconHint = column_1 |
333 | + item.mimetype = column_3 |
334 | + item.displayName = column_4 |
335 | + item.comment = column_5 |
336 | item.focus = true |
337 | - item.dndUri = dndUri |
338 | + item.dndUri = column_6 |
339 | } |
340 | } |
341 | } |
Hi Tiago,
great work, thanks for this! But we need tests to be able to accept this.
I think testing the following will be sufficient:
1. open dash, check somehow that this Home lens is showing
2. open dash, type a character, check search results appear in Home lens
Consult the existing tests to get an idea where to start. Getting started guide here: /wiki.ubuntu. com/Unity2DTest ability /bazaar. launchpad. net/~gerboland/ unity-2d/ dash-test/ view/head: /tests/ places/ show_hide_ tests.rb
https:/
and this old test of mine might help you get started. Not it's for unity-2d, so things have changed a little
https:/
Ping me if you need a hand
-G