Merge lp:~azzar1/unity/lp-1281058 into lp:unity

Proposed by Andrea Azzarone
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: no longer in the source branch.
Merged at revision: 3802
Proposed branch: lp:~azzar1/unity/lp-1281058
Merge into: lp:unity
Diff against target: 187 lines (+63/-9)
4 files modified
UnityCore/GnomeSessionManager.cpp (+32/-0)
UnityCore/GnomeSessionManagerImpl.h (+4/-0)
UnityCore/SessionManager.h (+4/-0)
shutdown/SessionView.cpp (+23/-9)
To merge this branch: bzr merge lp:~azzar1/unity/lp-1281058
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Marco Trevisan (Treviño) Approve
Brandon Schaefer (community) Approve
Review via email: mp+215331@code.launchpad.net

Commit message

Add a warning in the session dialog if other sessions are still open.

Description of the change

Add a warning in the session dialog if other sessions are still open. Would be nice to use just the "Session" property but the changed signal is not emitted.

To post a comment you must log in.
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: Approve (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Do we need any sort of UIFE? Or some sort of string exception? Other wise looks good to me.

review: Approve
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

14 + dm_proxy_->Connect("SessionAdded", sigc::hide(sigc::mem_fun(this, &Impl::UpdateHaveOtherOpenSessions)));
15 + dm_proxy_->Connect("SessionRemoved", sigc::hide(sigc::mem_fun(this, &Impl::UpdateHaveOtherOpenSessions)));

I didn't check if the proxy correctly emits the PropertyChanged signal (as per the standard it should do it), but can't you just use ConnectProperty("Sessions", ....) ?

77 + nux::Property<bool> have_other_open_sessions;

nux::ROProperty maybe?

Finally, here you're showing the message before the "Hi, ..."; it looks weird to me.
Have you checked with a designer? It doesn't look that good here.

BTW, since the string we're adding is already included into the langpack for unity-greeter, I think we don't need an UIFe for that.

review: Needs Information
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> 14 + dm_proxy_->Connect("SessionAdded", sigc::hide(sigc::mem_fun(this,
> &Impl::UpdateHaveOtherOpenSessions)));
> 15 + dm_proxy_->Connect("SessionRemoved", sigc::hide(sigc::mem_fun(this,
> &Impl::UpdateHaveOtherOpenSessions)));
>
> I didn't check if the proxy correctly emits the PropertyChanged signal (as per
> the standard it should do it), but can't you just use
> ConnectProperty("Sessions", ....) ?
>

No we can't. I tried and didn't work.

> 77 + nux::Property<bool> have_other_open_sessions;
>
> nux::ROProperty maybe?

Oh yeah, i will updated it.

>
> Finally, here you're showing the message before the "Hi, ..."; it looks weird
> to me.
> Have you checked with a designer? It doesn't look that good here.
>
> BTW, since the string we're adding is already included into the langpack for
> unity-greeter, I think we don't need an UIFe for that.

No, but it's the same design of unity-greeter.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

Done!!

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

String updated.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'UnityCore/GnomeSessionManager.cpp'
--- UnityCore/GnomeSessionManager.cpp 2014-04-10 04:52:59 +0000
+++ UnityCore/GnomeSessionManager.cpp 2014-05-09 16:08:54 +0000
@@ -85,6 +85,7 @@
85 , can_hibernate_(false)85 , can_hibernate_(false)
86 , pending_action_(shell::Action::NONE)86 , pending_action_(shell::Action::NONE)
87 , shell_server_(test_mode_ ? testing::DBUS_NAME : shell::DBUS_NAME)87 , shell_server_(test_mode_ ? testing::DBUS_NAME : shell::DBUS_NAME)
88 , open_sessions_(0)
88{89{
89 shell_server_.AddObjects(shell::INTROSPECTION_XML, shell::DBUS_OBJECT_PATH);90 shell_server_.AddObjects(shell::INTROSPECTION_XML, shell::DBUS_OBJECT_PATH);
90 shell_object_ = shell_server_.GetObject(shell::DBUS_INTERFACE);91 shell_object_ = shell_server_.GetObject(shell::DBUS_INTERFACE);
@@ -126,6 +127,22 @@
126 });127 });
127 }128 }
128129
130 {
131 dm_proxy_ = std::make_shared<glib::DBusProxy>("org.freedesktop.DisplayManager",
132 "/org/freedesktop/DisplayManager",
133 "org.freedesktop.DisplayManager",
134 G_BUS_TYPE_SYSTEM);
135
136 dm_proxy_->Connect("SessionAdded", sigc::hide(sigc::mem_fun(this, &Impl::UpdateHaveOtherOpenSessions)));
137 dm_proxy_->Connect("SessionRemoved", sigc::hide(sigc::mem_fun(this, &Impl::UpdateHaveOtherOpenSessions)));
138
139 UpdateHaveOtherOpenSessions();
140
141 manager_->have_other_open_sessions.SetGetterFunction([this]() {
142 return open_sessions_ > 1;
143 });
144 }
145
129 CallLogindMethod("CanHibernate", nullptr, [this] (GVariant* variant, glib::Error const& err) {146 CallLogindMethod("CanHibernate", nullptr, [this] (GVariant* variant, glib::Error const& err) {
130 if (err)147 if (err)
131 {148 {
@@ -422,6 +439,21 @@
422 prompt ? manager_->prompt_lock_requested.emit() : manager_->lock_requested.emit();439 prompt ? manager_->prompt_lock_requested.emit() : manager_->lock_requested.emit();
423}440}
424441
442void GnomeManager::Impl::UpdateHaveOtherOpenSessions()
443{
444 dm_proxy_->GetProperty("Sessions", [this](GVariant* variant) {
445 GVariantIter *sessions;
446 g_variant_get(variant, "ao", &sessions);
447 int open_sessions = g_variant_iter_n_children(sessions);
448
449 if (open_sessions_ != open_sessions)
450 {
451 open_sessions_ = open_sessions;
452 manager_->have_other_open_sessions.changed.emit(open_sessions_);
453 }
454 });
455}
456
425// Public implementation457// Public implementation
426458
427GnomeManager::GnomeManager()459GnomeManager::GnomeManager()
428460
=== modified file 'UnityCore/GnomeSessionManagerImpl.h'
--- UnityCore/GnomeSessionManagerImpl.h 2014-04-09 15:17:46 +0000
+++ UnityCore/GnomeSessionManagerImpl.h 2014-05-09 16:08:54 +0000
@@ -61,6 +61,7 @@
61 void CallLogindMethod(std::string const& method, GVariant* parameters = nullptr, glib::DBusProxy::CallFinishedCallback const& cb = nullptr);61 void CallLogindMethod(std::string const& method, GVariant* parameters = nullptr, glib::DBusProxy::CallFinishedCallback const& cb = nullptr);
62 void CallConsoleKitMethod(std::string const& method, GVariant* parameters = nullptr);62 void CallConsoleKitMethod(std::string const& method, GVariant* parameters = nullptr);
63 bool InteractiveMode();63 bool InteractiveMode();
64 void UpdateHaveOtherOpenSessions();
6465
65 GnomeManager* manager_;66 GnomeManager* manager_;
66 bool test_mode_;67 bool test_mode_;
@@ -73,6 +74,9 @@
73 glib::DBusObject::Ptr shell_object_;74 glib::DBusObject::Ptr shell_object_;
74 glib::DBusProxy::Ptr login_proxy_;75 glib::DBusProxy::Ptr login_proxy_;
75 glib::DBusProxy::Ptr presence_proxy_;76 glib::DBusProxy::Ptr presence_proxy_;
77 glib::DBusProxy::Ptr dm_proxy_;
78
79 int open_sessions_;
76};80};
7781
78} // namespace session82} // namespace session
7983
=== modified file 'UnityCore/SessionManager.h'
--- UnityCore/SessionManager.h 2014-04-10 04:47:58 +0000
+++ UnityCore/SessionManager.h 2014-05-09 16:08:54 +0000
@@ -23,6 +23,8 @@
23#include <sigc++/sigc++.h>23#include <sigc++/sigc++.h>
24#include <memory>24#include <memory>
2525
26#include <NuxCore/Property.h>
27
26namespace unity28namespace unity
27{29{
28namespace session30namespace session
@@ -36,6 +38,8 @@
36 Manager() = default;38 Manager() = default;
37 virtual ~Manager() = default;39 virtual ~Manager() = default;
3840
41 nux::ROProperty<bool> have_other_open_sessions;
42
39 virtual std::string RealName() const = 0;43 virtual std::string RealName() const = 0;
40 virtual std::string UserName() const = 0;44 virtual std::string UserName() const = 0;
41 virtual std::string HostName() const = 0;45 virtual std::string HostName() const = 0;
4246
=== modified file 'shutdown/SessionView.cpp'
--- shutdown/SessionView.cpp 2013-11-20 21:39:40 +0000
+++ shutdown/SessionView.cpp 2014-05-09 16:08:54 +0000
@@ -79,6 +79,7 @@
79 GetBoundingArea()->mouse_click.connect([this] (int, int, unsigned long, unsigned long) { request_close.emit(); });79 GetBoundingArea()->mouse_click.connect([this] (int, int, unsigned long, unsigned long) { request_close.emit(); });
8080
81 have_inhibitors.changed.connect(sigc::hide(sigc::mem_fun(this, &View::UpdateText)));81 have_inhibitors.changed.connect(sigc::hide(sigc::mem_fun(this, &View::UpdateText)));
82 manager_->have_other_open_sessions.changed.connect(sigc::hide(sigc::mem_fun(this, &View::UpdateText)));
8283
83 mode.SetSetterFunction([this] (Mode& target, Mode new_mode) {84 mode.SetSetterFunction([this] (Mode& target, Mode new_mode) {
84 if (new_mode == Mode::SHUTDOWN && !manager_->CanShutdown())85 if (new_mode == Mode::SHUTDOWN && !manager_->CanShutdown())
@@ -104,24 +105,32 @@
104105
105void View::UpdateText()106void View::UpdateText()
106{107{
107 const char* message = nullptr;108 std::string message;
109 std::string other_users_msg;
108 auto const& real_name = manager_->RealName();110 auto const& real_name = manager_->RealName();
109 auto const& name = (real_name.empty() ? manager_->UserName() : real_name);111 auto const& name = (real_name.empty() ? manager_->UserName() : real_name);
110112
113 other_users_msg = _("Other users are logged in. Restarting or shutting down will close their open applications and may cause them to lose work.\n\n");
114
111 if (mode() == Mode::SHUTDOWN)115 if (mode() == Mode::SHUTDOWN)
112 {116 {
113 title_->SetText(_("Shut Down"));117 title_->SetText(_("Shut Down"));
114 title_->SetVisible(true);118 title_->SetVisible(true);
115119
120 if (manager_->have_other_open_sessions())
121 {
122 message += other_users_msg;
123 }
124
116 if (have_inhibitors())125 if (have_inhibitors())
117 {126 {
118 message = _("Hi %s, you have open files that you might want to save " \127 message += _("Hi %s, you have open files that you might want to save " \
119 "before shutting down. Are you sure you want to continue?");128 "before shutting down. Are you sure you want to continue?");
120 }129 }
121 else130 else
122 {131 {
123 message = _("Goodbye, %s. Are you sure you want to close all programs " \132 message += _("Goodbye, %s. Are you sure you want to close all programs " \
124 "and shut down the computer?");133 "and shut down the computer?");
125 }134 }
126 }135 }
127 else if (mode() == Mode::LOGOUT)136 else if (mode() == Mode::LOGOUT)
@@ -144,27 +153,32 @@
144 {153 {
145 title_->SetVisible(false);154 title_->SetVisible(false);
146155
156 if (manager_->have_other_open_sessions())
157 {
158 message += other_users_msg;
159 }
160
147 if (have_inhibitors())161 if (have_inhibitors())
148 {162 {
149 if (buttons_layout_->GetChildren().size() > 3)163 if (buttons_layout_->GetChildren().size() > 3)
150 {164 {
151 // We have enough buttons to show the message without a new line.165 // We have enough buttons to show the message without a new line.
152 message = _("Hi %s, you have open files you might want to save. " \166 message += _("Hi %s, you have open files you might want to save. " \
153 "Would you like to…");167 "Would you like to…");
154 }168 }
155 else169 else
156 {170 {
157 message = _("Hi %s, you have open files you might want to save.\n" \171 message += _("Hi %s, you have open files you might want to save.\n" \
158 "Would you like to…");172 "Would you like to…");
159 }173 }
160 }174 }
161 else175 else
162 {176 {
163 message = _("Goodbye, %s. Would you like to…");177 message += _("Goodbye, %s. Would you like to…");
164 }178 }
165 }179 }
166180
167 subtitle_->SetText(glib::String(g_strdup_printf(message, name.c_str())).Str());181 subtitle_->SetText(glib::String(g_strdup_printf(message.c_str(), name.c_str())).Str());
168}182}
169183
170void View::Populate()184void View::Populate()