Merge lp:~3v1n0/unity/session-labels-fix into lp:unity

[Marco Trevisan (Treviño)]

SessionView: rename the Logout and Shutdown buttons as requested

Plus some code cleanup, using button-type to generate them instead of magic strings

Changed the labels of the session dialogs as requested by design. Also moved the textures/labels code generation to the SessionButton itself, that now can be generated by an Action type.

Added new tests, updating the old ones.

Screenshots:
http://ubuntuone.com/45dTsdpSJOKB1HEpVfyM69
http://ubuntuone.com/2DQ9uhmKOLLdJUCi1byYtQ

[Andrea Azzarone (azzar1)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'po/POTFILES.in'
--- po/POTFILES.in	2013-03-07 22:48:41 +0000
+++ po/POTFILES.in	2013-03-19 15:04:28 +0000
@@ -33,6 +33,7 @@
 shortcuts/ShortcutView.cpp
 shortcuts/CompizShortcutModeller.cpp
 shutdown/SessionView.cpp
+shutdown/SessionButton.cpp
 services/panel-service.c
 unity-shared/DashStyle.cpp
 unity-shared/SearchBar.cpp

=== modified file 'shutdown/SessionButton.cpp'
--- shutdown/SessionButton.cpp	2013-03-07 19:26:40 +0000
+++ shutdown/SessionButton.cpp	2013-03-19 15:04:28 +0000
@@ -22,7 +22,7 @@
 
 #include <Nux/VLayout.h>
 #include <UnityCore/Variant.h>
-
+#include <glib/gi18n-lib.h>
 
 namespace unity
 {
@@ -38,15 +38,47 @@
 
 NUX_IMPLEMENT_OBJECT_TYPE(Button);
 
-Button::Button(std::string const& label, std::string const& texture_name, NUX_FILE_LINE_DECL)
+Button::Button(Action action, NUX_FILE_LINE_DECL)
   : nux::View(NUX_FILE_LINE_PARAM)
   , highlighted(false)
+  , action([this] { return action_; })
   , label([this] { return label_view_->GetText(); })
+  , action_(action)
 {
   SetAcceptKeyNavFocusOnMouseDown(false);
   SetAcceptKeyNavFocusOnMouseEnter(true);
 
-  std::string texture_prefix = PKGDATADIR"/" + texture_name;
+  std::string texture_prefix = PKGDATADIR"/";
+  std::string label;
+
+  switch (action_)
+  {
+    case Action::LOCK:
+      texture_prefix += "lockscreen";
+      label = _("Lock");
+      break;
+    case Action::LOGOUT:
+      texture_prefix += "logout";
+      label = _("Log Out");
+      break;
+    case Action::SUSPEND:
+      texture_prefix += "suspend";
+      label = _("Suspend");
+      break;
+    case Action::HIBERNATE:
+      texture_prefix += "hibernate";
+      label = _("Hibernate");
+      break;
+    case Action::SHUTDOWN:
+      texture_prefix += "shutdown";
+      label = _("Shut Down");
+      break;
+    case Action::REBOOT:
+      texture_prefix += "restart";
+      label = _("Restart");
+      break;
+  }
+
   normal_tex_.Adopt(nux::CreateTexture2DFromFile((texture_prefix + ".png").c_str(), -1, true));
   highlight_tex_.Adopt(nux::CreateTexture2DFromFile((texture_prefix + "_highlight.png").c_str(), -1, true));
 

=== modified file 'shutdown/SessionButton.h'
--- shutdown/SessionButton.h	2013-03-07 19:26:40 +0000
+++ shutdown/SessionButton.h	2013-03-19 15:04:28 +0000
@@ -37,9 +37,20 @@
 {
   NUX_DECLARE_OBJECT_TYPE(Button, nux::View);
 public:
-  Button(std::string const& label, std::string const& texture_name, NUX_FILE_LINE_PROTO);
+  enum class Action
+  {
+    LOCK,
+    LOGOUT,
+    SUSPEND,
+    HIBERNATE,
+    SHUTDOWN,
+    REBOOT
+  };
+
+  Button(Action, NUX_FILE_LINE_PROTO);
 
   nux::Property<bool> highlighted;
+  nux::ROProperty<Action> action;
   nux::ROProperty<std::string> label;
 
   sigc::signal<void> activated;
@@ -54,6 +65,7 @@
 private:
   friend class TestSessionButton;
 
+  Action action_;
   IconTexture* image_view_;
   StaticCairoText* label_view_;
   nux::ObjectPtr<nux::BaseTexture> normal_tex_;

=== modified file 'shutdown/SessionView.cpp'
--- shutdown/SessionView.cpp	2013-03-11 07:17:56 +0000
+++ shutdown/SessionView.cpp	2013-03-19 15:04:28 +0000
@@ -173,11 +173,11 @@
 
   if (mode() == Mode::LOGOUT)
   {
-    auto* button = new Button(_("Lock"), "lockscreen", NUX_TRACKER_LOCATION);
+    auto* button = new Button(Button::Action::LOCK, NUX_TRACKER_LOCATION);
     button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::LockScreen));
     AddButton(button);
 
-    button = new Button(_("Logout"), "logout", NUX_TRACKER_LOCATION);
+    button = new Button(Button::Action::LOGOUT, NUX_TRACKER_LOCATION);
     button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::Logout));
     AddButton(button);
   }
@@ -185,20 +185,20 @@
   {
     if (mode() == Mode::FULL)
     {
-      auto* button = new Button(_("Lock"), "lockscreen", NUX_TRACKER_LOCATION);
+      auto* button = new Button(Button::Action::LOCK, NUX_TRACKER_LOCATION);
       button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::LockScreen));
       AddButton(button);
 
       if (manager_->CanSuspend())
       {
-        button = new Button(_("Suspend"), "suspend", NUX_TRACKER_LOCATION);
+        button = new Button(Button::Action::SUSPEND, NUX_TRACKER_LOCATION);
         button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::Suspend));
         AddButton(button);
       }
 
       if (manager_->CanHibernate())
       {
-        button = new Button(_("Hibernate"), "hibernate", NUX_TRACKER_LOCATION);
+        button = new Button(Button::Action::HIBERNATE, NUX_TRACKER_LOCATION);
         button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::Hibernate));
         AddButton(button);
       }
@@ -206,17 +206,17 @@
 
     if (manager_->CanShutdown())
     {
-      auto* button = new Button(_("Shutdown"), "shutdown", NUX_TRACKER_LOCATION);
+      auto* button = new Button(Button::Action::SHUTDOWN, NUX_TRACKER_LOCATION);
       button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::Shutdown));
       AddButton(button);
 
-      button = new Button(_("Restart"), "restart", NUX_TRACKER_LOCATION);
+      button = new Button(Button::Action::REBOOT, NUX_TRACKER_LOCATION);
       button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::Reboot));
       AddButton(button);
     }
     else if (mode() == Mode::FULL)
     {
-      auto* button = new Button(_("Logout"), "logout", NUX_TRACKER_LOCATION);
+      auto* button = new Button(Button::Action::LOGOUT, NUX_TRACKER_LOCATION);
       button->activated.connect(sigc::mem_fun(manager_.get(), &Manager::Logout));
       AddButton(button);
     }

=== modified file 'tests/test_session_button.cpp'
--- tests/test_session_button.cpp	2013-03-07 18:21:58 +0000
+++ tests/test_session_button.cpp	2013-03-19 15:04:28 +0000
@@ -31,7 +31,7 @@
 {
   struct ButtonWrap : Button
   {
-    ButtonWrap() : Button("ButtonLabel", "hibernate") {}
+    ButtonWrap() : Button(Action::LOCK) {}
 
     using Button::AcceptKeyNavFocusOnMouseEnter;
     using Button::AcceptKeyNavFocusOnMouseDown;
@@ -46,8 +46,8 @@
 
 TEST_F(TestSessionButton, Construct)
 {
+  EXPECT_EQ(button.action(), Button::Action::LOCK);
   EXPECT_FALSE(button.highlighted());
-  EXPECT_EQ(button.label(), "ButtonLabel");
   EXPECT_TRUE(button.AcceptKeyNavFocusOnMouseEnter());
   EXPECT_FALSE(button.AcceptKeyNavFocusOnMouseDown());
 }
@@ -120,5 +120,50 @@
   EXPECT_TRUE(activated);
 }
 
+// Action typed buttons tests
+
+struct ActionButton : public testing::TestWithParam<Button::Action> {
+  ActionButton()
+    : button(GetParam())
+  {}
+
+  std::string GetExpectedLabel()
+  {
+    switch (GetParam())
+    {
+      case Button::Action::LOCK:
+        return "Lock";
+      case Button::Action::LOGOUT:
+        return "Log Out";
+      case Button::Action::SUSPEND:
+        return "Suspend";
+      case Button::Action::HIBERNATE:
+        return "Hibernate";
+      case Button::Action::SHUTDOWN:
+        return "Shut Down";
+      case Button::Action::REBOOT:
+        return "Restart";
+    }
+
+    return "";
+  }
+
+  Button button;
+};
+
+INSTANTIATE_TEST_CASE_P(TestSessionButtonTypes, ActionButton,
+  testing::Values(Button::Action::LOCK, Button::Action::LOGOUT, Button::Action::SUSPEND,
+                  Button::Action::HIBERNATE, Button::Action::SHUTDOWN, Button::Action::REBOOT));
+
+TEST_P(/*TestSessionButtonTypes*/ActionButton, Label)
+{
+  EXPECT_EQ(button.label(), GetExpectedLabel());
+}
+
+TEST_P(/*TestSessionButtonTypes*/ActionButton, Action)
+{
+  EXPECT_EQ(button.action(), GetParam());
+}
+
 } // session
 } // unity
\ No newline at end of file

=== modified file 'tests/test_session_view.cpp'
--- tests/test_session_view.cpp	2013-03-07 22:42:29 +0000
+++ tests/test_session_view.cpp	2013-03-19 15:04:28 +0000
@@ -60,11 +60,11 @@
       return buttons;
     }
 
-    Button* GetButtonByLabel(std::string const& label) const
+    Button* GetButtonByAction(Button::Action action) const
     {
       for (auto const& button : GetButtons())
       {
-        if (button->label() == label)
+        if (button->action() == action)
           return button;
       }
 
@@ -129,29 +129,29 @@
   ON_CALL(*manager, CanHibernate()).WillByDefault(testing::Return(true));
   view.mode.changed.emit(View::Mode::FULL);
 
-  EXPECT_EQ(view.GetButtonByLabel("Logout"), nullptr);
-  EXPECT_NE(view.GetButtonByLabel("Lock"), nullptr);
-  EXPECT_NE(view.GetButtonByLabel("Suspend"), nullptr);
-  EXPECT_NE(view.GetButtonByLabel("Hibernate"), nullptr);
-  EXPECT_NE(view.GetButtonByLabel("Shutdown"), nullptr);
-  EXPECT_NE(view.GetButtonByLabel("Restart"), nullptr);
+  EXPECT_EQ(view.GetButtonByAction(Button::Action::LOGOUT), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::LOCK), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::SUSPEND), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::HIBERNATE), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::SHUTDOWN), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::REBOOT), nullptr);
 
   ON_CALL(*manager, CanShutdown()).WillByDefault(testing::Return(false));
   view.mode.changed.emit(View::Mode::FULL);
 
-  EXPECT_NE(view.GetButtonByLabel("Logout"), nullptr);
-  EXPECT_EQ(view.GetButtonByLabel("Shutdown"), nullptr);
-  EXPECT_EQ(view.GetButtonByLabel("Restart"), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::LOGOUT), nullptr);
+  EXPECT_EQ(view.GetButtonByAction(Button::Action::SHUTDOWN), nullptr);
+  EXPECT_EQ(view.GetButtonByAction(Button::Action::REBOOT), nullptr);
 
   ON_CALL(*manager, CanSuspend()).WillByDefault(testing::Return(false));
   view.mode.changed.emit(View::Mode::FULL);
 
-  EXPECT_EQ(view.GetButtonByLabel("Suspend"), nullptr);
+  EXPECT_EQ(view.GetButtonByAction(Button::Action::SUSPEND), nullptr);
 
   ON_CALL(*manager, CanHibernate()).WillByDefault(testing::Return(false));
   view.mode.changed.emit(View::Mode::FULL);
 
-  EXPECT_EQ(view.GetButtonByLabel("Hibernate"), nullptr);
+  EXPECT_EQ(view.GetButtonByAction(Button::Action::HIBERNATE), nullptr);
 }
 
 TEST_F(TestSessionView, ShutdownModeButtons)
@@ -160,8 +160,8 @@
   view.mode = View::Mode::SHUTDOWN;
 
   EXPECT_EQ(view.GetButtons().size(), 2);
-  EXPECT_NE(view.GetButtonByLabel("Shutdown"), nullptr);
-  EXPECT_NE(view.GetButtonByLabel("Restart"), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::SHUTDOWN), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::REBOOT), nullptr);
 }
 
 TEST_F(TestSessionView, LogoutModeButtons)
@@ -169,8 +169,8 @@
   view.mode = View::Mode::LOGOUT;
 
   EXPECT_EQ(view.GetButtons().size(), 2);
-  EXPECT_NE(view.GetButtonByLabel("Logout"), nullptr);
-  EXPECT_NE(view.GetButtonByLabel("Lock"), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::LOGOUT), nullptr);
+  EXPECT_NE(view.GetButtonByAction(Button::Action::LOCK), nullptr);
 }
 
 TEST_F(TestSessionView, FullModeTitle)
@@ -198,7 +198,7 @@
   bool request_hide = false;
   view.request_hide.connect([&request_hide] { request_hide = true; });
 
-  auto button = view.GetButtonByLabel("Lock");
+  auto button = view.GetButtonByAction(Button::Action::LOCK);
   ASSERT_NE(button, nullptr);
   button->activated.emit();
 
@@ -207,7 +207,7 @@
 
 TEST_F(TestSessionView, ButtonsActivateDeselectButton)
 {
-  auto button = view.GetButtonByLabel("Lock");
+  auto button = view.GetButtonByAction(Button::Action::LOCK);
   ASSERT_NE(button, nullptr);
   button->highlighted = true;
   button->activated.emit();
@@ -218,7 +218,7 @@
 TEST_F(TestSessionView, LockButtonActivateLocks)
 {
   EXPECT_CALL(*manager, LockScreen());
-  auto button = view.GetButtonByLabel("Lock");
+  auto button = view.GetButtonByAction(Button::Action::LOCK);
   ASSERT_NE(button, nullptr);
   button->activated.emit();
 }
@@ -227,7 +227,7 @@
 {
   view.mode = View::Mode::LOGOUT;
   EXPECT_CALL(*manager, Logout());
-  auto button = view.GetButtonByLabel("Logout");
+  auto button = view.GetButtonByAction(Button::Action::LOGOUT);
   ASSERT_NE(button, nullptr);
   button->activated.emit();
 }
@@ -238,7 +238,7 @@
   view.mode.changed.emit(View::Mode::FULL);
 
   EXPECT_CALL(*manager, Suspend());
-  auto button = view.GetButtonByLabel("Suspend");
+  auto button = view.GetButtonByAction(Button::Action::SUSPEND);
   ASSERT_NE(button, nullptr);
   button->activated.emit();
 }
@@ -249,7 +249,7 @@
   view.mode.changed.emit(View::Mode::FULL);
 
   EXPECT_CALL(*manager, Hibernate());
-  auto button = view.GetButtonByLabel("Hibernate");
+  auto button = view.GetButtonByAction(Button::Action::HIBERNATE);
   ASSERT_NE(button, nullptr);
   button->activated.emit();
 }
@@ -260,7 +260,7 @@
   view.mode = View::Mode::SHUTDOWN;
 
   EXPECT_CALL(*manager, Shutdown());
-  auto button = view.GetButtonByLabel("Shutdown");
+  auto button = view.GetButtonByAction(Button::Action::SHUTDOWN);
   ASSERT_NE(button, nullptr);
   button->activated.emit();
 }
@@ -271,7 +271,7 @@
   view.mode = View::Mode::SHUTDOWN;
 
   EXPECT_CALL(*manager, Reboot());
-  auto button = view.GetButtonByLabel("Restart");
+  auto button = view.GetButtonByAction(Button::Action::REBOOT);
   ASSERT_NE(button, nullptr);
   button->activated.emit();
 }