Merge lp:~cimi/unity8/fix-open-new-scope-from-tmp into lp:unity8
- fix-open-new-scope-from-tmp
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Albert Astals Cid |
Approved revision: | 1563 |
Merged at revision: | 1616 |
Proposed branch: | lp:~cimi/unity8/fix-open-new-scope-from-tmp |
Merge into: | lp:unity8 |
Prerequisite: | lp:~aacid/unity8/testFor1316660 |
Diff against target: |
420 lines (+130/-23) 14 files modified
debian/control (+2/-2) qml/Dash/Dash.qml (+6/-8) qml/Dash/DashContent.qml (+1/-2) tests/mocks/Unity/CMakeLists.txt (+1/-1) tests/mocks/Unity/fake_previewmodel.cpp (+15/-1) tests/mocks/Unity/fake_previewmodel.h (+7/-1) tests/mocks/Unity/fake_previewstack.cpp (+2/-2) tests/mocks/Unity/fake_previewstack.h (+3/-1) tests/mocks/Unity/fake_previewwidgetmodel.cpp (+1/-0) tests/mocks/Unity/fake_scope.cpp (+5/-5) tests/mocks/Unity/fake_scopes.cpp (+12/-0) tests/mocks/Unity/fake_scopes.h (+5/-0) tests/mocks/Unity/fake_scopesoverview.cpp (+1/-0) tests/qmltests/Dash/tst_Dash.qml (+69/-0) |
To merge this branch: | bzr merge lp:~cimi/unity8/fix-open-new-scope-from-tmp |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Albert Astals Cid (community) | Approve | ||
Michał Sawicz | Pending | ||
Review via email: mp+249471@code.launchpad.net |
This proposal supersedes a proposal from 2015-02-04.
Commit message
Fix temp scopes opening temp scopes, correctly close previously opened temp scope with its preview
Description of the change
* Are there any related MPs required for this MP to build/function as expected? Please list.
https:/
https:/
* 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?
n/a
* If you changed the UI, has there been a design review?
n/a
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal | # |
Andrea Cimitan (cimi) wrote : Posted in a previous version of this proposal | # |
> I don't understand the MR, you're adding to fake_scopes three methods that you
> don't use
future proof, if we wanted to test those, since they added API for this branch
and you're removing the code in closeScopes that checks that the
> scope you closed is indeed a scope that was opened.
>
that code is broken... m_openScope is never updated
> Also what's the point of having the scopeThatOpened
> can simply use scopes.closeScope?
I can switch to that too... but didn't want to move far from the previous implementation
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Continuous integration, rev:1552
http://
Executed test runs:
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal | # |
> > I don't understand the MR, you're adding to fake_scopes three methods that
> you
> > don't use
>
> future proof, if we wanted to test those, since they added API for this branch
Hmmm, no, the only added api for the branch is
Q_INVOKABLE void closeScope(
the others are private methods you'll never see
>
> and you're removing the code in closeScopes that checks that the
> > scope you closed is indeed a scope that was opened.
> >
> that code is broken... m_openScope is never updated
Broken where? after your changes? may be, then what about fixing it?
> > Also what's the point of having the scopeThatOpened
> you
> > can simply use scopes.closeScope?
>
> I can switch to that too... but didn't want to move far from the previous
> implementation
Well, you made Pawel to add this new method and now you're not using it?
Also, why are you using
dashTempScope
?
Can you please make it work with a click that calls activate in the scope and thus ends up in Scope::activate that emits openScope?
That way we're properly exercising all the parts of the code
Michał Sawicz (saviq) wrote : Posted in a previous version of this proposal | # |
You need to depend on the new unity-api version, bump the API requirement in tests/mocks/
Albert Astals Cid (aacid) wrote : | # |
Please drop scopeThatOpened
Albert Astals Cid (aacid) wrote : | # |
Seems like you don't need to store m_scopes in PreviewStack and you can just pass it down to the PreviewModel in the constructor, no?
Albert Astals Cid (aacid) wrote : | # |
You have declared
unity:
but not implemented it, just kill it?
Albert Astals Cid (aacid) wrote : | # |
Please mark closeScope in Scopes with override to mark it implements the declaration of the same function in the internface
Andrea Cimitan (cimi) wrote : | # |
Done
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1557
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:/
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1558
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Albert Astals Cid (aacid) wrote : | # |
Can you please turn
if (!m_tempScopes.
m_tempScopes
}
into just
m_tempScopes
It's a QSet after all, inserting won't produce a duplicate anyway
Albert Astals Cid (aacid) wrote : | # |
* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes, bug is fixed
* Did CI run pass?
No the jenkins one because it needs new unity-api, ran locally and found nothing.
* Did you make sure that the branch does not contain spurious tags?
Yes
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1559
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Albert Astals Cid (aacid) wrote : | # |
Somehow i missed a qml test failure, testPreview segfaults, please fix
- 1563. By Andrea Cimitan
-
Fix testPreview
Albert Astals Cid (aacid) wrote : | # |
It's good again
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1563
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
Preview Diff
1 | === modified file 'debian/control' | |||
2 | --- debian/control 2015-02-11 17:12:22 +0000 | |||
3 | +++ debian/control 2015-02-16 12:09:57 +0000 | |||
4 | @@ -27,7 +27,7 @@ | |||
5 | 27 | libqmenumodel-dev (>= 0.2.9), | 27 | libqmenumodel-dev (>= 0.2.9), |
6 | 28 | libqt5xmlpatterns5-dev, | 28 | libqt5xmlpatterns5-dev, |
7 | 29 | libsystemsettings-dev, | 29 | libsystemsettings-dev, |
9 | 30 | libunity-api-dev (>= 7.95), | 30 | libunity-api-dev (>= 7.96), |
10 | 31 | libusermetricsoutput1-dev, | 31 | libusermetricsoutput1-dev, |
11 | 32 | libxcb1-dev, | 32 | libxcb1-dev, |
12 | 33 | pkg-config, | 33 | pkg-config, |
13 | @@ -126,7 +126,7 @@ | |||
14 | 126 | unity-application-impl-4, | 126 | unity-application-impl-4, |
15 | 127 | unity-notifications-impl-3, | 127 | unity-notifications-impl-3, |
16 | 128 | unity-plugin-scopes | unity-scopes-impl, | 128 | unity-plugin-scopes | unity-scopes-impl, |
18 | 129 | unity-scopes-impl-4, | 129 | unity-scopes-impl-6, |
19 | 130 | unity8-fake-env | unity-application-impl, | 130 | unity8-fake-env | unity-application-impl, |
20 | 131 | ${misc:Depends}, | 131 | ${misc:Depends}, |
21 | 132 | Breaks: unity8 (<< 7.86), | 132 | Breaks: unity8 (<< 7.86), |
22 | 133 | 133 | ||
23 | === modified file 'qml/Dash/Dash.qml' | |||
24 | --- qml/Dash/Dash.qml 2015-01-12 08:31:10 +0000 | |||
25 | +++ qml/Dash/Dash.qml 2015-02-16 12:09:57 +0000 | |||
26 | @@ -122,7 +122,6 @@ | |||
27 | 122 | dash.setCurrentScope(scopeId, true, false); | 122 | dash.setCurrentScope(scopeId, true, false); |
28 | 123 | } | 123 | } |
29 | 124 | onOpenScope: { | 124 | onOpenScope: { |
30 | 125 | scopeItem.scopeThatOpenedScope = currentScope; | ||
31 | 126 | scopeItem.scope = scope; | 125 | scopeItem.scope = scope; |
32 | 127 | x = -width; | 126 | x = -width; |
33 | 128 | } | 127 | } |
34 | @@ -130,7 +129,7 @@ | |||
35 | 130 | UbuntuNumberAnimation { | 129 | UbuntuNumberAnimation { |
36 | 131 | onRunningChanged: { | 130 | onRunningChanged: { |
37 | 132 | if (!running && dashContent.x == 0) { | 131 | if (!running && dashContent.x == 0) { |
39 | 133 | scopeItem.scopeThatOpenedScope.closeScope(scopeItem.scope); | 132 | scopes.closeScope(scopeItem.scope); |
40 | 134 | scopeItem.scope = null; | 133 | scopeItem.scope = null; |
41 | 135 | } | 134 | } |
42 | 136 | } | 135 | } |
43 | @@ -187,7 +186,6 @@ | |||
44 | 187 | onOpenScope: { | 186 | onOpenScope: { |
45 | 188 | bottomEdgeController.enableAnimation = true; | 187 | bottomEdgeController.enableAnimation = true; |
46 | 189 | bottomEdgeController.progress = 0; | 188 | bottomEdgeController.progress = 0; |
47 | 190 | scopeItem.scopeThatOpenedScope = scopesList.scope; | ||
48 | 191 | scopeItem.scope = scope; | 189 | scopeItem.scope = scope; |
49 | 192 | dashContent.x = -dashContent.width; | 190 | dashContent.x = -dashContent.width; |
50 | 193 | } | 191 | } |
51 | @@ -199,8 +197,7 @@ | |||
52 | 199 | } | 197 | } |
53 | 200 | } | 198 | } |
54 | 201 | 199 | ||
57 | 202 | DashBackground | 200 | DashBackground { |
56 | 203 | { | ||
58 | 204 | anchors.fill: scopeItem | 201 | anchors.fill: scopeItem |
59 | 205 | visible: scopeItem.visible | 202 | visible: scopeItem.visible |
60 | 206 | } | 203 | } |
61 | @@ -209,8 +206,6 @@ | |||
62 | 209 | id: scopeItem | 206 | id: scopeItem |
63 | 210 | objectName: "dashTempScopeItem" | 207 | objectName: "dashTempScopeItem" |
64 | 211 | 208 | ||
65 | 212 | property var scopeThatOpenedScope: null | ||
66 | 213 | |||
67 | 214 | x: dashContent.x + width | 209 | x: dashContent.x + width |
68 | 215 | y: dashContent.y | 210 | y: dashContent.y |
69 | 216 | width: parent.width | 211 | width: parent.width |
70 | @@ -229,7 +224,10 @@ | |||
71 | 229 | dashContent.gotoScope(scopeId); | 224 | dashContent.gotoScope(scopeId); |
72 | 230 | } | 225 | } |
73 | 231 | onOpenScope: { | 226 | onOpenScope: { |
75 | 232 | dashContent.openScope(scope); | 227 | scopeItem.closePreview(); |
76 | 228 | var oldScope = scopeItem.scope; | ||
77 | 229 | scopeItem.scope = scope; | ||
78 | 230 | scopes.closeScope(oldScope); | ||
79 | 233 | } | 231 | } |
80 | 234 | } | 232 | } |
81 | 235 | } | 233 | } |
82 | 236 | 234 | ||
83 | === modified file 'qml/Dash/DashContent.qml' | |||
84 | --- qml/Dash/DashContent.qml 2015-01-05 15:08:04 +0000 | |||
85 | +++ qml/Dash/DashContent.qml 2015-02-16 12:09:57 +0000 | |||
86 | @@ -84,8 +84,7 @@ | |||
87 | 84 | 84 | ||
88 | 85 | set_current_index = undefined; | 85 | set_current_index = undefined; |
89 | 86 | 86 | ||
92 | 87 | if (dashContentList.count > index) | 87 | if (dashContentList.count > index) { |
91 | 88 | { | ||
93 | 89 | dashContentList.currentIndex = index | 88 | dashContentList.currentIndex = index |
94 | 90 | 89 | ||
95 | 91 | if (reset) { | 90 | if (reset) { |
96 | 92 | 91 | ||
97 | === modified file 'tests/mocks/Unity/CMakeLists.txt' | |||
98 | --- tests/mocks/Unity/CMakeLists.txt 2015-02-04 13:22:27 +0000 | |||
99 | +++ tests/mocks/Unity/CMakeLists.txt 2015-02-16 12:09:57 +0000 | |||
100 | @@ -6,7 +6,7 @@ | |||
101 | 6 | add_subdirectory(DashCommunicator) | 6 | add_subdirectory(DashCommunicator) |
102 | 7 | 7 | ||
103 | 8 | pkg_search_module(GOBJECT gobject-2.0 REQUIRED) | 8 | pkg_search_module(GOBJECT gobject-2.0 REQUIRED) |
105 | 9 | pkg_check_modules(SCOPES_API REQUIRED unity-shell-scopes=5) | 9 | pkg_check_modules(SCOPES_API REQUIRED unity-shell-scopes=6) |
106 | 10 | 10 | ||
107 | 11 | include_directories( | 11 | include_directories( |
108 | 12 | ${CMAKE_CURRENT_BINARY_DIR} | 12 | ${CMAKE_CURRENT_BINARY_DIR} |
109 | 13 | 13 | ||
110 | === modified file 'tests/mocks/Unity/fake_previewmodel.cpp' | |||
111 | --- tests/mocks/Unity/fake_previewmodel.cpp 2014-08-11 09:57:29 +0000 | |||
112 | +++ tests/mocks/Unity/fake_previewmodel.cpp 2015-02-16 12:09:57 +0000 | |||
113 | @@ -22,18 +22,23 @@ | |||
114 | 22 | #include "fake_previewmodel.h" | 22 | #include "fake_previewmodel.h" |
115 | 23 | 23 | ||
116 | 24 | // local | 24 | // local |
117 | 25 | #include "fake_scope.h" | ||
118 | 26 | #include "fake_scopes.h" | ||
119 | 25 | #include "fake_previewwidgetmodel.h" | 27 | #include "fake_previewwidgetmodel.h" |
120 | 26 | 28 | ||
121 | 27 | // Qt | 29 | // Qt |
122 | 28 | #include <QDebug> | 30 | #include <QDebug> |
123 | 29 | 31 | ||
125 | 30 | PreviewModel::PreviewModel(QObject* parent) | 32 | PreviewModel::PreviewModel(QObject* parent, Scope* scope) |
126 | 31 | : unity::shell::scopes::PreviewModelInterface(parent) | 33 | : unity::shell::scopes::PreviewModelInterface(parent) |
127 | 32 | , m_loaded(true) | 34 | , m_loaded(true) |
128 | 35 | , m_scope(scope) | ||
129 | 33 | { | 36 | { |
130 | 34 | // we have one column by default | 37 | // we have one column by default |
131 | 35 | PreviewWidgetModel* columnModel = new PreviewWidgetModel(this); | 38 | PreviewWidgetModel* columnModel = new PreviewWidgetModel(this); |
132 | 36 | m_previewWidgetModels.append(columnModel); | 39 | m_previewWidgetModels.append(columnModel); |
133 | 40 | connect(this, SIGNAL(triggered(QString const&, QString const&, QVariantMap const&)), | ||
134 | 41 | this, SLOT(triggeredSlot(QString const&, QString const&, QVariantMap const&))); | ||
135 | 37 | } | 42 | } |
136 | 38 | 43 | ||
137 | 39 | void PreviewModel::setWidgetColumnCount(int count) | 44 | void PreviewModel::setWidgetColumnCount(int count) |
138 | @@ -80,3 +85,12 @@ | |||
139 | 80 | Q_EMIT loadedChanged(); | 85 | Q_EMIT loadedChanged(); |
140 | 81 | } | 86 | } |
141 | 82 | } | 87 | } |
142 | 88 | |||
143 | 89 | void PreviewModel::triggeredSlot(QString const&, QString const&, QVariantMap const&) { | ||
144 | 90 | if (m_scope) { | ||
145 | 91 | Scopes *scopes = dynamic_cast<Scopes*>(m_scope->parent()); | ||
146 | 92 | Scope* scope = scopes->getScopeFromAll("MockScope9"); | ||
147 | 93 | scopes->addTempScope(scope); | ||
148 | 94 | Q_EMIT m_scope->openScope(scope); | ||
149 | 95 | } | ||
150 | 96 | } | ||
151 | 83 | 97 | ||
152 | === modified file 'tests/mocks/Unity/fake_previewmodel.h' | |||
153 | --- tests/mocks/Unity/fake_previewmodel.h 2014-08-11 09:57:29 +0000 | |||
154 | +++ tests/mocks/Unity/fake_previewmodel.h 2015-02-16 12:09:57 +0000 | |||
155 | @@ -25,12 +25,14 @@ | |||
156 | 25 | 25 | ||
157 | 26 | class PreviewWidgetModel; | 26 | class PreviewWidgetModel; |
158 | 27 | 27 | ||
159 | 28 | class Scope; | ||
160 | 29 | |||
161 | 28 | class PreviewModel : public unity::shell::scopes::PreviewModelInterface | 30 | class PreviewModel : public unity::shell::scopes::PreviewModelInterface |
162 | 29 | { | 31 | { |
163 | 30 | Q_OBJECT | 32 | Q_OBJECT |
164 | 31 | 33 | ||
165 | 32 | public: | 34 | public: |
167 | 33 | explicit PreviewModel(QObject* parent = 0); | 35 | explicit PreviewModel(QObject* parent = 0, Scope* scope = 0); |
168 | 34 | 36 | ||
169 | 35 | QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; | 37 | QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; |
170 | 36 | int rowCount(const QModelIndex& parent = QModelIndex()) const override; | 38 | int rowCount(const QModelIndex& parent = QModelIndex()) const override; |
171 | @@ -42,9 +44,13 @@ | |||
172 | 42 | 44 | ||
173 | 43 | Q_INVOKABLE void setLoaded(bool); // Only available for testing | 45 | Q_INVOKABLE void setLoaded(bool); // Only available for testing |
174 | 44 | 46 | ||
175 | 47 | private Q_SLOTS: | ||
176 | 48 | void triggeredSlot(QString const&, QString const&, QVariantMap const&); | ||
177 | 49 | |||
178 | 45 | private: | 50 | private: |
179 | 46 | QList<PreviewWidgetModel*> m_previewWidgetModels; | 51 | QList<PreviewWidgetModel*> m_previewWidgetModels; |
180 | 47 | bool m_loaded; | 52 | bool m_loaded; |
181 | 53 | Scope* m_scope; | ||
182 | 48 | }; | 54 | }; |
183 | 49 | 55 | ||
184 | 50 | Q_DECLARE_METATYPE(PreviewModel*) | 56 | Q_DECLARE_METATYPE(PreviewModel*) |
185 | 51 | 57 | ||
186 | === modified file 'tests/mocks/Unity/fake_previewstack.cpp' | |||
187 | --- tests/mocks/Unity/fake_previewstack.cpp 2014-05-19 11:10:58 +0000 | |||
188 | +++ tests/mocks/Unity/fake_previewstack.cpp 2015-02-16 12:09:57 +0000 | |||
189 | @@ -21,10 +21,10 @@ | |||
190 | 21 | #include "fake_previewmodel.h" | 21 | #include "fake_previewmodel.h" |
191 | 22 | #include "fake_scope.h" | 22 | #include "fake_scope.h" |
192 | 23 | 23 | ||
194 | 24 | PreviewStack::PreviewStack(QObject* parent) | 24 | PreviewStack::PreviewStack(QObject* parent, Scope *scope) |
195 | 25 | : unity::shell::scopes::PreviewStackInterface(parent) | 25 | : unity::shell::scopes::PreviewStackInterface(parent) |
196 | 26 | { | 26 | { |
198 | 27 | m_previews << new PreviewModel(this); | 27 | m_previews << new PreviewModel(this, scope); |
199 | 28 | } | 28 | } |
200 | 29 | 29 | ||
201 | 30 | PreviewStack::~PreviewStack() | 30 | PreviewStack::~PreviewStack() |
202 | 31 | 31 | ||
203 | === modified file 'tests/mocks/Unity/fake_previewstack.h' | |||
204 | --- tests/mocks/Unity/fake_previewstack.h 2014-05-19 11:10:58 +0000 | |||
205 | +++ tests/mocks/Unity/fake_previewstack.h 2015-02-16 12:09:57 +0000 | |||
206 | @@ -25,12 +25,14 @@ | |||
207 | 25 | 25 | ||
208 | 26 | class PreviewModel; | 26 | class PreviewModel; |
209 | 27 | 27 | ||
210 | 28 | class Scope; | ||
211 | 29 | |||
212 | 28 | class PreviewStack : public unity::shell::scopes::PreviewStackInterface | 30 | class PreviewStack : public unity::shell::scopes::PreviewStackInterface |
213 | 29 | { | 31 | { |
214 | 30 | Q_OBJECT | 32 | Q_OBJECT |
215 | 31 | 33 | ||
216 | 32 | public: | 34 | public: |
218 | 33 | explicit PreviewStack(QObject* parent = 0); | 35 | explicit PreviewStack(QObject* parent = 0, Scope* scope = 0); |
219 | 34 | virtual ~PreviewStack(); | 36 | virtual ~PreviewStack(); |
220 | 35 | 37 | ||
221 | 36 | QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; | 38 | QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; |
222 | 37 | 39 | ||
223 | === modified file 'tests/mocks/Unity/fake_previewwidgetmodel.cpp' | |||
224 | --- tests/mocks/Unity/fake_previewwidgetmodel.cpp 2015-02-16 12:09:57 +0000 | |||
225 | +++ tests/mocks/Unity/fake_previewwidgetmodel.cpp 2015-02-16 12:09:57 +0000 | |||
226 | @@ -57,6 +57,7 @@ | |||
227 | 57 | QVariantMap attributes; | 57 | QVariantMap attributes; |
228 | 58 | QVariantMap buttonData; | 58 | QVariantMap buttonData; |
229 | 59 | buttonData["label"] = "Button"; | 59 | buttonData["label"] = "Button"; |
230 | 60 | buttonData["id"] = "open_click"; | ||
231 | 60 | QVariantList buttons; | 61 | QVariantList buttons; |
232 | 61 | buttons << buttonData << buttonData << buttonData; | 62 | buttons << buttonData << buttonData << buttonData; |
233 | 62 | attributes["actions"] = QVariant::fromValue(buttons); | 63 | attributes["actions"] = QVariant::fromValue(buttons); |
234 | 63 | 64 | ||
235 | === modified file 'tests/mocks/Unity/fake_scope.cpp' | |||
236 | --- tests/mocks/Unity/fake_scope.cpp 2014-12-15 12:39:00 +0000 | |||
237 | +++ tests/mocks/Unity/fake_scope.cpp 2015-02-16 12:09:57 +0000 | |||
238 | @@ -185,6 +185,7 @@ | |||
239 | 185 | if (result.toString() == "Result.2.2") { | 185 | if (result.toString() == "Result.2.2") { |
240 | 186 | Scopes *scopes = dynamic_cast<Scopes*>(parent()); | 186 | Scopes *scopes = dynamic_cast<Scopes*>(parent()); |
241 | 187 | m_openScope = scopes->getScopeFromAll("MockScope9"); | 187 | m_openScope = scopes->getScopeFromAll("MockScope9"); |
242 | 188 | scopes->addTempScope(m_openScope); | ||
243 | 188 | Q_EMIT openScope(m_openScope); | 189 | Q_EMIT openScope(m_openScope); |
244 | 189 | } | 190 | } |
245 | 190 | } | 191 | } |
246 | @@ -198,7 +199,7 @@ | |||
247 | 198 | } else { | 199 | } else { |
248 | 199 | // This probably leaks, do we don't care | 200 | // This probably leaks, do we don't care |
249 | 200 | // it's a test after all | 201 | // it's a test after all |
251 | 201 | return new PreviewStack; | 202 | return new PreviewStack(this, this); |
252 | 202 | } | 203 | } |
253 | 203 | } | 204 | } |
254 | 204 | 205 | ||
255 | @@ -208,11 +209,10 @@ | |||
256 | 208 | 209 | ||
257 | 209 | void Scope::closeScope(unity::shell::scopes::ScopeInterface* scope) | 210 | void Scope::closeScope(unity::shell::scopes::ScopeInterface* scope) |
258 | 210 | { | 211 | { |
262 | 211 | if (scope != m_openScope) { | 212 | Scopes *scopes = dynamic_cast<Scopes*>(parent()); |
263 | 212 | qDebug() << scope << m_openScope; | 213 | if (scopes) { |
264 | 213 | qFatal("Scope::closeScope got wrong scope in closeScope"); | 214 | return scopes->closeScope(scope); |
265 | 214 | } | 215 | } |
266 | 215 | m_openScope = nullptr; | ||
267 | 216 | } | 216 | } |
268 | 217 | 217 | ||
269 | 218 | QString Scope::currentNavigationId() const | 218 | QString Scope::currentNavigationId() const |
270 | 219 | 219 | ||
271 | === modified file 'tests/mocks/Unity/fake_scopes.cpp' | |||
272 | --- tests/mocks/Unity/fake_scopes.cpp 2014-12-16 09:03:37 +0000 | |||
273 | +++ tests/mocks/Unity/fake_scopes.cpp 2015-02-16 12:09:57 +0000 | |||
274 | @@ -23,6 +23,7 @@ | |||
275 | 23 | // TODO: Implement remaining pieces, like Categories (i.e. LensView now gives warnings) | 23 | // TODO: Implement remaining pieces, like Categories (i.e. LensView now gives warnings) |
276 | 24 | 24 | ||
277 | 25 | // Qt | 25 | // Qt |
278 | 26 | #include <QDebug> | ||
279 | 26 | #include <QTimer> | 27 | #include <QTimer> |
280 | 27 | 28 | ||
281 | 28 | Scopes::Scopes(QObject *parent) | 29 | Scopes::Scopes(QObject *parent) |
282 | @@ -177,6 +178,17 @@ | |||
283 | 177 | return m_scopesOverview; | 178 | return m_scopesOverview; |
284 | 178 | } | 179 | } |
285 | 179 | 180 | ||
286 | 181 | void Scopes::addTempScope(unity::shell::scopes::ScopeInterface* scope) | ||
287 | 182 | { | ||
288 | 183 | m_tempScopes.insert(scope); | ||
289 | 184 | } | ||
290 | 185 | |||
291 | 186 | void Scopes::closeScope(unity::shell::scopes::ScopeInterface* scope) | ||
292 | 187 | { | ||
293 | 188 | Q_ASSERT(m_tempScopes.contains(scope)); | ||
294 | 189 | m_tempScopes.remove(scope); | ||
295 | 190 | } | ||
296 | 191 | |||
297 | 180 | void Scopes::setFavorite(const QString& scopeId, bool favorite) | 192 | void Scopes::setFavorite(const QString& scopeId, bool favorite) |
298 | 181 | { | 193 | { |
299 | 182 | if (favorite) { | 194 | if (favorite) { |
300 | 183 | 195 | ||
301 | === modified file 'tests/mocks/Unity/fake_scopes.h' | |||
302 | --- tests/mocks/Unity/fake_scopes.h 2014-12-10 09:07:14 +0000 | |||
303 | +++ tests/mocks/Unity/fake_scopes.h 2015-02-16 12:09:57 +0000 | |||
304 | @@ -26,6 +26,7 @@ | |||
305 | 26 | 26 | ||
306 | 27 | // Qt | 27 | // Qt |
307 | 28 | #include <QList> | 28 | #include <QList> |
308 | 29 | #include <QSet> | ||
309 | 29 | #include <QTimer> | 30 | #include <QTimer> |
310 | 30 | 31 | ||
311 | 31 | class ScopesOverview; | 32 | class ScopesOverview; |
312 | @@ -59,6 +60,9 @@ | |||
313 | 59 | Q_INVOKABLE void setFavorite(const QString& scopeId, bool favorite) override; | 60 | Q_INVOKABLE void setFavorite(const QString& scopeId, bool favorite) override; |
314 | 60 | Q_INVOKABLE void moveFavoriteTo(const QString& scopeId, int index) override; | 61 | Q_INVOKABLE void moveFavoriteTo(const QString& scopeId, int index) override; |
315 | 61 | 62 | ||
316 | 63 | void addTempScope(unity::shell::scopes::ScopeInterface* scope); | ||
317 | 64 | Q_INVOKABLE void closeScope(unity::shell::scopes::ScopeInterface* scope) override; | ||
318 | 65 | |||
319 | 62 | // This is used as part of implementation of the other C++ code, not API | 66 | // This is used as part of implementation of the other C++ code, not API |
320 | 63 | QList<Scope*> favScopes() const; | 67 | QList<Scope*> favScopes() const; |
321 | 64 | QList<Scope*> nonFavScopes() const; | 68 | QList<Scope*> nonFavScopes() const; |
322 | @@ -70,6 +74,7 @@ | |||
323 | 70 | private: | 74 | private: |
324 | 71 | QList<Scope*> m_scopes; // the favorite ones | 75 | QList<Scope*> m_scopes; // the favorite ones |
325 | 72 | QList<Scope*> m_allScopes; | 76 | QList<Scope*> m_allScopes; |
326 | 77 | QSet<unity::shell::scopes::ScopeInterface*> m_tempScopes; | ||
327 | 73 | ScopesOverview *m_scopesOverview; | 78 | ScopesOverview *m_scopesOverview; |
328 | 74 | bool m_loaded; | 79 | bool m_loaded; |
329 | 75 | QTimer timer; | 80 | QTimer timer; |
330 | 76 | 81 | ||
331 | === modified file 'tests/mocks/Unity/fake_scopesoverview.cpp' | |||
332 | --- tests/mocks/Unity/fake_scopesoverview.cpp 2014-12-16 09:03:37 +0000 | |||
333 | +++ tests/mocks/Unity/fake_scopesoverview.cpp 2015-02-16 12:09:57 +0000 | |||
334 | @@ -48,6 +48,7 @@ | |||
335 | 48 | Q_EMIT gotoScope(result.toString()); | 48 | Q_EMIT gotoScope(result.toString()); |
336 | 49 | } else { | 49 | } else { |
337 | 50 | m_openScope = scopes->getScopeFromAll(result.toString()); | 50 | m_openScope = scopes->getScopeFromAll(result.toString()); |
338 | 51 | scopes->addTempScope(m_openScope); | ||
339 | 51 | Q_EMIT openScope(m_openScope); | 52 | Q_EMIT openScope(m_openScope); |
340 | 52 | } | 53 | } |
341 | 53 | } | 54 | } |
342 | 54 | 55 | ||
343 | === modified file 'tests/qmltests/Dash/tst_Dash.qml' | |||
344 | --- tests/qmltests/Dash/tst_Dash.qml 2015-01-09 10:42:42 +0000 | |||
345 | +++ tests/qmltests/Dash/tst_Dash.qml 2015-02-16 12:09:57 +0000 | |||
346 | @@ -428,5 +428,74 @@ | |||
347 | 428 | tryCompare(bottomEdgeController, "progress", 0); | 428 | tryCompare(bottomEdgeController, "progress", 0); |
348 | 429 | tryCompare(dashContentList, "currentIndex", 1) | 429 | tryCompare(dashContentList, "currentIndex", 1) |
349 | 430 | } | 430 | } |
350 | 431 | |||
351 | 432 | function test_close_temp_scope_preview_opening_scope() { | ||
352 | 433 | // Show the manage dash | ||
353 | 434 | touchFlick(dash, dash.width / 2, dash.height - 1, dash.width / 2, units.gu(2)); | ||
354 | 435 | var bottomEdgeController = findInvisibleChild(dash, "bottomEdgeController"); | ||
355 | 436 | tryCompare(bottomEdgeController, "progress", 1); | ||
356 | 437 | |||
357 | 438 | // Make sure stuff is loaded | ||
358 | 439 | var nonfavScopesListCategory = findChild(dash, "scopesListCategoryother"); | ||
359 | 440 | var nonfavScopesListCategoryList = findChild(nonfavScopesListCategory, "scopesListCategoryInnerList"); | ||
360 | 441 | tryCompare(nonfavScopesListCategoryList, "currentIndex", 0); | ||
361 | 442 | |||
362 | 443 | // Click on a non favorite scope | ||
363 | 444 | mouseClick(nonfavScopesListCategoryList.currentItem); | ||
364 | 445 | |||
365 | 446 | // Check the bottom edge (manage dash) is disabled from temp scope | ||
366 | 447 | var overviewDragHandle = findChild(dash, "overviewDragHandle"); | ||
367 | 448 | compare(overviewDragHandle.enabled, false); | ||
368 | 449 | |||
369 | 450 | // Check temp scope is there | ||
370 | 451 | var dashTempScopeItem = findChild(dash, "dashTempScopeItem"); | ||
371 | 452 | tryCompare(dashTempScopeItem, "x", 0); | ||
372 | 453 | tryCompare(dashTempScopeItem, "visible", true); | ||
373 | 454 | |||
374 | 455 | // Check the manage dash is gone | ||
375 | 456 | tryCompare(bottomEdgeController, "progress", 0); | ||
376 | 457 | |||
377 | 458 | // Open preview | ||
378 | 459 | var categoryListView = findChild(dashTempScopeItem, "categoryListView"); | ||
379 | 460 | categoryListView.positionAtBeginning(); | ||
380 | 461 | tryCompareFunction(function() { | ||
381 | 462 | var cardGrid = findChild(dashTempScopeItem, "dashCategory0"); | ||
382 | 463 | if (cardGrid != null) { | ||
383 | 464 | var tile = findChild(cardGrid, "delegate0"); | ||
384 | 465 | return tile != null; | ||
385 | 466 | } | ||
386 | 467 | return false; | ||
387 | 468 | }, | ||
388 | 469 | true); | ||
389 | 470 | var tile = findChild(findChild(dashTempScopeItem, "dashCategory0"), "delegate0"); | ||
390 | 471 | waitForRendering(tile); | ||
391 | 472 | mouseClick(tile); | ||
392 | 473 | var subPageLoader = findChild(dashTempScopeItem, "subPageLoader"); | ||
393 | 474 | tryCompare(subPageLoader, "open", true); | ||
394 | 475 | tryCompare(subPageLoader, "x", 0); | ||
395 | 476 | tryCompare(findChild(dashTempScopeItem, "categoryListView"), "visible", false); | ||
396 | 477 | var previewListRow0 = findChild(subPageLoader, "previewListRow0"); | ||
397 | 478 | touchFlick(previewListRow0, previewListRow0.width / 2, units.gu(20), previewListRow0.width / 2, units.gu(1)); | ||
398 | 479 | tryCompare(previewListRow0, "atYEnd", true); | ||
399 | 480 | tryCompare(previewListRow0, "moving", false); | ||
400 | 481 | var widget = findChild(subPageLoader, "widget-21"); | ||
401 | 482 | var initialWidgetHeight = widget.height; | ||
402 | 483 | var openButton = findChild(widget, "buttonopen_click"); | ||
403 | 484 | mouseClick(openButton); | ||
404 | 485 | |||
405 | 486 | tryCompare(subPageLoader, "open", false); | ||
406 | 487 | tryCompare(subPageLoader, "x", subPageLoader.width); | ||
407 | 488 | |||
408 | 489 | compare(dashTempScopeItem.scope.id, "MockScope9"); | ||
409 | 490 | |||
410 | 491 | // Go back | ||
411 | 492 | var dashTempScopeItemHeader = findChild(dashTempScopeItem, "scopePageHeader"); | ||
412 | 493 | var backButton = findChild(findChild(dashTempScopeItemHeader, "innerPageHeader"), "customBackButton"); | ||
413 | 494 | mouseClick(backButton); | ||
414 | 495 | |||
415 | 496 | // Check temp scope is gone | ||
416 | 497 | tryCompare(dashTempScopeItem, "x", dash.width); | ||
417 | 498 | tryCompare(dashTempScopeItem, "visible", false); | ||
418 | 499 | } | ||
419 | 431 | } | 500 | } |
420 | 432 | } | 501 | } |
I don't understand the MR, you're adding to fake_scopes three methods that you don't use and you're removing the code in closeScopes that checks that the scope you closed is indeed a scope that was opened.
Also what's the point of having the scopeThatOpened Scope variable now that you can simply use scopes.closeScope?