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

Proposed by Andrea Azzarone on 2014-04-11
Status: Merged
Approved by: Marco Trevisan (Treviño) on 2014-04-16
Approved revision: 3788
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 continuous-integration Approve on 2014-04-16
Marco Trevisan (Treviño) Approve on 2014-04-16
Brandon Schaefer (community) 2014-04-11 Approve on 2014-04-11
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.
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
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
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
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.

lp:~azzar1/unity/lp-1281058 updated on 2014-04-16
3786. By Andrea Azzarone on 2014-04-16

Add an extra /n.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~azzar1/unity/lp-1281058 updated on 2014-04-16
3787. By Andrea Azzarone on 2014-04-16

Use a ROProperty.

Andrea Azzarone (azzar1) wrote :

Done!!

lp:~azzar1/unity/lp-1281058 updated on 2014-04-16
3788. By Andrea Azzarone on 2014-04-16

Send the changed event.

review: Approve
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~azzar1/unity/lp-1281058 updated on 2014-05-09
3789. By Andrea Azzarone on 2014-05-09

Merge trunk.

3790. By Andrea Azzarone on 2014-05-09

Update string.

Andrea Azzarone (azzar1) wrote :

String updated.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'UnityCore/GnomeSessionManager.cpp'
2--- UnityCore/GnomeSessionManager.cpp 2014-04-10 04:52:59 +0000
3+++ UnityCore/GnomeSessionManager.cpp 2014-05-09 16:08:54 +0000
4@@ -85,6 +85,7 @@
5 , can_hibernate_(false)
6 , pending_action_(shell::Action::NONE)
7 , shell_server_(test_mode_ ? testing::DBUS_NAME : shell::DBUS_NAME)
8+ , open_sessions_(0)
9 {
10 shell_server_.AddObjects(shell::INTROSPECTION_XML, shell::DBUS_OBJECT_PATH);
11 shell_object_ = shell_server_.GetObject(shell::DBUS_INTERFACE);
12@@ -126,6 +127,22 @@
13 });
14 }
15
16+ {
17+ dm_proxy_ = std::make_shared<glib::DBusProxy>("org.freedesktop.DisplayManager",
18+ "/org/freedesktop/DisplayManager",
19+ "org.freedesktop.DisplayManager",
20+ G_BUS_TYPE_SYSTEM);
21+
22+ dm_proxy_->Connect("SessionAdded", sigc::hide(sigc::mem_fun(this, &Impl::UpdateHaveOtherOpenSessions)));
23+ dm_proxy_->Connect("SessionRemoved", sigc::hide(sigc::mem_fun(this, &Impl::UpdateHaveOtherOpenSessions)));
24+
25+ UpdateHaveOtherOpenSessions();
26+
27+ manager_->have_other_open_sessions.SetGetterFunction([this]() {
28+ return open_sessions_ > 1;
29+ });
30+ }
31+
32 CallLogindMethod("CanHibernate", nullptr, [this] (GVariant* variant, glib::Error const& err) {
33 if (err)
34 {
35@@ -422,6 +439,21 @@
36 prompt ? manager_->prompt_lock_requested.emit() : manager_->lock_requested.emit();
37 }
38
39+void GnomeManager::Impl::UpdateHaveOtherOpenSessions()
40+{
41+ dm_proxy_->GetProperty("Sessions", [this](GVariant* variant) {
42+ GVariantIter *sessions;
43+ g_variant_get(variant, "ao", &sessions);
44+ int open_sessions = g_variant_iter_n_children(sessions);
45+
46+ if (open_sessions_ != open_sessions)
47+ {
48+ open_sessions_ = open_sessions;
49+ manager_->have_other_open_sessions.changed.emit(open_sessions_);
50+ }
51+ });
52+}
53+
54 // Public implementation
55
56 GnomeManager::GnomeManager()
57
58=== modified file 'UnityCore/GnomeSessionManagerImpl.h'
59--- UnityCore/GnomeSessionManagerImpl.h 2014-04-09 15:17:46 +0000
60+++ UnityCore/GnomeSessionManagerImpl.h 2014-05-09 16:08:54 +0000
61@@ -61,6 +61,7 @@
62 void CallLogindMethod(std::string const& method, GVariant* parameters = nullptr, glib::DBusProxy::CallFinishedCallback const& cb = nullptr);
63 void CallConsoleKitMethod(std::string const& method, GVariant* parameters = nullptr);
64 bool InteractiveMode();
65+ void UpdateHaveOtherOpenSessions();
66
67 GnomeManager* manager_;
68 bool test_mode_;
69@@ -73,6 +74,9 @@
70 glib::DBusObject::Ptr shell_object_;
71 glib::DBusProxy::Ptr login_proxy_;
72 glib::DBusProxy::Ptr presence_proxy_;
73+ glib::DBusProxy::Ptr dm_proxy_;
74+
75+ int open_sessions_;
76 };
77
78 } // namespace session
79
80=== modified file 'UnityCore/SessionManager.h'
81--- UnityCore/SessionManager.h 2014-04-10 04:47:58 +0000
82+++ UnityCore/SessionManager.h 2014-05-09 16:08:54 +0000
83@@ -23,6 +23,8 @@
84 #include <sigc++/sigc++.h>
85 #include <memory>
86
87+#include <NuxCore/Property.h>
88+
89 namespace unity
90 {
91 namespace session
92@@ -36,6 +38,8 @@
93 Manager() = default;
94 virtual ~Manager() = default;
95
96+ nux::ROProperty<bool> have_other_open_sessions;
97+
98 virtual std::string RealName() const = 0;
99 virtual std::string UserName() const = 0;
100 virtual std::string HostName() const = 0;
101
102=== modified file 'shutdown/SessionView.cpp'
103--- shutdown/SessionView.cpp 2013-11-20 21:39:40 +0000
104+++ shutdown/SessionView.cpp 2014-05-09 16:08:54 +0000
105@@ -79,6 +79,7 @@
106 GetBoundingArea()->mouse_click.connect([this] (int, int, unsigned long, unsigned long) { request_close.emit(); });
107
108 have_inhibitors.changed.connect(sigc::hide(sigc::mem_fun(this, &View::UpdateText)));
109+ manager_->have_other_open_sessions.changed.connect(sigc::hide(sigc::mem_fun(this, &View::UpdateText)));
110
111 mode.SetSetterFunction([this] (Mode& target, Mode new_mode) {
112 if (new_mode == Mode::SHUTDOWN && !manager_->CanShutdown())
113@@ -104,24 +105,32 @@
114
115 void View::UpdateText()
116 {
117- const char* message = nullptr;
118+ std::string message;
119+ std::string other_users_msg;
120 auto const& real_name = manager_->RealName();
121 auto const& name = (real_name.empty() ? manager_->UserName() : real_name);
122
123+ 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");
124+
125 if (mode() == Mode::SHUTDOWN)
126 {
127 title_->SetText(_("Shut Down"));
128 title_->SetVisible(true);
129
130+ if (manager_->have_other_open_sessions())
131+ {
132+ message += other_users_msg;
133+ }
134+
135 if (have_inhibitors())
136 {
137- message = _("Hi %s, you have open files that you might want to save " \
138- "before shutting down. Are you sure you want to continue?");
139+ message += _("Hi %s, you have open files that you might want to save " \
140+ "before shutting down. Are you sure you want to continue?");
141 }
142 else
143 {
144- message = _("Goodbye, %s. Are you sure you want to close all programs " \
145- "and shut down the computer?");
146+ message += _("Goodbye, %s. Are you sure you want to close all programs " \
147+ "and shut down the computer?");
148 }
149 }
150 else if (mode() == Mode::LOGOUT)
151@@ -144,27 +153,32 @@
152 {
153 title_->SetVisible(false);
154
155+ if (manager_->have_other_open_sessions())
156+ {
157+ message += other_users_msg;
158+ }
159+
160 if (have_inhibitors())
161 {
162 if (buttons_layout_->GetChildren().size() > 3)
163 {
164 // We have enough buttons to show the message without a new line.
165- 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. " \
167 "Would you like to…");
168 }
169 else
170 {
171- message = _("Hi %s, you have open files you might want to save.\n" \
172+ message += _("Hi %s, you have open files you might want to save.\n" \
173 "Would you like to…");
174 }
175 }
176 else
177 {
178- message = _("Goodbye, %s. Would you like to…");
179+ message += _("Goodbye, %s. Would you like to…");
180 }
181 }
182
183- subtitle_->SetText(glib::String(g_strdup_printf(message, name.c_str())).Str());
184+ subtitle_->SetText(glib::String(g_strdup_printf(message.c_str(), name.c_str())).Str());
185 }
186
187 void View::Populate()