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 | |
6 | implicitHeight: expandingItem.height |
7 | |
8 | + signal filterSelected() |
9 | + |
10 | ListItems.Expandable { |
11 | id: expandingItem |
12 | objectName: "expandingItem" |
13 | @@ -67,6 +69,7 @@ |
14 | width: parent.width |
15 | Repeater { |
16 | model: widgetData.options |
17 | + objectName: "optionsRepeater" |
18 | |
19 | ListItems.Standard { |
20 | text: label |
21 | @@ -83,6 +86,7 @@ |
22 | |
23 | onClicked: { |
24 | widgetData.options.setChecked(index, !checked); |
25 | + root.filterSelected() |
26 | } |
27 | } |
28 | } |
29 | |
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 | //! Widget data, forwarded to the widget as is. |
35 | property var widgetData: null |
36 | |
37 | + // Emitted iff filter type is FilterOptionSelector |
38 | + signal singleSelectionFilterSelected() |
39 | + |
40 | implicitHeight: title.height + title.anchors.topMargin + loader.height |
41 | |
42 | Label { |
43 | @@ -77,5 +80,10 @@ |
44 | item.widgetId = Qt.binding(function() { return root.widgetId } ) |
45 | item.widgetData = Qt.binding(function() { return root.widgetData } ) |
46 | } |
47 | + |
48 | + Connections { |
49 | + target: widgetType == Filters.OptionSelectorFilter ? loader.item : null |
50 | + onFilterSelected: root.singleSelectionFilterSelected() |
51 | + } |
52 | } |
53 | } |
54 | |
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 | categoryView.pageHeader.closePopup(); |
60 | categoryView.pageHeader.unfocus(); |
61 | } |
62 | + |
63 | + onExtraPanelOptionSelected: { |
64 | + categoryView.pageHeader.closePopup(); |
65 | + categoryView.pageHeader.unfocus(); |
66 | + } |
67 | } |
68 | } |
69 | |
70 | |
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 | |
76 | signal historyItemClicked(string text) |
77 | signal dashNavigationLeafClicked() |
78 | + signal extraPanelOptionSelected() |
79 | |
80 | function resetNavigation() { |
81 | dashNavigation.resetNavigation(); |
82 | @@ -181,6 +182,8 @@ |
83 | widgetId: filter ? filter.filterId : "" |
84 | widgetType: filter ? filter.filterType : -1 |
85 | widgetData: filter |
86 | + |
87 | + onSingleSelectionFilterSelected: extraPanelOptionSelected() |
88 | } |
89 | } |
90 | |
91 | |
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 | m_primaryNavigationFilter = new FakeOptionSelectorFilter("OSF3", "PFTag", "Which food you like More", false, primaryNavigationFilterOptionLabels, this); |
97 | m_filters->addFakeFilters(); |
98 | connect(m_filters, &Filters::activeFiltersCountChanged, this, &Scope::activeFiltersCountChanged); |
99 | + connect(m_primaryNavigationFilter, &FakeOptionSelectorFilter::isActiveChanged, |
100 | + this, &unity::shell::scopes::ScopeInterface::primaryNavigationTagChanged); |
101 | } |
102 | |
103 | QString Scope::id() const |
104 | @@ -325,6 +327,8 @@ |
105 | |
106 | QString Scope::primaryNavigationTag() const |
107 | { |
108 | + if (m_hasPrimaryFilter && m_primaryNavigationFilter->isActive()) |
109 | + return m_primaryNavigationFilter->filterTag(); |
110 | if (m_currentNavigationId == "root") |
111 | return QString(); |
112 | else |
113 | |
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 | tryCompare(categoryListView, "atYBeginning", true) |
119 | } |
120 | |
121 | + function test_navigationFilterPopupClosesWhenOptionSelected() { |
122 | + dash.setCurrentScope("LongPrimaryNavigation") |
123 | + |
124 | + var dashContentList = findChild(dashContent, "dashContentList") |
125 | + var searchButton = findChild(dashContentList.currentItem, "search_button") |
126 | + verify(searchButton) |
127 | + var extraPanel = findChild(dashContentList.currentItem, "peExtraPanel") |
128 | + verify(extraPanel) |
129 | + |
130 | + var primaryFilter = findChild(extraPanel, "primaryFilter") |
131 | + var expandingItem = findChild(primaryFilter, "expandingItem") |
132 | + verify(expandingItem) |
133 | + tryCompare(expandingItem, "expanded", false) |
134 | + |
135 | + mouseClick(searchButton) |
136 | + expandingItem.expanded = true |
137 | + tryCompare(expandingItem, "expanded", true) |
138 | + tryCompareFunction(function() { return expandingItem.height == expandingItem.expandedHeight; }, true); |
139 | + |
140 | + var optionsRepeater = findChild(expandingItem, "optionsRepeater") |
141 | + verify(optionsRepeater) |
142 | + verify(optionsRepeater.itemAt(0)) |
143 | + tryCompare(optionsRepeater.itemAt(0), "visible", true) |
144 | + mouseClick(optionsRepeater.itemAt(0)) |
145 | + tryCompare(expandingItem, "visible", false) |
146 | + } |
147 | + |
148 | function test_manage_dash_clickscope_unfavoritable() { |
149 | // Show the manage dash |
150 | 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.