Merge lp:~mblayman/entertainer/translate-events into lp:entertainer

[Matt Layman]

Screen can now use an callback to return to their previous screen.

[Matt Layman (mblayman)]

I probably should have proposed this a long time ago. I did some refactoring a while back to remove the stage usage from the ScreenHistory and create the corresponding callback to match up with "move_to_new_screen".

In light of the fact that Samuel has now also created a screen_back callback in the back_button branch, I'm proposing this so we could do a comparison of the two implementations.

[Matt Layman (mblayman)]

I should add that the name of this branch is a complete misnomer. I was planning to do something else with this branch and the refactoring that I started out with was just something I did during my initial analysis.

[Samuel Buffet (samuel-buffet)]

Matt,

I'm okay with the 2 goals of this branch.

This branch will lead me to change my callback variable name in "back_button" but there's no compatibility issues.
In "back_button" it's just handled on the Screen Class and wrapped in a "screen_back" method because I need that on every screen.

Bellow some minor comments :

=== modified file 'entertainerlib/frontend/gui/screen_history.py'
--- entertainerlib/frontend/gui/screen_history.py 2009-05-06 03:40:22 +0000
+++ entertainerlib/frontend/gui/screen_history.py 2009-06-02 01:16:03 +0000
@@ -1,69 +1,35 @@
 # Copyright (c) 2009 Entertainer Developers - See COPYING - GPLv2
 '''Stack container for screen objects'''

> def is_empty(self):
>- """
>- Is screen history empty.
>- @return boolean value. True if history is empty, otherwise False.
>- """
>+ '''Is screen history empty'''
> if len(self.screens) == 0:
> return True
> else:
> return False
>

Maybe a @property makes sense here instead of a method call to fit with the recent changes we've made.
The docstring needs a full stop.

Thanks,
Samuel-

review: Approve

[lp:~mblayman/entertainer/translate-events]

361. By Matt Layman on 2009-06-03

is_empty is now a property.

362. By Matt Layman on 2009-06-03

Fixed the EntertainerTest name.

[Matt Layman (mblayman)]

Samuel,

> > def is_empty(self):
> >- """
> >- Is screen history empty.
> >- @return boolean value. True if history is empty, otherwise False.
> >- """
> >+ '''Is screen history empty'''
> > if len(self.screens) == 0:
> > return True
> > else:
> > return False
> >
>
> Maybe a @property makes sense here instead of a method call to fit with the
> recent changes we've made.
> The docstring needs a full stop.

is_empty is now a property (and I threw in a new test case to boot). Here is the partial diff:

matt@eden:~/repo/translate-events/entertainerlib/tests$ bzr diff -r -3..-1
=== modified file 'entertainerlib/frontend/gui/screen_history.py'
--- entertainerlib/frontend/gui/screen_history.py 2009-06-02 01:16:03 +0000
+++ entertainerlib/frontend/gui/screen_history.py 2009-06-03 01:54:03 +0000
@@ -26,8 +26,9 @@
             return None
         return self.screens.pop()

+ @property
     def is_empty(self):
- '''Is screen history empty'''
+ '''Return a boolean indicating if the screen history is empty.'''
         if len(self.screens) == 0:
             return True
         else:

=== modified file 'entertainerlib/frontend/gui/user_interface.py'
--- entertainerlib/frontend/gui/user_interface.py 2009-06-02 01:16:03 +0000
+++ entertainerlib/frontend/gui/user_interface.py 2009-06-03 01:54:03 +0000
@@ -351,7 +351,7 @@

     def _handle_navigate_back(self, event):
         '''Handle UserEvent.NAVIGATE_BACK.'''
- if not self.history.is_empty():
+ if not self.history.is_empty:
             self.move_to_previous_screen()

     def _handle_navigate_home(self, event):

=== added file 'entertainerlib/tests/test_screenhistory.py'
--- entertainerlib/tests/test_screenhistory.py 1970-01-01 00:00:00 +0000
+++ entertainerlib/tests/test_screenhistory.py 2009-06-03 01:54:03 +0000
@@ -0,0 +1,26 @@
+# Copyright (c) 2009 Entertainer Developers - See COPYING - GPLv2
+'''Tests ScreenHistory'''
+
+from entertainerlib.frontend.gui.screen_history import ScreenHistory
+from entertainerlib.frontend.gui.screens.screen import Screen
+from entertainerlib.tests import EntertainerTest
+
+class ScreenHistoryTest(EntertainerTest):
+ '''Test for entertainerlib.frontend.gui.screen_history'''
+
+ def setUp(self):
+ EntertainerTest.setUp(self)
+
+ self.screen_history = ScreenHistory(None)
+
+ def test_create(self):
+ '''Test correct ScreenHistory initialization.'''
+ self.assertTrue(isinstance(self.screen_history, ScreenHistory))
+
+ def test_is_empty(self):
+ '''Test the is_empty property.'''
+ self.assertTrue(self.screen_history.is_empty)
+ screen = Screen()
+ self.screen_history.add_screen(screen)
+ self.assertFalse(self.screen_history.is_empty)
+

=== modified file 'entertainerlib/frontend/gui/screen_history.py'
--- entertainerlib/frontend/gui/screen_history.py	2009-05-06 03:40:22 +0000
+++ entertainerlib/frontend/gui/screen_history.py	2009-06-02 01:16:03 +0000
@@ -1,69 +1,35 @@
 # Copyright (c) 2009 Entertainer Developers - See COPYING - GPLv2
 '''Stack container for screen objects'''
 
-class ScreenHistory:
-    """
-    ScreenHistory contains latest screens. History is a stack container.
-
-    This class is a simple container for Screen objects. It contains all
-    recently used screens and allows user to get previous screen from the
-    stack.
-    """
-
-    def __init__(self, stage, size):
-        """
-        Initialize screen history.
-        @param stage: Stage object
-        """
-        self.size = size      # How many screens we keep in memory
-        self.screens = []     # Screen stack (contains Screen objects)
-        self.stage = stage
-
-    def __len__(self):
-        return len(self.screens)
+from entertainerlib.utils.configuration import Configuration
+
+class ScreenHistory(object):
+    '''ScreenHistory contains the latest screens in is a stack.'''
+
+    def __init__(self, remove_from_stage_callback):
+        self.config = Configuration()
+        self.screens = []
+        self.remove_from_stage_callback = remove_from_stage_callback
 
     def add_screen(self, screen):
-        """
-        Add screen to the history. Push screen into screen stack.
-        @param screen: Screen object that is added to the history stack
-        """
-        if len(self.screens) < self.size:
+        '''Push the provided screen onto the history stack.'''
+        if len(self.screens) < self.config.history_size():
             self.screens.append(screen)
         else:
-            self.stage.remove(self.screens[0]) # Remove from stage
+            self.remove_from_stage_callback(self.screens[0])
             del self.screens[0] # Drop the oldest screen from the history
             self.screens.append(screen)
 
     def get_screen(self):
-        """
-        Get most recent screen from the history. Pops screen from screen stack
-        @return Screen object from history stack
-        """
+        '''Get the latest screen and pop it from the stack.'''
         if len(self.screens) == 0:
             return None
         return self.screens.pop()
 
     def is_empty(self):
-        """
-        Is screen history empty.
-        @return boolean value. True if history is empty, otherwise False.
-        """
+        '''Is screen history empty'''
         if len(self.screens) == 0:
             return True
         else:
             return False
 
-    def get_number_of_screens(self):
-        """
-        Get number of screens in history.
-        @return integer - Number of screens
-        """
-        return len(self.screens)
-
-    def get_all_screens(self):
-        """
-        Get all screens from the history. Latest screen is the last one.
-        @return List of Screen objects
-        """
-        return self.screens
-

=== modified file 'entertainerlib/frontend/gui/screens/factory.py'
--- entertainerlib/frontend/gui/screens/factory.py	2009-04-28 22:30:06 +0000
+++ entertainerlib/frontend/gui/screens/factory.py	2009-06-02 01:16:03 +0000
@@ -25,8 +25,8 @@
     '''Generate a screen based on the type provided.'''
 
     def __init__(self, feed_library, image_library, music_library,
-        video_library, media_player, screen_history,
-        move_to_new_screen_callback, change_screen_callback):
+        video_library, media_player, move_to_new_screen_callback,
+        move_to_previous_screen_callback):
         '''All the possible common things that a screen could use to initialize
         are factory instance variables. Any additional specific information
         needed for a screen will be provided in a dictionary that is received
@@ -37,9 +37,8 @@
         self.music_library = music_library
         self.video_library = video_library
         self.media_player = media_player
-        self.screen_history = screen_history
         self.move_to_new_screen_callback = move_to_new_screen_callback
-        self.change_screen_callback = change_screen_callback
+        self.move_to_previous_screen_callback = move_to_previous_screen_callback
 
     def generate_screen(self, screen_type, kwargs=None):
         '''Generate the proper screen by delegating to the proper generator.'''
@@ -145,8 +144,8 @@
 
     def _generate_question(self, kwargs):
         '''Generate a Question screen.'''
-        kwargs['change_screen_callback'] = self.change_screen_callback
-        kwargs['screen_history'] = self.screen_history
+        kwargs['move_to_previous_screen_callback'] = \
+            self.move_to_previous_screen_callback
         return Question(**kwargs)
 
     def _generate_rss(self, kwargs):

=== modified file 'entertainerlib/frontend/gui/screens/question.py'
--- entertainerlib/frontend/gui/screens/question.py	2009-05-06 01:35:19 +0000
+++ entertainerlib/frontend/gui/screens/question.py	2009-06-02 01:16:03 +0000
@@ -2,7 +2,6 @@
 '''Question - Screen displays a question label and specified buttons.'''
 
 from entertainerlib.frontend.gui.screens.screen import Screen
-from entertainerlib.frontend.gui.transitions.transition import Transition
 from entertainerlib.frontend.gui.widgets.label import Label
 from entertainerlib.frontend.gui.widgets.text_menu import TextMenu
 from entertainerlib.frontend.gui.widgets.text_menu_item import TextMenuItem
@@ -11,14 +10,11 @@
     '''Screen is displayed when the application needs to ask a close ended
     question to the user.'''
 
-    def __init__(self, screen_history, change_screen_callback, question,
+    def __init__(self, move_to_previous_screen_callback, question,
         answers, callbacks=None):
         Screen.__init__(self, 'Question')
 
-        self.theme = self.config.theme
-
-        self.screen_history = screen_history
-        self.change_screen_callback = change_screen_callback
+        self.move_to_previous_screen_callback = move_to_previous_screen_callback
         self.answers = answers
 
         if callbacks is None:
@@ -36,7 +32,7 @@
 
     def display_answers(self):
         '''Display a menu with answers on the screen.'''
-        self.menu = TextMenu(self.theme, self.config.show_effects())
+        self.menu = TextMenu(self.config.theme, self.config.show_effects())
         self.menu.set_name("questionmenu")
         self.menu.set_row_count(1)
         self.menu.set_visible_column_count(7)
@@ -65,9 +61,7 @@
 
     def go_back(self):
         '''Go back to previous screen'''
-        screen = self.screen_history.get_screen()
-        screen.update()
-        self.change_screen_callback(screen, Transition.BACKWARD)
+        self.move_to_previous_screen_callback()
 
     def _move_menu(self, menu_direction):
         '''Move the menu in the given direction.'''

=== modified file 'entertainerlib/frontend/gui/user_interface.py'
--- entertainerlib/frontend/gui/user_interface.py	2009-05-09 17:03:51 +0000
+++ entertainerlib/frontend/gui/user_interface.py	2009-06-02 01:16:03 +0000
@@ -88,7 +88,7 @@
             self.is_fullscreen = False
 
         # Initialize Screen history (allows user to navigate "back")
-        self.history = ScreenHistory(self.stage, self.config.history_size())
+        self.history = ScreenHistory(self._remove_from_stage)
 
         self.player = MediaPlayer(self.stage,
             self.config.get_stage_width(), self.config.get_stage_height())
@@ -109,8 +109,7 @@
         # Screen factory to create new screens
         self.screen_factory = ScreenFactory(
             feed_library, image_library, music_library, video_library,
-            self.player, self.history,
-            self.move_to_new_screen, self.change_screen)
+            self.player, self.move_to_new_screen, self.move_to_previous_screen)
 
         def default_key_to_user_event():
             '''Return the default user event provided by an unmapped keyboard
@@ -235,6 +234,12 @@
         screen = self.create_screen(screen_type, kwargs)
         self.change_screen(screen, transition)
 
+    def move_to_previous_screen(self):
+        '''Callback method to return to the previous screen in history.'''
+        screen = self.history.get_screen()
+        screen.update()
+        self.change_screen(screen, Transition.BACKWARD)
+
     def show(self):
         '''Show the user interface.'''
         self.window.show_all()
@@ -347,9 +352,7 @@
     def _handle_navigate_back(self, event):
         '''Handle UserEvent.NAVIGATE_BACK.'''
         if not self.history.is_empty():
-            screen = self.history.get_screen()
-            screen.update()
-            self.change_screen(screen, Transition.BACKWARD)
+            self.move_to_previous_screen()
 
     def _handle_navigate_home(self, event):
         '''Handle UserEvent.NAVIGATE_HOME.'''

=== modified file 'entertainerlib/tests/test_screenfactory.py'
--- entertainerlib/tests/test_screenfactory.py	2009-04-28 22:30:06 +0000
+++ entertainerlib/tests/test_screenfactory.py	2009-06-02 01:16:03 +0000
@@ -51,7 +51,7 @@
         media_player = MockMediaPlayer()
         video_library = MockVideoLibrary()
         self.factory = ScreenFactory(feed_library, image_libary, music_library,
-            video_library, media_player, None, None, None)
+            video_library, media_player, None, None)
 
         self.kwargs = {}
 

=== modified file 'entertainerlib/tests/test_userinterface.py'
--- entertainerlib/tests/test_userinterface.py	2009-05-06 01:35:19 +0000
+++ entertainerlib/tests/test_userinterface.py	2009-06-02 01:16:03 +0000
@@ -4,10 +4,12 @@
 import clutter
 
 from entertainerlib.frontend.gui.screens.question import Question
+from entertainerlib.frontend.gui.transitions.transition import Transition
 from entertainerlib.frontend.gui.user_event import UserEvent
 from entertainerlib.frontend.gui.user_interface import UserInterface
 from entertainerlib.tests import EntertainerTest
 from entertainerlib.tests.mock import MockClutterKeyboardEvent
+from entertainerlib.tests.mock import MockFeedLibrary
 
 class UserInterfaceTest(EntertainerTest):
     '''Test for entertainerlib.frontend.gui.user_interface'''
@@ -15,7 +17,8 @@
     def setUp(self):
         EntertainerTest.setUp(self)
 
-        self.ui = UserInterface(None, None, None, None, None)
+        feed_library = MockFeedLibrary()
+        self.ui = UserInterface(feed_library, None, None, None, None)
 
     def test_create(self):
         '''Test correct UserInterface initialization'''
@@ -36,9 +39,19 @@
         bad_event = MockClutterKeyboardEvent(-1)
         self.ui.handle_keyboard_event(None, bad_event, mock_event_handler)
 
+    def test_move_to_previous_screen(self):
+        '''Test the ability to return to a previous screen.'''
+        self.ui.current = self.ui.create_screen('main')
+        old_screen = self.ui.current
+        screen = self.ui.create_screen('weather')
+        self.ui.change_screen(screen, Transition.FORWARD)
+
+        self.ui.move_to_previous_screen()
+        self.assertEquals(self.ui.current, old_screen)
+
     def test_confirm_exit(self):
         '''Test confirm exit stays on Question screen if it's already there.'''
-        question = Question(None, None, None, [])
+        question = Question(None, None, [])
         self.ui.current = question
         # Current will be the same if the screen didn't move.
         self.assertEqual(self.ui.current, question)