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.
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
1462. By Ying-Chun Liu on 2014-12-17

Use signal to handle the hide event.

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

Unmerged revisions

1462. By Ying-Chun Liu on 2014-12-17

Use signal to handle the hide event.

1461. By Ying-Chun Liu on 2014-12-09

Fix bugs

1460. By Ying-Chun Liu on 2014-12-03

Message replied then auto-close.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/Panel/IndicatorPage.qml'
2--- qml/Panel/IndicatorPage.qml 2014-10-31 17:07:08 +0000
3+++ qml/Panel/IndicatorPage.qml 2014-12-17 15:11:49 +0000
4@@ -27,6 +27,8 @@
5 property alias highlightFollowsCurrentItem : mainMenu.highlightFollowsCurrentItem
6 readonly property alias factory: _factory
7
8+ signal repliedLastMessage()
9+
10 Indicators.UnityMenuModelStack {
11 id: menuStack
12 head: main.menuModel
13@@ -92,11 +94,18 @@
14
15 property int selectedIndex: -1
16 property bool blockCurrentIndexChange: false
17+ property bool messageReplied: false
18 // for count = 0
19 onCountChanged: {
20 if (count == 0 && selectedIndex != -1) {
21 selectedIndex = -1;
22 }
23+ if (messageReplied) {
24+ messageReplied = false;
25+ if (count == 0) {
26+ main.repliedLastMessage();
27+ }
28+ }
29 }
30 // for highlight following
31 onSelectedIndexChanged: {
32@@ -153,6 +162,9 @@
33 if (item.hasOwnProperty("menuIndex")) {
34 item.menuIndex = Qt.binding(function() { return modelIndex; });
35 }
36+ if (item.hasOwnProperty("mainMenu")) {
37+ item.mainMenu = Qt.binding(function() { return mainMenu; });
38+ }
39 }
40
41 Binding {
42
43=== modified file 'qml/Panel/Indicators/MessageMenuItemFactory.qml'
44--- qml/Panel/Indicators/MessageMenuItemFactory.qml 2014-09-09 16:01:25 +0000
45+++ qml/Panel/Indicators/MessageMenuItemFactory.qml 2014-12-17 15:11:49 +0000
46@@ -30,6 +30,7 @@
47 property var menuModel: null
48 property QtObject menuData: null
49 property int menuIndex: -1
50+ property var mainMenu: null
51
52 property bool selected: false
53 signal menuSelected
54@@ -165,6 +166,7 @@
55 }
56 onReplied: {
57 replyAction.activate(value);
58+ messageFactoryItem.mainMenu.messageReplied = true;
59 }
60 onTriggered: {
61 if (selected) {
62
63=== modified file 'qml/Panel/IndicatorsMenu.qml'
64--- qml/Panel/IndicatorsMenu.qml 2014-10-21 19:32:03 +0000
65+++ qml/Panel/IndicatorsMenu.qml 2014-12-17 15:11:49 +0000
66@@ -79,6 +79,7 @@
67 visible: root.unitProgress > 0
68 enabled: contentEnabled
69 currentMenuIndex: bar.currentItemIndex
70+ indicatorsMenu: root
71 }
72
73 Handle {
74
75=== modified file 'qml/Panel/MenuContent.qml'
76--- qml/Panel/MenuContent.qml 2014-10-23 11:59:22 +0000
77+++ qml/Panel/MenuContent.qml 2014-12-17 15:11:49 +0000
78@@ -27,6 +27,7 @@
79
80 property QtObject indicatorsModel: null
81 property int currentMenuIndex: -1
82+ property var indicatorsMenu: null
83 color: "#221e1c" // FIXME not in palette yet
84
85 width: units.gu(40)
86@@ -70,6 +71,10 @@
87 busName: indicatorProperties.busName
88 actionsObjectPath: indicatorProperties.actionsObjectPath
89 menuObjectPath: indicatorProperties.menuObjectPath
90+
91+ onRepliedLastMessage: {
92+ content.indicatorsMenu.hide()
93+ }
94 }
95
96 onVisibleChanged: {

Subscribers

People subscribed via source and target branches