Merge lp:~josharenson/unity8/close-extra-panel-when-selected into lp:unity8
- close-extra-panel-when-selected
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Albert Astals Cid |
Approved revision: | 2354 |
Merged at revision: | 2421 |
Proposed branch: | lp:~josharenson/unity8/close-extra-panel-when-selected |
Merge into: | lp:unity8 |
Prerequisite: | lp:~josharenson/unity8/fix-primary-filter-scrolling |
Diff against target: |
150 lines (+51/-0) 6 files modified
qml/Dash/Filters/FilterOptionSelector.qml (+4/-0) qml/Dash/Filters/FilterWidgetFactory.qml (+8/-0) qml/Dash/GenericScopeView.qml (+5/-0) qml/Dash/PageHeaderExtraPanel.qml (+3/-0) tests/mocks/Unity/fake_scope.cpp (+4/-0) tests/qmltests/Dash/tst_Dash.qml (+27/-0) |
To merge this branch: | bzr merge lp:~josharenson/unity8/close-extra-panel-when-selected |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Albert Astals Cid (community) | Approve | ||
Unity8 CI Bot | continuous-integration | Needs Fixing | |
Review via email:
|
Commit message
Close the PageHeaderExtra
Description of the change
* Are there any related MPs required for this MP to build/function as expected? Please list.
Yes, ~josharenson/
* 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, clean
* 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?
Not sure if Albert already got approval for this or not...
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Josh Arenson (josharenson) wrote : | # |
Updated signal name and added a test
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2340
https:/
Executed test runs:
UNSTABLE: https:/
UNSTABLE: https:/
FAILURE: 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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2342
https:/
Executed test runs:
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: 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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2343
https:/
Executed test runs:
UNSTABLE: https:/
SUCCESS: https:/
FAILURE: 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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
I don't think that "expandingItem.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2344
https:/
Executed test runs:
UNSTABLE: https:/
UNSTABLE: https:/
FAILURE: 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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2345
https:/
Executed test runs:
UNSTABLE: https:/
UNSTABLE: https:/
FAILURE: 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:/
- 2348. By Josh Arenson
-
merge trunk
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Instead of
dashContent.
in the test can you do
switch_
?
This way when/if it fails it'll be easier to reproduce "manually" with make tryDashContent
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2346
https:/
Executed test runs:
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 2349. By Josh Arenson
-
merge prereq
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Josh Arenson (josharenson) wrote : | # |
Moved test, fixed conflicts, stripped tags
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2347
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2347
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Something went wrong with the merging?
Now it doesn't even compile :S
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2348
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 2350. By Josh Arenson
-
Merge trunk
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2349
https:/
Executed test runs:
FAILURE: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2350
https:/
Executed test runs:
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 2351. By Josh Arenson
-
Rebase
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2351
https:/
Executed test runs:
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 2352. By Josh Arenson
-
merge prereq
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Josh Arenson (josharenson) wrote : | # |
Untangled the disastrous merge situation. For real.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2352
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
"""
+ function test_navigation
"""
Did you use tab chars here?
- 2353. By Josh Arenson
-
Fix tab
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Josh Arenson (josharenson) wrote : | # |
> """
> + function test_navigation
> {
> """
>
> Did you use tab chars here?
Thats what I get for copy pasting with gedit :-p. Fixing now.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2353
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
FAILURE: 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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Looks good now.
But to make it behave more like the real world we should have
http://
on the mock plugin, so that actually when selecting a option the "navitgation block" shows up.
Can you add that to this MR? Or you prefer we do it in a separate one?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Josh Arenson (josharenson) wrote : | # |
> Looks good now.
>
> But to make it behave more like the real world we should have
>
> http://
>
> on the mock plugin, so that actually when selecting a option the "navitgation
> block" shows up.
>
> Can you add that to this MR? Or you prefer we do it in a separate one?
I'll add it. It isn't necessary for this branch, but it is a small change and its more correct.
- 2354. By Josh Arenson
-
Set the filterTag in the search box when one is selected
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2354
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
FAILURE: 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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes
* Did CI run pass? If not, please explain why.
Yes, except broken autopilot
Preview Diff
1 | === modified file 'qml/Dash/Filters/FilterOptionSelector.qml' | |||
2 | --- qml/Dash/Filters/FilterOptionSelector.qml 2016-01-27 10:39:24 +0000 | |||
3 | +++ qml/Dash/Filters/FilterOptionSelector.qml 2016-05-09 20:25:44 +0000 | |||
4 | @@ -25,6 +25,8 @@ | |||
5 | 25 | 25 | ||
6 | 26 | implicitHeight: expandingItem.height | 26 | implicitHeight: expandingItem.height |
7 | 27 | 27 | ||
8 | 28 | signal filterSelected() | ||
9 | 29 | |||
10 | 28 | ListItems.Expandable { | 30 | ListItems.Expandable { |
11 | 29 | id: expandingItem | 31 | id: expandingItem |
12 | 30 | objectName: "expandingItem" | 32 | objectName: "expandingItem" |
13 | @@ -67,6 +69,7 @@ | |||
14 | 67 | width: parent.width | 69 | width: parent.width |
15 | 68 | Repeater { | 70 | Repeater { |
16 | 69 | model: widgetData.options | 71 | model: widgetData.options |
17 | 72 | objectName: "optionsRepeater" | ||
18 | 70 | 73 | ||
19 | 71 | ListItems.Standard { | 74 | ListItems.Standard { |
20 | 72 | text: label | 75 | text: label |
21 | @@ -83,6 +86,7 @@ | |||
22 | 83 | 86 | ||
23 | 84 | onClicked: { | 87 | onClicked: { |
24 | 85 | widgetData.options.setChecked(index, !checked); | 88 | widgetData.options.setChecked(index, !checked); |
25 | 89 | root.filterSelected() | ||
26 | 86 | } | 90 | } |
27 | 87 | } | 91 | } |
28 | 88 | } | 92 | } |
29 | 89 | 93 | ||
30 | === modified file 'qml/Dash/Filters/FilterWidgetFactory.qml' | |||
31 | --- qml/Dash/Filters/FilterWidgetFactory.qml 2016-04-19 13:02:11 +0000 | |||
32 | +++ qml/Dash/Filters/FilterWidgetFactory.qml 2016-05-09 20:25:44 +0000 | |||
33 | @@ -31,6 +31,9 @@ | |||
34 | 31 | //! Widget data, forwarded to the widget as is. | 31 | //! Widget data, forwarded to the widget as is. |
35 | 32 | property var widgetData: null | 32 | property var widgetData: null |
36 | 33 | 33 | ||
37 | 34 | // Emitted iff filter type is FilterOptionSelector | ||
38 | 35 | signal singleSelectionFilterSelected() | ||
39 | 36 | |||
40 | 34 | implicitHeight: title.height + title.anchors.topMargin + loader.height | 37 | implicitHeight: title.height + title.anchors.topMargin + loader.height |
41 | 35 | 38 | ||
42 | 36 | Label { | 39 | Label { |
43 | @@ -77,5 +80,10 @@ | |||
44 | 77 | item.widgetId = Qt.binding(function() { return root.widgetId } ) | 80 | item.widgetId = Qt.binding(function() { return root.widgetId } ) |
45 | 78 | item.widgetData = Qt.binding(function() { return root.widgetData } ) | 81 | item.widgetData = Qt.binding(function() { return root.widgetData } ) |
46 | 79 | } | 82 | } |
47 | 83 | |||
48 | 84 | Connections { | ||
49 | 85 | target: widgetType == Filters.OptionSelectorFilter ? loader.item : null | ||
50 | 86 | onFilterSelected: root.singleSelectionFilterSelected() | ||
51 | 87 | } | ||
52 | 80 | } | 88 | } |
53 | 81 | } | 89 | } |
54 | 82 | 90 | ||
55 | === modified file 'qml/Dash/GenericScopeView.qml' | |||
56 | --- qml/Dash/GenericScopeView.qml 2016-05-04 18:08:35 +0000 | |||
57 | +++ qml/Dash/GenericScopeView.qml 2016-05-09 20:25:44 +0000 | |||
58 | @@ -689,6 +689,11 @@ | |||
59 | 689 | categoryView.pageHeader.closePopup(); | 689 | categoryView.pageHeader.closePopup(); |
60 | 690 | categoryView.pageHeader.unfocus(); | 690 | categoryView.pageHeader.unfocus(); |
61 | 691 | } | 691 | } |
62 | 692 | |||
63 | 693 | onExtraPanelOptionSelected: { | ||
64 | 694 | categoryView.pageHeader.closePopup(); | ||
65 | 695 | categoryView.pageHeader.unfocus(); | ||
66 | 696 | } | ||
67 | 692 | } | 697 | } |
68 | 693 | } | 698 | } |
69 | 694 | 699 | ||
70 | 695 | 700 | ||
71 | === modified file 'qml/Dash/PageHeaderExtraPanel.qml' | |||
72 | --- qml/Dash/PageHeaderExtraPanel.qml 2016-05-09 20:25:43 +0000 | |||
73 | +++ qml/Dash/PageHeaderExtraPanel.qml 2016-05-09 20:25:44 +0000 | |||
74 | @@ -36,6 +36,7 @@ | |||
75 | 36 | 36 | ||
76 | 37 | signal historyItemClicked(string text) | 37 | signal historyItemClicked(string text) |
77 | 38 | signal dashNavigationLeafClicked() | 38 | signal dashNavigationLeafClicked() |
78 | 39 | signal extraPanelOptionSelected() | ||
79 | 39 | 40 | ||
80 | 40 | function resetNavigation() { | 41 | function resetNavigation() { |
81 | 41 | dashNavigation.resetNavigation(); | 42 | dashNavigation.resetNavigation(); |
82 | @@ -181,6 +182,8 @@ | |||
83 | 181 | widgetId: filter ? filter.filterId : "" | 182 | widgetId: filter ? filter.filterId : "" |
84 | 182 | widgetType: filter ? filter.filterType : -1 | 183 | widgetType: filter ? filter.filterType : -1 |
85 | 183 | widgetData: filter | 184 | widgetData: filter |
86 | 185 | |||
87 | 186 | onSingleSelectionFilterSelected: extraPanelOptionSelected() | ||
88 | 184 | } | 187 | } |
89 | 185 | } | 188 | } |
90 | 186 | 189 | ||
91 | 187 | 190 | ||
92 | === modified file 'tests/mocks/Unity/fake_scope.cpp' | |||
93 | --- tests/mocks/Unity/fake_scope.cpp 2016-05-09 20:25:43 +0000 | |||
94 | +++ tests/mocks/Unity/fake_scope.cpp 2016-05-09 20:25:44 +0000 | |||
95 | @@ -52,6 +52,8 @@ | |||
96 | 52 | m_primaryNavigationFilter = new FakeOptionSelectorFilter("OSF3", "PFTag", "Which food you like More", false, primaryNavigationFilterOptionLabels, this); | 52 | m_primaryNavigationFilter = new FakeOptionSelectorFilter("OSF3", "PFTag", "Which food you like More", false, primaryNavigationFilterOptionLabels, this); |
97 | 53 | m_filters->addFakeFilters(); | 53 | m_filters->addFakeFilters(); |
98 | 54 | connect(m_filters, &Filters::activeFiltersCountChanged, this, &Scope::activeFiltersCountChanged); | 54 | connect(m_filters, &Filters::activeFiltersCountChanged, this, &Scope::activeFiltersCountChanged); |
99 | 55 | connect(m_primaryNavigationFilter, &FakeOptionSelectorFilter::isActiveChanged, | ||
100 | 56 | this, &unity::shell::scopes::ScopeInterface::primaryNavigationTagChanged); | ||
101 | 55 | } | 57 | } |
102 | 56 | 58 | ||
103 | 57 | QString Scope::id() const | 59 | QString Scope::id() const |
104 | @@ -325,6 +327,8 @@ | |||
105 | 325 | 327 | ||
106 | 326 | QString Scope::primaryNavigationTag() const | 328 | QString Scope::primaryNavigationTag() const |
107 | 327 | { | 329 | { |
108 | 330 | if (m_hasPrimaryFilter && m_primaryNavigationFilter->isActive()) | ||
109 | 331 | return m_primaryNavigationFilter->filterTag(); | ||
110 | 328 | if (m_currentNavigationId == "root") | 332 | if (m_currentNavigationId == "root") |
111 | 329 | return QString(); | 333 | return QString(); |
112 | 330 | else | 334 | else |
113 | 331 | 335 | ||
114 | === modified file 'tests/qmltests/Dash/tst_Dash.qml' | |||
115 | --- tests/qmltests/Dash/tst_Dash.qml 2016-05-09 20:25:43 +0000 | |||
116 | +++ tests/qmltests/Dash/tst_Dash.qml 2016-05-09 20:25:44 +0000 | |||
117 | @@ -146,6 +146,33 @@ | |||
118 | 146 | tryCompare(categoryListView, "atYBeginning", true) | 146 | tryCompare(categoryListView, "atYBeginning", true) |
119 | 147 | } | 147 | } |
120 | 148 | 148 | ||
121 | 149 | function test_navigationFilterPopupClosesWhenOptionSelected() { | ||
122 | 150 | dash.setCurrentScope("LongPrimaryNavigation") | ||
123 | 151 | |||
124 | 152 | var dashContentList = findChild(dashContent, "dashContentList") | ||
125 | 153 | var searchButton = findChild(dashContentList.currentItem, "search_button") | ||
126 | 154 | verify(searchButton) | ||
127 | 155 | var extraPanel = findChild(dashContentList.currentItem, "peExtraPanel") | ||
128 | 156 | verify(extraPanel) | ||
129 | 157 | |||
130 | 158 | var primaryFilter = findChild(extraPanel, "primaryFilter") | ||
131 | 159 | var expandingItem = findChild(primaryFilter, "expandingItem") | ||
132 | 160 | verify(expandingItem) | ||
133 | 161 | tryCompare(expandingItem, "expanded", false) | ||
134 | 162 | |||
135 | 163 | mouseClick(searchButton) | ||
136 | 164 | expandingItem.expanded = true | ||
137 | 165 | tryCompare(expandingItem, "expanded", true) | ||
138 | 166 | tryCompareFunction(function() { return expandingItem.height == expandingItem.expandedHeight; }, true); | ||
139 | 167 | |||
140 | 168 | var optionsRepeater = findChild(expandingItem, "optionsRepeater") | ||
141 | 169 | verify(optionsRepeater) | ||
142 | 170 | verify(optionsRepeater.itemAt(0)) | ||
143 | 171 | tryCompare(optionsRepeater.itemAt(0), "visible", true) | ||
144 | 172 | mouseClick(optionsRepeater.itemAt(0)) | ||
145 | 173 | tryCompare(expandingItem, "visible", false) | ||
146 | 174 | } | ||
147 | 175 | |||
148 | 149 | function test_manage_dash_clickscope_unfavoritable() { | 176 | function test_manage_dash_clickscope_unfavoritable() { |
149 | 150 | // Show the manage dash | 177 | // Show the manage dash |
150 | 151 | touchFlick(dash, dash.width / 2, dash.height - 1, dash.width / 2, units.gu(2)); | 178 | touchFlick(dash, dash.width / 2, dash.height - 1, dash.width / 2, units.gu(2)); |
Please use a different name + comment instead of " signal filterOptionSel ected() " in the future we may use it for other "single selection" filters like a radio button,
Also a test would be nice.