Merge lp:~paulliu/unity8/lp1378469_MessageMenu into lp:unity8

Proposed by Ying-Chun Liu
Status: Rejected
Rejected by: Nick Dedekind
Proposed branch: lp:~paulliu/unity8/lp1378469_MessageMenu
Merge into: lp:unity8
Diff against target: 96 lines (+20/-0)
4 files modified
qml/Panel/IndicatorPage.qml (+12/-0)
qml/Panel/Indicators/MessageMenuItemFactory.qml (+2/-0)
qml/Panel/IndicatorsMenu.qml (+1/-0)
qml/Panel/MenuContent.qml (+5/-0)
To merge this branch: bzr merge lp:~paulliu/unity8/lp1378469_MessageMenu
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Nick Dedekind (community) Needs Fixing
Unity Team Pending
Review via email: mp+244164@code.launchpad.net

Commit message

Auto hide the indicators menu when last message is replied. (LP: 1378469)

Description of the change

 * 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?
N/A
 * If you changed the UI, has there been a design review?
N/A

To post a comment you must log in.
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Hm. i'd rather use a signal from the Factory to close the indicator rather than calling hide directly.
We used to have this feature when clicking buttons/etc in the indicator. Take a look at revision 585.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
1462. By Ying-Chun Liu

Use signal to handle the hide event.

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

Urg. This is a bit nasty. Not only is this putting specific knowledge about how an indicator works into the page, it's also going to be a bit buggy.

Firstly, we're assuming that the page will be empty when we reply to the last message. Secondly, if we reply to a message when there are more than one in there and then clear all the messages at a later time then the indicators will close, due to the cached "messageReplied" property.

I think we need another component to keep track of how many messages are left (eg. MessageCounter.qml can use a filter model with model.type == "com.canonical.indicator.messages.messageitem"). Then, if we reply to a message and the count of messages drops to zero within 'x' seconds (still not the best), then signal a panel close. You could add the timer to what you have already; but it's just so much code speific to the messaging menu...

This is prabably a bit more complicated than you originally thought; so if you want me to take over the bug let me know :)

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Unmerged revisions

1462. By Ying-Chun Liu

Use signal to handle the hide event.

1461. By Ying-Chun Liu

Fix bugs

1460. By Ying-Chun Liu

Message replied then auto-close.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/Panel/IndicatorPage.qml'
--- qml/Panel/IndicatorPage.qml 2014-10-31 17:07:08 +0000
+++ qml/Panel/IndicatorPage.qml 2014-12-17 15:11:49 +0000
@@ -27,6 +27,8 @@
27 property alias highlightFollowsCurrentItem : mainMenu.highlightFollowsCurrentItem27 property alias highlightFollowsCurrentItem : mainMenu.highlightFollowsCurrentItem
28 readonly property alias factory: _factory28 readonly property alias factory: _factory
2929
30 signal repliedLastMessage()
31
30 Indicators.UnityMenuModelStack {32 Indicators.UnityMenuModelStack {
31 id: menuStack33 id: menuStack
32 head: main.menuModel34 head: main.menuModel
@@ -92,11 +94,18 @@
9294
93 property int selectedIndex: -195 property int selectedIndex: -1
94 property bool blockCurrentIndexChange: false96 property bool blockCurrentIndexChange: false
97 property bool messageReplied: false
95 // for count = 098 // for count = 0
96 onCountChanged: {99 onCountChanged: {
97 if (count == 0 && selectedIndex != -1) {100 if (count == 0 && selectedIndex != -1) {
98 selectedIndex = -1;101 selectedIndex = -1;
99 }102 }
103 if (messageReplied) {
104 messageReplied = false;
105 if (count == 0) {
106 main.repliedLastMessage();
107 }
108 }
100 }109 }
101 // for highlight following110 // for highlight following
102 onSelectedIndexChanged: {111 onSelectedIndexChanged: {
@@ -153,6 +162,9 @@
153 if (item.hasOwnProperty("menuIndex")) {162 if (item.hasOwnProperty("menuIndex")) {
154 item.menuIndex = Qt.binding(function() { return modelIndex; });163 item.menuIndex = Qt.binding(function() { return modelIndex; });
155 }164 }
165 if (item.hasOwnProperty("mainMenu")) {
166 item.mainMenu = Qt.binding(function() { return mainMenu; });
167 }
156 }168 }
157169
158 Binding {170 Binding {
159171
=== modified file 'qml/Panel/Indicators/MessageMenuItemFactory.qml'
--- qml/Panel/Indicators/MessageMenuItemFactory.qml 2014-09-09 16:01:25 +0000
+++ qml/Panel/Indicators/MessageMenuItemFactory.qml 2014-12-17 15:11:49 +0000
@@ -30,6 +30,7 @@
30 property var menuModel: null30 property var menuModel: null
31 property QtObject menuData: null31 property QtObject menuData: null
32 property int menuIndex: -132 property int menuIndex: -1
33 property var mainMenu: null
3334
34 property bool selected: false35 property bool selected: false
35 signal menuSelected36 signal menuSelected
@@ -165,6 +166,7 @@
165 }166 }
166 onReplied: {167 onReplied: {
167 replyAction.activate(value);168 replyAction.activate(value);
169 messageFactoryItem.mainMenu.messageReplied = true;
168 }170 }
169 onTriggered: {171 onTriggered: {
170 if (selected) {172 if (selected) {
171173
=== modified file 'qml/Panel/IndicatorsMenu.qml'
--- qml/Panel/IndicatorsMenu.qml 2014-10-21 19:32:03 +0000
+++ qml/Panel/IndicatorsMenu.qml 2014-12-17 15:11:49 +0000
@@ -79,6 +79,7 @@
79 visible: root.unitProgress > 079 visible: root.unitProgress > 0
80 enabled: contentEnabled80 enabled: contentEnabled
81 currentMenuIndex: bar.currentItemIndex81 currentMenuIndex: bar.currentItemIndex
82 indicatorsMenu: root
82 }83 }
8384
84 Handle {85 Handle {
8586
=== modified file 'qml/Panel/MenuContent.qml'
--- qml/Panel/MenuContent.qml 2014-10-23 11:59:22 +0000
+++ qml/Panel/MenuContent.qml 2014-12-17 15:11:49 +0000
@@ -27,6 +27,7 @@
2727
28 property QtObject indicatorsModel: null28 property QtObject indicatorsModel: null
29 property int currentMenuIndex: -129 property int currentMenuIndex: -1
30 property var indicatorsMenu: null
30 color: "#221e1c" // FIXME not in palette yet31 color: "#221e1c" // FIXME not in palette yet
3132
32 width: units.gu(40)33 width: units.gu(40)
@@ -70,6 +71,10 @@
70 busName: indicatorProperties.busName71 busName: indicatorProperties.busName
71 actionsObjectPath: indicatorProperties.actionsObjectPath72 actionsObjectPath: indicatorProperties.actionsObjectPath
72 menuObjectPath: indicatorProperties.menuObjectPath73 menuObjectPath: indicatorProperties.menuObjectPath
74
75 onRepliedLastMessage: {
76 content.indicatorsMenu.hide()
77 }
73 }78 }
7479
75 onVisibleChanged: {80 onVisibleChanged: {

Subscribers

People subscribed via source and target branches