Unity 8

Merge lp:~aacid/unity8/clickOpenMenuClosesIt into lp:unity8

  1. clickOpenMenuClosesIt
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Superseded
Proposed branch: lp:~aacid/unity8/clickOpenMenuClosesIt
Merge into: lp:unity8
Diff against target: 154 lines (+77/-6)
3 files modified
qml/ApplicationMenus/MenuBar.qml (+8/-1)
qml/ApplicationMenus/MenuPopup.qml (+19/-5)
tests/qmltests/ApplicationMenus/tst_MenuBar.qml (+50/-0)
To merge this branch: bzr merge lp:~aacid/unity8/clickOpenMenuClosesIt
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Needs Fixing
Daniel d'Andrada Pending
Review via email: mp+315649@code.launchpad.net

This proposal supersedes a proposal from 2017-01-20.

This proposal has been superseded by a proposal from 2017-01-26.

Commit message

Clicking on an open menu closes it + test

Description of the change

 * Are there any related MPs required for this MP to build/function as expected?
No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
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
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:2773
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2986/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3890
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2277
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2277
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3918
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3763
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3763/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3763
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3763/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3763
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3763/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3763
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3763/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3763
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3763/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3763
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3763/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2986/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:2773
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2992/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3896
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2283
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2283
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3924
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3769
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3769/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3769
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3769/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3769
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3769/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3769
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3769/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3769
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3769/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3769
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3769/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2992/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

* 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

review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2774
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3037/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3952
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/2309
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=zesty,testname=qmluitests.sh/2309
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3980
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3824
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3824/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3824
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3824/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3824
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3824/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3824
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3824/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3824
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3824/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3824
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3824/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3037/rebuild

review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/clickOpenMenuClosesIt updated
2775. By Albert Astals Cid

Merge

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'qml/ApplicationMenus/MenuBar.qml'
2--- qml/ApplicationMenus/MenuBar.qml 2017-01-17 09:45:22 +0000
3+++ qml/ApplicationMenus/MenuBar.qml 2017-01-26 09:02:07 +0000
4@@ -114,6 +114,7 @@
5 function show() {
6 if (!__popup) {
7 __popup = menuComponent.createObject(root, { objectName: visualItem.objectName + "-menu" });
8+ __popup.childActivated.connect(dismiss);
9 // force the current item to be the newly popped up menu
10 } else {
11 __popup.show();
12@@ -212,7 +213,13 @@
13 updateCurrentItemFromPosition(Qt.point(mouse.x, mouse.y))
14 }
15 }
16- onClicked: updateCurrentItemFromPosition(Qt.point(mouse.x, mouse.y))
17+ onClicked: {
18+ var prevItem = d.currentItem;
19+ updateCurrentItemFromPosition(Qt.point(mouse.x, mouse.y))
20+ if (prevItem && d.currentItem == prevItem) {
21+ prevItem.hide();
22+ }
23+ }
24
25 function updateCurrentItemFromPosition(point) {
26 var pos = mapToItem(row, point.x, point.y);
27
28=== modified file 'qml/ApplicationMenus/MenuPopup.qml'
29--- qml/ApplicationMenus/MenuPopup.qml 2017-01-18 12:08:05 +0000
30+++ qml/ApplicationMenus/MenuPopup.qml 2017-01-26 09:02:07 +0000
31@@ -26,6 +26,8 @@
32 objectName: "menu"
33 backgroundColor: theme.palette.normal.overlay
34
35+ signal childActivated()
36+
37 property alias unityMenuModel: repeater.model
38
39 function show() {
40@@ -263,16 +265,22 @@
41 return mapToItem(container, 0, y).y;
42 })
43 });
44+ popup.retreat.connect(function() {
45+ popup.destroy();
46+ popup = null;
47+ menuItem.forceActiveFocus();
48+ });
49+ popup.childActivated.connect(function() {
50+ popup.destroy();
51+ popup = null;
52+ root.childActivated();
53+ });
54 } else if (popup) {
55 popup.visible = true;
56 }
57- popup.retreat.connect(function() {
58- popup.destroy();
59- popup = null;
60- menuItem.forceActiveFocus();
61- })
62 } else {
63 root.unityMenuModel.activate(__ownIndex);
64+ root.childActivated();
65 }
66 }
67
68@@ -377,6 +385,7 @@
69
70 property var unityMenuModel: null
71 signal retreat()
72+ signal childActivated()
73
74 Binding {
75 target: item
76@@ -392,6 +401,11 @@
77
78 Keys.onLeftPressed: retreat()
79
80+ Connections {
81+ target: item
82+ onChildActivated: childActivated();
83+ }
84+
85 Component.onCompleted: item.select(0);
86 onVisibleChanged: if (visible) { item.select(0); }
87 }
88
89=== modified file 'tests/qmltests/ApplicationMenus/tst_MenuBar.qml'
90--- tests/qmltests/ApplicationMenus/tst_MenuBar.qml 2017-01-24 07:41:35 +0000
91+++ tests/qmltests/ApplicationMenus/tst_MenuBar.qml 2017-01-26 09:02:07 +0000
92@@ -180,6 +180,41 @@
93 tryCompare(priv, "currentItem", menuItem);
94 }
95
96+ function test_menuActivateClosesMenu() {
97+ menuBackend.modelData = appMenuData.generateTestData(3,3,0,0,"menu");
98+ var priv = findInvisibleChild(menuBar, "d");
99+
100+ var menuItem = findChild(menuBar, "menuBar-item0");
101+ menuItem.show();
102+ compare(priv.currentItem, menuItem, "CurrentItem should be set to item 0");
103+ compare(priv.currentItem.popupVisible, true, "Popup should be visible");
104+
105+ var actionItem = findChild(menuBar, "menuBar-item0-menu-item0-actionItem");
106+ mouseClick(actionItem);
107+ compare(priv.currentItem, null, "CurrentItem should be null");
108+ }
109+
110+ function test_subMenuActivateClosesMenu() {
111+ menuBackend.modelData = appMenuData.generateTestData(3,4,1,0,"menu");
112+ var priv = findInvisibleChild(menuBar, "d");
113+
114+ var menuItem = findChild(menuBar, "menuBar-item0");
115+ menuItem.show();
116+ compare(priv.currentItem, menuItem, "CurrentItem should be set to item 0");
117+ compare(priv.currentItem.popupVisible, true, "Popup should be visible");
118+
119+ var actionItem = findChild(menuBar, "menuBar-item0-menu-item0-actionItem");
120+ mouseClick(actionItem);
121+
122+ actionItem = findChild(menuBar, "menuBar-item0-menu-item0-menu-item0-actionItem");
123+ mouseClick(actionItem);
124+
125+ actionItem = findChild(menuBar, "menuBar-item0-menu-item0-menu-item0-menu-item0-actionItem");
126+ mouseClick(actionItem);
127+
128+ compare(priv.currentItem, null, "CurrentItem should be null");
129+ }
130+
131 function test_openAppMenuShortcut() {
132 var priv = findInvisibleChild(menuBar, "d");
133
134@@ -192,5 +227,20 @@
135 keyClick(Qt.Key_F10, Qt.AltModifier);
136 compare(priv.currentItem, menuItem1, "First enabled item should be opened");
137 }
138+
139+ function test_clickOpenMenuClosesMenu() {
140+ menuBackend.modelData = appMenuData.generateTestData(3,3,0,0,"menu");
141+ var priv = findInvisibleChild(menuBar, "d");
142+
143+ var menuItem = findChild(menuBar, "menuBar-item0");
144+ waitForRendering(menuItem);
145+ mouseClick(menuItem);
146+ compare(priv.currentItem, menuItem, "CurrentItem should be set to item 0");
147+ compare(priv.currentItem.popupVisible, true, "Popup should be visible");
148+
149+ waitForRendering(menuItem);
150+ mouseClick(menuItem);
151+ compare(priv.currentItem, null, "CurrentItem should be null");
152+ }
153 }
154 }

Subscribers

People subscribed via source and target branches