Merge lp:~nick-dedekind/unity8/remove-indicator-visibility-filter into lp:unity8

Proposed by Nick Dedekind on 2015-01-20
Status: Merged
Approved by: Daniel d'Andrada on 2015-01-21
Approved revision: 1562
Merged at revision: 1573
Proposed branch: lp:~nick-dedekind/unity8/remove-indicator-visibility-filter
Merge into: lp:unity8
Diff against target: 403 lines (+8/-255)
12 files modified
plugins/Unity/Indicators/CMakeLists.txt (+0/-1)
plugins/Unity/Indicators/indicators.h (+1/-2)
plugins/Unity/Indicators/plugin.cpp (+0/-2)
plugins/Unity/Indicators/visibleindicatorsmodel.cpp (+0/-90)
plugins/Unity/Indicators/visibleindicatorsmodel.h (+0/-56)
qml/Panel/Indicators/VisibleIndicators.qml (+0/-84)
qml/Shell.qml (+4/-8)
run.sh (+1/-1)
tests/mocks/Unity/Indicators/CMakeLists.txt (+0/-1)
tests/mocks/Unity/Indicators/fakeplugin.cpp (+0/-2)
tests/qmltests/Panel/IndicatorTest.qml (+1/-7)
tests/qmltests/Panel/tst_MenuContent.qml (+1/-1)
To merge this branch: bzr merge lp:~nick-dedekind/unity8/remove-indicator-visibility-filter
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) 2015-01-20 Approve on 2015-01-21
PS Jenkins bot continuous-integration Needs Fixing on 2015-01-21
Review via email: mp+247039@code.launchpad.net

Commit Message

Removed filtering the indicator model by visibility.

Description of the Change

No longer need to filter the indicator model by visibility.

 * Are there any related MPs required for this MP to build/function as expected? Please list.
No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

 * Did you make sure that your branch does not contain spurious tags?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
No

 * If you changed the UI, has there been a design review?
N/A

To post a comment you must log in.
Daniel d'Andrada (dandrader) wrote :

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes. All fine.

* Did CI run pass? If not, please explain why.
No results yet.

* Did you make sure that the branch does not contain spurious tags?
Yes.

review: Approve
Daniel d'Andrada (dandrader) wrote :

"make testMenuContent" is failing.

"""
FAIL! : qmltestrunner::MenuContentTest::test_show_menu() Uncaught exception: Cannot call method 'data' of undefined
   Loc: [/home/dandrader/unity8/remove-indicator-visibility-filter/tests/qmltests/Panel/tst_MenuContent.qml(114)]
"""

review: Needs Fixing
1562. By Nick Dedekind on 2015-01-21

fixed MenuContent test

Nick Dedekind (nick-dedekind) wrote :

> "make testMenuContent" is failing.
>
> """
> FAIL! : qmltestrunner::MenuContentTest::test_show_menu() Uncaught exception:
> Cannot call method 'data' of undefined
> Loc: [/home/dandrader/unity8/remove-indicator-visibility-
> filter/tests/qmltests/Panel/tst_MenuContent.qml(114)]
> """

Thanks. Fixed.

Daniel d'Andrada (dandrader) wrote :

Good to go.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Unity/Indicators/CMakeLists.txt'
2--- plugins/Unity/Indicators/CMakeLists.txt 2014-11-11 15:28:13 +0000
3+++ plugins/Unity/Indicators/CMakeLists.txt 2015-01-21 15:47:40 +0000
4@@ -29,7 +29,6 @@
5 sharedunitymenumodel.cpp
6 unitymenumodelcache.cpp
7 unitymenumodelstack.cpp
8- visibleindicatorsmodel.cpp
9 )
10 add_definitions(-DUNITYINDICATORS_LIBRARY)
11
12
13=== modified file 'plugins/Unity/Indicators/indicators.h'
14--- plugins/Unity/Indicators/indicators.h 2014-10-03 10:17:29 +0000
15+++ plugins/Unity/Indicators/indicators.h 2015-01-21 15:47:40 +0000
16@@ -74,8 +74,7 @@
17 enum Roles {
18 Identifier = 0,
19 Position,
20- IndicatorProperties,
21- IsVisible
22+ IndicatorProperties
23 };
24
25 IndicatorsModelRole(QObject*parent=0):QObject(parent) {}
26
27=== modified file 'plugins/Unity/Indicators/plugin.cpp'
28--- plugins/Unity/Indicators/plugin.cpp 2014-11-11 15:28:13 +0000
29+++ plugins/Unity/Indicators/plugin.cpp 2015-01-21 15:47:40 +0000
30@@ -34,7 +34,6 @@
31 #include "sharedunitymenumodel.h"
32 #include "unitymenumodelcache.h"
33 #include "unitymenumodelstack.h"
34-#include "visibleindicatorsmodel.h"
35
36 #include <unitymenumodel.h>
37
38@@ -55,7 +54,6 @@
39 qmlRegisterType<ModelActionRootState>(uri, 0, 1, "ModelActionRootState");
40 qmlRegisterType<ActionRootState>(uri, 0, 1, "ActionRootState");
41 qmlRegisterType<ModelPrinter>(uri, 0, 1, "ModelPrinter");
42- qmlRegisterType<VisibleIndicatorsModel>(uri, 0, 1, "VisibleIndicatorsModel");
43 qmlRegisterType<SharedUnityMenuModel>(uri, 0, 1, "SharedUnityMenuModel");
44
45 qmlRegisterSingletonType<UnityMenuModelCache>(uri, 0, 1, "UnityMenuModelCache", menuModelCacheSingleton);
46
47=== removed file 'plugins/Unity/Indicators/visibleindicatorsmodel.cpp'
48--- plugins/Unity/Indicators/visibleindicatorsmodel.cpp 2014-10-03 10:17:29 +0000
49+++ plugins/Unity/Indicators/visibleindicatorsmodel.cpp 1970-01-01 00:00:00 +0000
50@@ -1,90 +0,0 @@
51-/*
52- * Copyright (C) 2012, 2013 Canonical, Ltd.
53- *
54- * This program is free software; you can redistribute it and/or modify
55- * it under the terms of the GNU General Public License as published by
56- * the Free Software Foundation; version 3.
57- *
58- * This program is distributed in the hope that it will be useful,
59- * but WITHOUT ANY WARRANTY; without even the implied warranty of
60- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
61- * GNU General Public License for more details.
62- *
63- * You should have received a copy of the GNU General Public License
64- * along with this program. If not, see <http://www.gnu.org/licenses/>.
65- */
66-
67-// self
68-#include "visibleindicatorsmodel.h"
69-#include "indicators.h"
70-
71-VisibleIndicatorsModel::VisibleIndicatorsModel(QObject *parent)
72- : QIdentityProxyModel(parent),
73- m_inserting(false)
74-{
75- QObject::connect(this, SIGNAL(rowsAboutToBeInserted(QModelIndex, int, int)), this, SLOT(onBeginRowInserted(QModelIndex, int, int)));
76- QObject::connect(this, SIGNAL(rowsInserted(QModelIndex, int, int)), this, SLOT(onRowInserted(QModelIndex, int, int)));
77-}
78-
79-QHash<int, QByteArray> VisibleIndicatorsModel::roleNames() const
80-{
81- static QHash<int, QByteArray> roles;
82- if (roles.isEmpty())
83- {
84- roles[IndicatorsModelRole::Identifier] = "identifier";
85- roles[IndicatorsModelRole::Position] = "position";
86- roles[IndicatorsModelRole::IndicatorProperties] = "indicatorProperties";
87- roles[IndicatorsModelRole::IsVisible] = "isVisible";
88- }
89- return roles;
90-}
91-
92-void VisibleIndicatorsModel::setSourceModel(QAbstractItemModel *model)
93-{
94- if (sourceModel() != model) {
95- QIdentityProxyModel::setSourceModel(model);
96- Q_EMIT modelChanged();
97- }
98-}
99-
100-QVariantMap VisibleIndicatorsModel::visible() const
101-{
102- return m_visible;
103-}
104-
105-void VisibleIndicatorsModel::onBeginRowInserted(const QModelIndex&, int, int)
106-{
107- m_inserting = true;
108-}
109-
110-void VisibleIndicatorsModel::onRowInserted(const QModelIndex&, int, int)
111-{
112- m_inserting = false;
113-}
114-
115-void VisibleIndicatorsModel::setVisible(const QVariantMap& visible)
116-{
117- if (m_visible != visible) {
118- m_visible = visible;
119- Q_EMIT visibleChanged();
120-
121- // need to tell the view that the visible data has changed.
122- if (!m_inserting && rowCount() > 0) {
123- Q_EMIT dataChanged(index(0, 0), index(rowCount() - 1, 0), QVector<int>() << IndicatorsModelRole::IsVisible);
124- }
125- }
126-}
127-
128-QVariant VisibleIndicatorsModel::data(const QModelIndex &index, int role) const
129-{
130- if (role != IndicatorsModelRole::IsVisible) {
131- return QIdentityProxyModel::data(index, role);
132- }
133-
134- if (!index.isValid()) {
135- return false;
136- }
137-
138- QString ident = QIdentityProxyModel::data(index, IndicatorsModelRole::Identifier).toString();
139- return m_visible.value(ident, false).toBool();
140-}
141
142=== removed file 'plugins/Unity/Indicators/visibleindicatorsmodel.h'
143--- plugins/Unity/Indicators/visibleindicatorsmodel.h 2013-09-02 07:39:53 +0000
144+++ plugins/Unity/Indicators/visibleindicatorsmodel.h 1970-01-01 00:00:00 +0000
145@@ -1,56 +0,0 @@
146-/*
147- * Copyright (C) 2012, 2013 Canonical, Ltd.
148- *
149- * This program is free software; you can redistribute it and/or modify
150- * it under the terms of the GNU General Public License as published by
151- * the Free Software Foundation; version 3.
152- *
153- * This program is distributed in the hope that it will be useful,
154- * but WITHOUT ANY WARRANTY; without even the implied warranty of
155- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
156- * GNU General Public License for more details.
157- *
158- * You should have received a copy of the GNU General Public License
159- * along with this program. If not, see <http://www.gnu.org/licenses/>.
160- *
161- * Authors:
162- * Nick Dedekind <nick.dedekind@canonical.com>
163- */
164-
165-#ifndef VISIBLEINDICATORSMODEL_H
166-#define VISIBLEINDICATORSMODEL_H
167-
168-#include <QIdentityProxyModel>
169-
170-class VisibleIndicatorsModel : public QIdentityProxyModel
171-{
172- Q_OBJECT
173-
174- Q_PROPERTY(QAbstractItemModel* model READ sourceModel WRITE setSourceModel NOTIFY modelChanged)
175- Q_PROPERTY(QVariantMap visible READ visible WRITE setVisible NOTIFY visibleChanged)
176-
177-public:
178- explicit VisibleIndicatorsModel(QObject *parent = 0);
179-
180- virtual void setSourceModel(QAbstractItemModel *model);
181- virtual QHash<int, QByteArray> roleNames() const;
182- virtual QVariant data(const QModelIndex &index, int role) const;
183-
184- QVariantMap visible() const;
185- void setVisible(const QVariantMap& visible);
186-
187-public Q_SLOTS:
188- void onBeginRowInserted(const QModelIndex&, int start, int end);
189- void onRowInserted(const QModelIndex&, int start, int end);
190-
191-Q_SIGNALS:
192- void totalCountChanged();
193- void modelChanged();
194- void visibleChanged();
195-
196-private:
197- QVariantMap m_visible;
198- bool m_inserting;
199-};
200-
201-#endif // VISIBLEINDICATORSMODEL_H
202
203=== removed file 'qml/Panel/Indicators/VisibleIndicators.qml'
204--- qml/Panel/Indicators/VisibleIndicators.qml 2014-12-19 14:51:35 +0000
205+++ qml/Panel/Indicators/VisibleIndicators.qml 1970-01-01 00:00:00 +0000
206@@ -1,84 +0,0 @@
207-/*
208- * Copyright 2013 Canonical Ltd.
209- *
210- * This program is free software; you can redistribute it and/or modify
211- * it under the terms of the GNU Lesser General Public License as published by
212- * the Free Software Foundation; version 3.
213- *
214- * This program is distributed in the hope that it will be useful,
215- * but WITHOUT ANY WARRANTY; without even the implied warranty of
216- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
217- * GNU Lesser General Public License for more details.
218- *
219- * You should have received a copy of the GNU Lesser General Public License
220- * along with this program. If not, see <http://www.gnu.org/licenses/>.
221- *
222- * Authors:
223- * Nick Dedekind <nick.dedekind@canonical.com>
224- */
225-
226-import QtQuick 2.0
227-import Unity.Indicators 0.1 as Indicators
228-import Utils 0.1
229-
230-Item {
231- property UnitySortFilterProxyModel model: filterModel
232-
233- function initialise(profile) {
234- indicatorsModel.load(profile);
235- }
236-
237- UnitySortFilterProxyModel {
238- id: filterModel
239- filterRole: Indicators.IndicatorsModelRole.IsVisible
240- filterRegExp: RegExp("^true$")
241- dynamicSortFilter: true
242-
243- model: Indicators.VisibleIndicatorsModel {
244- id: visibleIndicatorsModel
245- model: indicatorsModel
246- }
247- }
248-
249- Indicators.IndicatorsModel {
250- id: indicatorsModel
251- }
252-
253- Repeater {
254- id: repeater
255- model: indicatorsModel
256-
257- property var visibleIndicators: undefined
258- onVisibleIndicatorsChanged: {
259- if (visibleIndicators !== undefined) {
260- visibleIndicatorsModel.visible = visibleIndicators;
261- }
262- }
263-
264- delegate: IndicatorDelegate {
265- id: item
266- objectName: model.identifier + "-delegate"
267- identifier: model.identifier
268- Component.onCompleted: {
269- for(var pName in indicatorProperties) {
270- if (item.hasOwnProperty(pName)) {
271- item[pName] = indicatorProperties[pName];
272- }
273- }
274- updateVisibility();
275- }
276-
277- onEnabledChanged: {
278- updateVisibility()
279- }
280-
281- function updateVisibility() {
282- if (repeater.visibleIndicators === undefined) {
283- repeater.visibleIndicators = {}
284- }
285- repeater.visibleIndicators[model.identifier] = enabled;
286- repeater.visibleIndicatorsChanged();
287- }
288- }
289- }
290-}
291
292=== modified file 'qml/Shell.qml'
293--- qml/Shell.qml 2015-01-09 10:41:41 +0000
294+++ qml/Shell.qml 2015-01-21 15:47:40 +0000
295@@ -36,11 +36,11 @@
296 import "Components"
297 import "Notifications"
298 import "Stages"
299-import "Panel/Indicators"
300 import "Wizard"
301 import Unity.Notifications 1.0 as NotificationBackend
302 import Unity.Session 0.1
303 import Unity.DashCommunicator 0.1
304+import Unity.Indicators 0.1 as Indicators
305
306 Item {
307 id: shell
308@@ -737,13 +737,9 @@
309 minimizedPanelHeight: units.gu(3)
310 expandedPanelHeight: units.gu(7)
311
312- indicatorsModel: visibleIndicators.model
313- }
314-
315- VisibleIndicators {
316- id: visibleIndicators
317- // TODO: This should be sourced by device type (eg "desktop", "tablet", "phone"...)
318- Component.onCompleted: initialise(indicatorProfile)
319+ indicatorsModel: Indicators.IndicatorsModel {
320+ Component.onCompleted: load(indicatorProfile);
321+ }
322 }
323 callHint {
324 greeterShown: greeter.shown || lockscreen.shown
325
326=== modified file 'run.sh'
327--- run.sh 2014-09-02 09:14:28 +0000
328+++ run.sh 2015-01-21 15:47:40 +0000
329@@ -60,7 +60,7 @@
330 if $USE_MOCKS; then
331 rm -f $PWD/builddir/nonmirplugins/LightDM # undo symlink (from below) for cleanliness
332 export QML2_IMPORT_PATH=$QML2_IMPORT_PATH:$PWD/builddir/tests/mocks:$PWD/builddir/plugins:$PWD/builddir/modules
333- export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/builddir/tests/mocks/libusermetrics
334+ export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/builddir/tests/mocks/libusermetrics:$PWD/builddir/tests/mocks/QMenuModel
335 else
336 # Just link our LightDM mock into the nonmirplugins folder. We don't want
337 # the rest of our plugins to be used.
338
339=== modified file 'tests/mocks/Unity/Indicators/CMakeLists.txt'
340--- tests/mocks/Unity/Indicators/CMakeLists.txt 2014-10-07 10:28:59 +0000
341+++ tests/mocks/Unity/Indicators/CMakeLists.txt 2015-01-21 15:47:40 +0000
342@@ -19,7 +19,6 @@
343 ${CMAKE_SOURCE_DIR}/plugins/Unity/Indicators/sharedunitymenumodel.cpp
344 ${CMAKE_SOURCE_DIR}/plugins/Unity/Indicators/unitymenumodelcache.cpp
345 ${CMAKE_SOURCE_DIR}/plugins/Unity/Indicators/unitymenumodelstack.cpp
346- ${CMAKE_SOURCE_DIR}/plugins/Unity/Indicators/visibleindicatorsmodel.cpp
347 )
348
349 add_library(IndicatorsFakeQml SHARED
350
351=== modified file 'tests/mocks/Unity/Indicators/fakeplugin.cpp'
352--- tests/mocks/Unity/Indicators/fakeplugin.cpp 2014-10-07 10:28:59 +0000
353+++ tests/mocks/Unity/Indicators/fakeplugin.cpp 2015-01-21 15:47:40 +0000
354@@ -29,7 +29,6 @@
355 #include "sharedunitymenumodel.h"
356 #include "fakeunitymenumodelcache.h"
357 #include "unitymenumodelstack.h"
358-#include "visibleindicatorsmodel.h"
359
360 #include <unitymenumodel.h>
361
362@@ -49,7 +48,6 @@
363 // external
364 qmlRegisterType<MenuContentActivator>(uri, 0, 1, "MenuContentActivator");
365 qmlRegisterType<UnityMenuModelStack>(uri, 0, 1, "UnityMenuModelStack");
366- qmlRegisterType<VisibleIndicatorsModel>(uri, 0, 1, "VisibleIndicatorsModel");
367 qmlRegisterType<SharedUnityMenuModel>(uri, 0, 1, "SharedUnityMenuModel");
368
369 qmlRegisterSingletonType<FakeUnityMenuModelCache>(uri, 0, 1, "UnityMenuModelCache", menuModelCacheSingleton);
370
371=== modified file 'tests/qmltests/Panel/IndicatorTest.qml'
372--- tests/qmltests/Panel/IndicatorTest.qml 2014-10-17 14:55:35 +0000
373+++ tests/qmltests/Panel/IndicatorTest.qml 2015-01-21 15:47:40 +0000
374@@ -26,15 +26,9 @@
375 id: root
376 color: "white"
377
378- property alias indicatorsModel: __visibleIndicatorsModel
379+ property alias indicatorsModel: __indicatorsModel
380 property alias originalModelData: __indicatorsModel.originalModelData
381
382- Indicators.VisibleIndicatorsModel {
383- id: __visibleIndicatorsModel
384-
385- model: __indicatorsModel
386- }
387-
388 Indicators.IndicatorsModel {
389 id: __indicatorsModel
390 Component.onCompleted: load();
391
392=== modified file 'tests/qmltests/Panel/tst_MenuContent.qml'
393--- tests/qmltests/Panel/tst_MenuContent.qml 2014-10-17 14:55:35 +0000
394+++ tests/qmltests/Panel/tst_MenuContent.qml 2015-01-21 15:47:40 +0000
395@@ -111,7 +111,7 @@
396 var menuIndex = i%menuCount;
397
398 activate_content(menuIndex);
399- testItemObjectName = indicatorsModel.model.data(menuIndex, Indicators.IndicatorsModelRole.Identifier);
400+ testItemObjectName = indicatorsModel.data(menuIndex, Indicators.IndicatorsModelRole.Identifier);
401 compare(listView.currentIndex, menuIndex, "Current tab index does not match selected tab index");
402 tryCompareFunction(current_item_equals_test_item, true);
403 }

Subscribers

People subscribed via source and target branches