Unity 8

debian/changelog (+6/-0)
debian/control (+2/-1)
plugins/Unity/Indicators/indicatorsmanager.cpp (+3/-5)
plugins/Unity/Session/dbusunitysessionservice.cpp (+4/-1)
qml/Components/Dialogs.qml (+6/-47)
qml/Greeter/Greeter.qml (+6/-0)
qml/Panel/IndicatorItemRow.qml (+5/-2)
qml/Panel/Indicators/MenuItemFactory.qml (+16/-3)

To merge this branch:

bzr merge lp:~unity-team/unity8/sessionIndicatorForDevices

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Terry		2016-03-14	Approve on 2016-03-15
Unity8 CI Bot	continuous-integration	2016-03-14	Needs Fixing on 2016-03-15
Review via email: mp+288940@code.launchpad.net

This proposal supersedes a proposal from 2016-03-11.

Commit Message

Session indicator convergence

Description of the Change

Session indicator convergence

Enables the session indicator also on devices, tweaking/hiding some of the items that don't make sense there.

Cf. https://docs.google.com/spreadsheets/d/1coL7U4BrOMrK3op7y9TPmmLcBAlgtzoUcKIYrbFhQvU

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

https://code.launchpad.net/~unity-team/unity8/better-windowed-logic/+merge/288817 and https://code.launchpad.net/~lukas-kde/indicator-session/desktopModeSwitch/+merge/288413

Also needs indicator-session to be installed by default on devices!

* 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?

Yes

* If you changed the UI, has there been a design review?

Yes, as per https://docs.google.com/spreadsheets/d/1coL7U4BrOMrK3op7y9TPmmLcBAlgtzoUcKIYrbFhQvU

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-10: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-11: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2016-03-14: Posted in a previous version of this proposal

Text conflict in qml/OrientedShell.qml
1 conflicts encountered.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14: Posted in a previous version of this proposal

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

review: Needs Fixing (continuous-integration)

Michael Terry (mterry) wrote on 2016-03-14: Posted in a previous version of this proposal

+ Connections {
+ target: DBusUnitySessionService
+ onLockRequested: lightDM.greeter.showGreeter()
+ }

I think this would better fit in Greeter.qml. We try to consolidate all the greeter logic in there, so that it's more easily ignorable if we're running without a greeter (which some split greeter scenarios need).

+ PromptLock();

Huh... org.freedesktop.DisplayManager does not seem to emit a signal when we ask it to lock... I guess the intention was for lightdm to switch VTs. And then the desktop session would notice the vt switch? I'd have to check how we know to lock the session in such cases.

But I guess this is OK for now. We might have to confirm everything works as expected here in the split greeter branches though.

+ if ((new_indicator->identifier() == QStringLiteral("indicator-session"))

Can we not just drop that check and enable the correct profile for indicator-session? Does it still not have profile support? It would be more correct to fix that in the indicator than patch its menu items in unity8...

I haven't run this yet, this was just looking at code.

review: Needs Fixing

Michael Terry (mterry) wrote on 2016-03-14: Posted in a previous version of this proposal

Oh, and as the autopilot test failure shows, you need Replaces & Breaks for unity8-schemas:

https://wiki.debian.org/PackageTransition

Lukáš Tinkl (lukas-kde) wrote on 2016-03-14:

> Huh... org.freedesktop.DisplayManager does not seem to emit a signal when we ask it to lock... I > guess the intention was for lightdm to switch VTs. And then the desktop session would notice
> the vt switch? I'd have to check how we know to lock the session in such cases.

> But I guess this is OK for now. We might have to confirm everything works as expected here in the > split greeter branches though.

Yup, I guess in the long term, this signal should be emitted

> + if ((new_indicator->identifier() == QStringLiteral("indicator-session"))

> Can we not just drop that check and enable the correct profile for indicator-session? Does it
> still not have profile support? It would be more correct to fix that in the indicator than
> patch its menu items in unity8...

Yeah, no proper "phone" profile and certainly no time to do it now :/

lp:~unity-team/unity8/sessionIndicatorForDevices updated on 2016-03-14

2222. By Michael Terry on 2016-03-14: Fix up packaging a bit
2223. By Lukáš Tinkl on 2016-03-14: move the lock connection to Greeter.qml

Lukáš Tinkl (lukas-kde) wrote on 2016-03-14:

Moved the Connections to Greeter.qml, as requested

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

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

review: Needs Fixing (continuous-integration)

Michael Terry (mterry) wrote on 2016-03-14:

OK. Code looks fine, just need to test once silo builds.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-14:

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

review: Needs Fixing (continuous-integration)

Michael Terry (mterry) wrote on 2016-03-14:

Well it works. But I've got some beefs:

- "About This Device" should probably be just "About", following System Settings' lead.

- The dialog we show on shut down is oddly different than the normal shutdown. We should use the same button sorting (Shut down on top) and labels (Restart instead of Reboot). And the menu and dialog use "Shut down" when the normal dialog uses "Power off".

We could just use the same dialog. But we'd still have to fix the menu. Has design had a chance to comment on any of this?

- The long standing bug about duplicate separator items really stands out here and makes the menu look bad... Is that an easy fix? /me looks into it real quick

Michael Terry (mterry) wrote on 2016-03-14:

(I realize that some of these issues might be on the indicator-session side, I'm just noting what I saw with the combo.)

Michael Terry (mterry) wrote on 2016-03-14:

I see, it seems that the duplicates are just because this MP strips some items during load(). But it doens't strip their associated separators.

Lukáš Tinkl (lukas-kde) wrote on 2016-03-15:

> Well it works. But I've got some beefs:
>
> - "About This Device" should probably be just "About", following System
> Settings' lead.

Nope, the design document (linked in description) says "About this device"

> - The dialog we show on shut down is oddly different than the normal shutdown.
> We should use the same button sorting (Shut down on top) and labels (Restart
> instead of Reboot). And the menu and dialog use "Shut down" when the normal
> dialog uses "Power off".
>
> We could just use the same dialog. But we'd still have to fix the menu. Has
> design had a chance to comment on any of this?

Yes, I will unify this, no idea why we even had 2 different dialogs there

> - The long standing bug about duplicate separator items really stands out here
> and makes the menu look bad... Is that an easy fix? /me looks into it real
> quick

Hmm, this one is tough, I'm afraid it's impossible to remove them :/

lp:~unity-team/unity8/sessionIndicatorForDevices updated on 2016-03-15

2224. By Lukáš Tinkl on 2016-03-15: don§t confuse the user and show just one power/shutdown dialog

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

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

review: Needs Fixing (continuous-integration)

lp:~unity-team/unity8/sessionIndicatorForDevices updated on 2016-03-15

2225. By Lukáš Tinkl on 2016-03-15

gross hack for hiding the indicator-session for < 50GU screens

acked by dednick

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-15:

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

review: Needs Fixing (continuous-integration)

Michael Terry (mterry) wrote on 2016-03-15:

Tested this, worked fine.

There are a couple hacks in here that we'll want to clean up down the road (indicator-session should get proper profile support: bug 1557716, indicator-session should emit shutdown instead of reboot: bug 1557717, and the hard check for 90gu)

And we don't need unity8-schemas anymore, since the indicator-service only uses them if they are available (doesn't hard-depend on our packages anymore).

lp:~unity-team/unity8/sessionIndicatorForDevices updated on 2016-03-15

2226. By Lukáš Tinkl on 2016-03-15

remove unity8-schemas

not needed anymore, indicator-session has a runtime check

Michael Terry (mterry) wrote on 2016-03-15:

Awesome, looks fine now.

review: Approve

lp:~unity-team/unity8/sessionIndicatorForDevices updated on 2016-03-15

2227. By Lukáš Tinkl on 2016-03-15: make units.gu(60) the threshold between phone and tablet for the session indicator

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Mohammad Rakibul Hasan

Unity Team

 === modified file 'debian/changelog'
 --- debian/changelog	2016-03-10 22:44:16 +0000
 +++ debian/changelog	2016-03-15 23:11:02 +0000
@@ -1,3 +1,9 @@
++unity8 (8.12+16.04.20160310.4-0ubuntu1) UNRELEASED; urgency=medium
++
++  *
++
++ -- Michael Terry <mterry@ubuntu.com>  Mon, 14 Mar 2016 11:27:53 -0400
++
  unity8 (8.11+16.04.20160310.4-0ubuntu1) xenial; urgency=medium
    [ Albert Astals Cid ]
 === modified file 'debian/control'
 --- debian/control	2016-03-10 22:42:34 +0000
 +++ debian/control	2016-03-15 23:11:02 +0000
@@ -111,7 +111,8 @@
           unity8-private | unity-launcher-impl,
           ${misc:Depends},
           ${shlibs:Depends},
--Recommends: unity-scope-click,
++Recommends: indicator-session,
++            unity-scope-click,
              unity-scope-mediascanner2,
  Breaks: indicator-network (<< 0.5.1+14.10.20141014),
          ubuntu-touch-session (<< 0.107),
 === modified file 'plugins/Unity/Indicators/indicatorsmanager.cpp'
 --- plugins/Unity/Indicators/indicatorsmanager.cpp	2016-02-19 08:44:42 +0000
 +++ plugins/Unity/Indicators/indicatorsmanager.cpp	2016-03-15 23:11:02 +0000
@@ -1,5 +1,5 @@
  /*
-- * Copyright (C) 2013 Canonical, Ltd.
++ * Copyright (C) 2013-2016 Canonical, Ltd.
+  *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License as published by
@@ -12,8 +12,6 @@
+  *
   * You should have received a copy of the GNU General Public License
   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
-- *
-- * Author: Nick Dedekind <nick.dedekind@canonical.com>
   */
  #include "indicatorsmanager.h"
@@ -286,12 +284,12 @@
      new_indicator->init(data->m_fileInfo.fileName(), settings);
      // convergence:
--    // 1) enable session indicator conditionally, typically when running in a multisession/multiuser environment
++    // 1) enable session indicator
      // 2) on a PC, switch the battery/power indicator to desktop mode,
      //    can't control brightness for now and phone-on-desktop broken (FIXME)
      //
      // The rest of the indicators respect their default profile (which is "phone", even on desktop PCs)
--    if ((new_indicator->identifier() == QStringLiteral("indicator-session") && m_platform.isMultiSession())
++    if ((new_indicator->identifier() == QStringLiteral("indicator-session"))
              || (new_indicator->identifier() == QStringLiteral("indicator-power") && m_platform.isPC())) {
          new_indicator->setProfile(QStringLiteral("desktop"));
      } else {
 === modified file 'plugins/Unity/Session/dbusunitysessionservice.cpp'
 --- plugins/Unity/Session/dbusunitysessionservice.cpp	2015-11-24 20:00:37 +0000
 +++ plugins/Unity/Session/dbusunitysessionservice.cpp	2016-03-15 23:11:02 +0000
@@ -336,6 +336,9 @@
  void DBusUnitySessionService::Lock()
+ {
++    // signal u8 to show the lockscreen/greeter
++    PromptLock();
++
      // lock the session using the org.freedesktop.DisplayManager system DBUS service
      const QString sessionPath = QString::fromLocal8Bit(qgetenv("XDG_SESSION_PATH"));
      QDBusMessage msg = QDBusMessage::createMethodCall(QStringLiteral("org.freedesktop.DisplayManager"),
@@ -348,7 +351,7 @@
      connect(watcher, &QDBusPendingCallWatcher::finished,
          this, [this](QDBusPendingCallWatcher* watcher) {
--        QDBusPendingReply<QVariant> reply = *watcher;
++        QDBusPendingReply<void> reply = *watcher;
          watcher->deleteLater();
          if (reply.isError()) {
              qWarning() << "Lock call failed" << reply.error().message();
 === modified file 'qml/Components/Dialogs.qml'
 --- qml/Components/Dialogs.qml	2015-11-06 10:06:58 +0000
 +++ qml/Components/Dialogs.qml	2016-03-15 23:11:02 +0000
@@ -167,40 +167,6 @@
+     }
      Component {
--        id: shutdownDialogComponent
--        ShellDialog {
--            id: shutdownDialog
--            title: i18n.ctr("Title: Reboot/Shut down dialog", "Shut down")
--            text: i18n.tr("Are you sure you want to shut down?")
--            Button {
--                text: i18n.ctr("Button: Reboot the system", "Reboot")
--                onClicked: {
--                    root.closeAllApps();
--                    unitySessionService.reboot();
--                    shutdownDialog.hide();
--                }
--                color: UbuntuColors.lightGrey
--            }
--            Button {
--                text: i18n.ctr("Button: Shut down the system", "Shut down")
--                onClicked: {
--                    root.closeAllApps();
--                    unitySessionService.shutdown();
--                    shutdownDialog.hide();
--                }
--                color: UbuntuColors.red
--            }
--            Button {
--                text: i18n.tr("Cancel")
--                onClicked: {
--                    shutdownDialog.hide();
--                }
--                color: UbuntuColors.lightGrey
--            }
--        }
--    }
--
--    Component {
          id: rebootDialogComponent
          ShellDialog {
              id: rebootDialog
@@ -273,23 +239,16 @@
          onShutdownRequested: {
              // Display a dialog to ask the user to confirm.
--            if (!dialogLoader.active) {
--                dialogLoader.sourceComponent = shutdownDialogComponent;
--                dialogLoader.focus = true;
--                dialogLoader.active = true;
--            }
++            showPowerDialog();
+         }
          onRebootRequested: {
              // Display a dialog to ask the user to confirm.
--            if (!dialogLoader.active) {
--                // display a combined reboot/shutdown dialog, sadly the session indicator calls rather the "Reboot()" method
--                // than shutdown when clicking on the "Shutdown..." menu item
--                // FIXME: when/if session indicator is fixed, put the rebootDialogComponent here
--                dialogLoader.sourceComponent = shutdownDialogComponent;
--                dialogLoader.focus = true;
--                dialogLoader.active = true;
--            }
++
++            // display a combined reboot/shutdown dialog, sadly the session indicator calls rather the "Reboot()" method
++            // than shutdown when clicking on the "Shutdown..." menu item
++            // FIXME: when/if session indicator is fixed, put the rebootDialogComponent here
++            showPowerDialog();
+         }
          onLogoutReady: {
 === modified file 'qml/Greeter/Greeter.qml'
 --- qml/Greeter/Greeter.qml	2016-01-11 17:37:02 +0000
 +++ qml/Greeter/Greeter.qml	2016-03-15 23:11:02 +0000
@@ -20,6 +20,7 @@
  import Ubuntu.Components 1.3
  import Ubuntu.SystemImage 0.1
  import Unity.Launcher 0.1
++import Unity.Session 0.1
  import "../Components"
  Showable {
@@ -443,6 +444,11 @@
+         }
+     }
++    Connections {
++        target: DBusUnitySessionService
++        onLockRequested: root.forceShow()
++    }
++
      Binding {
          target: lightDM.greeter
          property: "active"
 === modified file 'qml/Panel/IndicatorItemRow.qml'
 --- qml/Panel/IndicatorItemRow.qml	2015-08-13 09:08:15 +0000
 +++ qml/Panel/IndicatorItemRow.qml	2016-03-15 23:11:02 +0000
@@ -15,6 +15,7 @@
   */
  import QtQuick 2.4
++import QtQuick.Window 2.2
  import Ubuntu.Components 1.3
  Item {
@@ -183,7 +184,9 @@
                  property int ownIndex: index
                  property bool overflow: row.width - x > overFlowWidth
--                property bool hidden: !expanded && (overflow || !indicatorVisible)
++                property bool hidden: !expanded && (overflow || !indicatorVisible || hideSessionIndicator)
++                // HACK for indicator-session
++                readonly property bool hideSessionIndicator: identifier == "indicator-session" && Math.min(Screen.width, Screen.height) <= units.gu(60)
                  height: row.height
                  expanded: root.expanded
@@ -199,7 +202,7 @@
                      NumberAnimation { duration: UbuntuAnimation.SnapDuration; easing: UbuntuAnimation.StandardEasing }
+                 }
--                width: (expanded || indicatorVisible) ? implicitWidth : 0
++                width: ((expanded || indicatorVisible) && !hideSessionIndicator) ? implicitWidth : 0
                  Behavior on width {
                      NumberAnimation { duration: UbuntuAnimation.SnapDuration; easing: UbuntuAnimation.StandardEasing }
 === modified file 'qml/Panel/Indicators/MenuItemFactory.qml'
 --- qml/Panel/Indicators/MenuItemFactory.qml	2016-03-10 13:14:29 +0000
 +++ qml/Panel/Indicators/MenuItemFactory.qml	2016-03-15 23:11:02 +0000
@@ -15,6 +15,7 @@
   */
  import QtQuick 2.4
++import QtQuick.Window 2.2
  import Ubuntu.Settings.Menus 0.1 as Menus
  import Ubuntu.Settings.Components 0.1
  import QMenuModel 0.1
@@ -22,6 +23,7 @@
  import Ubuntu.Components.ListItems 1.3 as ListItems
  import Ubuntu.Components 1.3
  import Unity.Session 0.1
++import Unity.Platform 1.0
  Item {
      id: menuFactory
@@ -61,9 +63,11 @@
              "com.canonical.indicator.calendar": calendarMenu,
              "com.canonical.indicator.location": timezoneMenu,
--
--            "indicator.user-menu-item": userMenuItem,
--            "indicator.guest-menu-item": userMenuItem
++        },
++        "indicator-session": {
++            "indicator.user-menu-item": Platform.isPC ? userMenuItem : null,
++            "indicator.guest-menu-item": Platform.isPC ? userMenuItem : null,
++            "com.canonical.indicator.switch": Math.min(Screen.width, Screen.height) > units.gu(60) ? switchMenu : null // Desktop mode switch
          },
          "indicator-messages" : {
              "com.canonical.indicator.button"         : messagesButtonMenu
@@ -976,6 +980,15 @@
+     }
      function load(modelData, context) {
++        // tweak indicator-session items
++        if (context === "indicator-session") {
++            if ((modelData.action === "indicator.logout" || modelData.action === "indicator.suspend" || modelData.action === "indicator.hibernate" ||
++                 modelData.action === "indicator.reboot")
++                    && !Platform.isPC) {
++                return null; // logout, suspend and hibernate hidden on devices
++            }
++        }
++
          if (modelData.type !== undefined && modelData.type !== "") {
              var component = undefined;