Entertainer Media Center

Merge lp://staging/~samuel-buffet/entertainer/reactive_tabs into lp://staging/entertainer

reactive_tabs
Merge into trunk

Proposed by Samuel Buffet on 2009-05-07

Status:

Merged

Approved by:

Matt Layman on 2009-05-12

Approved revision:

405

Merged at revision:

not available

Proposed branch:

lp://staging/~samuel-buffet/entertainer/reactive_tabs

Merge into:

lp://staging/entertainer

Diff against target:

None lines

To merge this branch:

bzr merge lp://staging/~samuel-buffet/entertainer/reactive_tabs

Related bugs:

Bug #315611: UI Problem - Artist Tab

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Matt Layman		2009-05-07	Approve on 2009-05-11
Review via email: mp+6315@code.staging.launchpad.net

Commit message

Tabs are now reactive to pointer events.

Revision history for this message

Samuel Buffet (samuel-buffet) wrote on 2009-05-07:

Hi everybody,

This branch provides reactive Tabs on Screens like Music, AudioPlay etc

The interaction is at the moment limited to switching from one tab to another one by clicking on the Tab title. Further interactions will be possible when other reactive widgets will be integrated on Tabs.

I've also slightly changed effect when switching (now there's a fade out AND a fade in)

Samuel-

lp://staging/~samuel-buffet/entertainer/reactive_tabs updated on 2009-05-10

380. By Samuel Buffet on 2009-05-10: merged with trunk 376

Revision history for this message

Matt Layman (mblayman) wrote on 2009-05-11:

I really liked the results of this branch Samuel. The few added python properties went a long way to cleaning up some code that used to be fairly awkward and confusing. The fade in and fade out will have to grow on me a little (I'm just jaded from the old transition), but I'm fine with the change you made. Like I said on IRC, it only looks slightly weird (IMO) when there are multiple "no content" message between tabs. Here are my specific review comments:

General:
* Need a full stop for docstrings (since they are normal English sentences).
* _on_activate should be _on_activated because the event is 'activated.' This will affect the doc string too ('''Tab activated.''').

entertainerlib/frontend/gui/tabs/series_tab.py
* _update_series_info doc string. There is no such thing as a TV-Serie. Series always has an 's' on the end in English. I know this one is just a carry over from the previous doc string, but now is a good time to correct the spelling mistake.

entertainerlib/frontend/gui/tabs/tab.py
* Tab inherits from Base which inherits from object so there is no need for Tab to directly inherit from object.

entertainerlib/frontend/gui/widgets/tab_group.py
* TabGroup inherits from Base which inherits from object so there is no need for TabGroup to directly inherit from object.
* If you're inheriting from Base, there is no need for another call to "self.config = Configuration()" because that happens in the init of Base.
* Could you add a comment about the timelines code in _activate_tab? I had to comment out the lines to see that they affected the label name of each tab. It wasn't very clear what that code did.

I noticed that there are no tests for this branch either. I'm going to approve this code because I'm not sure exactly what you could test. However, I will leave it up to your discretion to write tests that might be helpful for the long term maintenance of this code. Maybe you could test the boundary conditions of the TabGroup? Simulate clicking new tabs and see if the current tab got updated correctly? I think that there are some worthwhile things here to test, and I hope that you agree and can come up with some stuff to add to the test suite.

Thanks,
Matt

General:
 * Need a full stop for docstrings (since they are normal English sentences).
 * _on_activate should be _on_activated because the event is 'activated.' This will affect the doc string too ('''Tab activated.''').

entertainerlib/frontend/gui/tabs/series_tab.py
 * _update_series_info doc string. There is no such thing as a TV-Serie. Series always has an 's' on the end in English. I know this one is just a carry over from the previous doc string, but now is a good time to correct the spelling mistake.

entertainerlib/frontend/gui/tabs/tab.py
 * Tab inherits from Base which inherits from object so there is no need for Tab to directly inherit from object.

entertainerlib/frontend/gui/widgets/tab_group.py
 * TabGroup inherits from Base which inherits from object so there is no need for TabGroup to directly inherit from object.
 * If you're inheriting from Base, there is no need for another call to "self.config = Configuration()" because that happens in the init of Base.
 * Could you add a comment about the timelines code in _activate_tab? I had to comment out the lines to see that they affected the label name of each tab. It wasn't very clear what that code did.

Thanks,
Matt

review: Approve

lp://staging/~samuel-buffet/entertainer/reactive_tabs updated on 2009-05-11

381. By Samuel Buffet on 2009-05-11: - docstrings update in album.tab.py
- self._on_activate is now self._on_activated
382. By Samuel Buffet on 2009-05-11: - docstrings update in artists_tab.py
- self._on_activate is now self._on_activated
383. By Samuel Buffet on 2009-05-11: - docstrings update in lyrics_tab.py
- self._on_activate is now self._on_activated
384. By Samuel Buffet on 2009-05-11: - docstrings update in movies_tab.py
- self._on_activate is now self._on_activated
385. By Samuel Buffet on 2009-05-11: - docstrings update in series_tab.py
- self._on_activate is now self._on_activated
386. By Samuel Buffet on 2009-05-11: - docstrings update in tracks_tab.py
- self._on_activate is now self._on_activated
387. By Samuel Buffet on 2009-05-11: - docstrings update in video_clips_tab.py
- self._on_activate is now self._on_activated
388. By Samuel Buffet on 2009-05-11: docstring update in series_tab.py because TV-serie doesn't exist in English.
389. By Samuel Buffet on 2009-05-11: - removed object in Tab and TabGroup Classes (they inherit from Base which already inherit from object)
- removed self.config = Configuration() from TabGroup __init__ because that's already done in Base's __init__
390. By Samuel Buffet on 2009-05-11: lint fix
391. By Samuel Buffet on 2009-05-11: comments added in tab_group.py to give more information about what we do with timelines in the _activate_tab method.
392. By Samuel Buffet on 2009-05-11: a test has been created for the TabGroup Class
393. By Samuel Buffet on 2009-05-11: - some fixes because the Tab Class uses now an active property
- Screen is fixed because the TabGroup Class uses an active property
- creation of the active property on TabGroup
- 2 new methods added to the music library mock
- TabGroup test improvments
394. By Samuel Buffet on 2009-05-11: removed the useless resize_widget method
395. By Samuel Buffet on 2009-05-11: 2 new tests added to test_tabgroup.py
- test_add_tab and
- test_keyboard_navigation
396. By Samuel Buffet on 2009-05-11: removed some forgotten set_active
397. By Samuel Buffet on 2009-05-11: fixed a forgotten _on_activate in series_tab.py
398. By Samuel Buffet on 2009-05-11: docstrings clean up to comply with standards.
399. By Samuel Buffet on 2009-05-11: - created an activated signal for the TabGroup Class
- removed the deactivated signal for the Tab Class
400. By Samuel Buffet on 2009-05-11: - fix the tests
- tabgroup change its opacity accordingly to is state active/deactive
- clean up

Revision history for this message

Samuel Buffet (samuel-buffet) wrote on 2009-05-11:

Download full text (41.6 KiB)

Matt,

> I really liked the results of this branch Samuel. The few added python
> properties went a long way to cleaning up some code that used to be fairly
> awkward and confusing. The fade in and fade out will have to grow on me a
> little (I'm just jaded from the old transition), but I'm fine with the change
> you made. Like I said on IRC, it only looks slightly weird (IMO) when there
> are multiple "no content" message between tabs. Here are my specific review
> comments:
>
> General:
> * Need a full stop for docstrings (since they are normal English sentences).
> * _on_activate should be _on_activated because the event is 'activated.' This
> will affect the doc string too ('''Tab activated.''').

Yeah, I don't know why I've choosen on_activate for the "activated" signal. I surly was dreaming a bit.

>
> entertainerlib/frontend/gui/tabs/series_tab.py
> * _update_series_info doc string. There is no such thing as a TV-Serie.
> Series always has an 's' on the end in English. I know this one is just a
> carry over from the previous doc string, but now is a good time to correct the
> spelling mistake.

Fixed.

> entertainerlib/frontend/gui/tabs/tab.py
> * Tab inherits from Base which inherits from object so there is no need for
> Tab to directly inherit from object.

Yes totally right, thanks.

> entertainerlib/frontend/gui/widgets/tab_group.py
> * TabGroup inherits from Base which inherits from object so there is no need
> for TabGroup to directly inherit from object.
> * If you're inheriting from Base, there is no need for another call to
> "self.config = Configuration()" because that happens in the init of Base.

Yes totally again.

> * Could you add a comment about the timelines code in _activate_tab? I had to
> comment out the lines to see that they affected the label name of each tab. It
> wasn't very clear what that code did.

Comment added.

> I noticed that there are no tests for this branch either. I'm going to approve
> this code because I'm not sure exactly what you could test. However, I will
> leave it up to your discretion to write tests that might be helpful for the
> long term maintenance of this code. Maybe you could test the boundary
> conditions of the TabGroup? Simulate clicking new tabs and see if the current
> tab got updated correctly? I think that there are some worthwhile things here
> to test, and I hope that you agree and can come up with some stuff to add to
> the test suite.

Yes Matt, I fully agree with you once more. I've created test for this widget as you will see in the diff. But not really for the reactive part of the widget (for the standard one). I'm missing inspiration for the reactive part, I've not found a good way to do that so I'll come to that later.

See the incremental diff which is a bit big because I took the opportunity to change/fix a few things I should have done before.

Samuel-

Matt,

> I really liked the results of this branch Samuel. The few added python
> properties went a long way to cleaning up some code that used to be fairly
> awkward and confusing. The fade in and fade out will have to grow on me a
> little (I'm just jaded from the old transition), but I'm fine with the change
> you made. Like I said on IRC, it only looks slightly weird (IMO) when there
> are multiple "no content" message between tabs. Here are my specific review
> comments:
> 
> General:
>  * Need a full stop for docstrings (since they are normal English sentences).
>  * _on_activate should be _on_activated because the event is 'activated.' This
> will affect the doc string too ('''Tab activated.''').

Yeah, I don't know why I've choosen on_activate for the "activated" signal. I surly was dreaming a bit.

> 
> entertainerlib/frontend/gui/tabs/series_tab.py
>  * _update_series_info doc string. There is no such thing as a TV-Serie.
> Series always has an 's' on the end in English. I know this one is just a
> carry over from the previous doc string, but now is a good time to correct the
> spelling mistake.

Fixed.

> entertainerlib/frontend/gui/tabs/tab.py
>  * Tab inherits from Base which inherits from object so there is no need for
> Tab to directly inherit from object.

Yes totally right, thanks.

> entertainerlib/frontend/gui/widgets/tab_group.py
>  * TabGroup inherits from Base which inherits from object so there is no need
> for TabGroup to directly inherit from object.
>  * If you're inheriting from Base, there is no need for another call to
> "self.config = Configuration()" because that happens in the init of Base.

Yes totally again.

>  * Could you add a comment about the timelines code in _activate_tab? I had to
> comment out the lines to see that they affected the label name of each tab. It
> wasn't very clear what that code did.

Comment added.

See the incremental diff which is a bit big because I took the opportunity to change/fix a few things I should have done before.

Samuel-

=== modified file 'entertainerlib/frontend/gui/screens/artist.py'
--- entertainerlib/frontend/gui/screens/artist.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/screens/artist.py	2009-05-11 11:37:05 +0000
@@ -39,7 +39,7 @@
         Override function from Screen class. See Screen class for
         better documentation.
         """
-        if self.tab_group.tab("albums").is_active():
+        if self.tab_group.tab("albums").active:
             return True
         else:
             return False

=== modified file 'entertainerlib/frontend/gui/screens/music.py'
--- entertainerlib/frontend/gui/screens/music.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/screens/music.py	2009-05-11 19:27:58 +0000
@@ -66,8 +66,8 @@
         better documentation.
         @return: Boolean
         """
-        if self.tab_group.tab("artists").is_active() or \
-            self.tab_group.tab("albums").is_active():
+        if self.tab_group.tab("artists").active or \
+            self.tab_group.tab("albums").active:
             return True
         else:
             return False
@@ -77,12 +77,12 @@
         Override function from Screen class. See Screen class for
         better documentation.
         """
-        if self.tab_group.tab("artists").is_active():
+        if self.tab_group.tab("artists").active:
             artist = self.tab_group.tab("artists").selected_artist
             self.media_player.set_playlist(Playlist(
                 self.music_library.get_tracks_by_artist(artist)))
             self.media_player.play()
-        elif self.tab_group.tab("albums").is_active():
+        elif self.tab_group.tab("albums").active:
             album = self.tab_group.tab("albums").selected_album
             self.media_player.set_playlist(Playlist(album.get_tracks()))
             self.media_player.play()

=== modified file 'entertainerlib/frontend/gui/screens/screen.py'
--- entertainerlib/frontend/gui/screens/screen.py	2009-05-06 03:40:22 +0000
+++ entertainerlib/frontend/gui/screens/screen.py	2009-05-11 19:27:45 +0000
@@ -39,7 +39,8 @@
             self.tab_group.set_y(self.get_abs_y(0.1))
             self.tab_group.set_anchor_point_from_gravity(clutter.GRAVITY_CENTER)
             self.tab_group.set_x(self.get_abs_x(0.5))
-            self.tab_group.set_active(True)
+            self.tab_group.active = True
+
             self.add(self.tab_group)
 
         if kind is None:

=== modified file 'entertainerlib/frontend/gui/tabs/albums_tab.py'
--- entertainerlib/frontend/gui/tabs/albums_tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/albums_tab.py	2009-05-11 19:52:21 +0000
@@ -65,7 +65,6 @@
         gobject.timeout_add(50, self._async_menu_build, albums, x_percent)
 
         self.menu.set_position(self.get_abs_x(0.0586), self.get_abs_y(0.1823))
-        self.menu.set_active(False)
         self.menu.show()
 
         if self.config.show_effects():
@@ -99,7 +98,7 @@
         self._selected_album = None
         gobject.timeout_add(50, self._init_selected_album)
 
-        self.connect('activated', self._on_activate)
+        self.connect('activated', self._on_activated)
         self.connect('deactivated', self._on_deactivated)
 
     def _async_menu_build(self, albums, item_size_percent):
@@ -131,7 +130,7 @@
 
     def _init_selected_album(self):
         '''The menu is loaded dynamically so the current menu item may not be
-        available initially. Keep trying until the value is available'''
+        available initially. Keep trying until the value is available.'''
         try:
             self._selected_album = \
                 self.menu.get_current_menuitem().get_userdata()
@@ -142,18 +141,18 @@
             return True
 
     def _get_selected_album(self):
-        '''Get the currently selected album from the menu'''
+        '''Get the currently selected album from the menu.'''
         return self.menu.get_current_menuitem().get_userdata()
 
     selected_album = property(_get_selected_album)
 
     def can_activate(self):
         '''Albums tab will always be created from an existing artist with at
-        least one album'''
+        least one album.'''
         return True
 
     def _update_album_info(self):
-        '''Update the album information labels'''
+        '''Update the album information labels.'''
         if self.active:
             album = self.menu.get_current_menuitem().get_userdata()
             self.album_title.set_text(album.get_title())
@@ -166,47 +165,47 @@
             self.album_tracks.set_text("")
 
     def _update_menu(self, direction):
-        '''Update the menu based on the provided menu direction'''
+        '''Update the menu based on the provided menu direction.'''
         self.menu.move(direction)
         self._update_album_info()
         self.li.set_current(self.menu.get_current_position() + 1)
         return False
 
     def _handle_up(self):
-        '''Handle the up user event'''
+        '''Handle the up user event.'''
         if self.menu.get_cursor_position()[1] == 0:
             return True # Move control back to tab bar
         else:
             return self._update_menu(self.menu.UP)
 
     def _handle_down(self):
-        '''Handle the down user event'''
+        '''Handle the down user event.'''
         return self._update_menu(self.menu.DOWN)
 
     def _handle_left(self):
-        '''Handle the left user event'''
+        '''Handle the left user event.'''
         return self._update_menu(self.menu.LEFT)
 
     def _handle_right(self):
-        '''Handle the right user event'''
+        '''Handle the right user event.'''
         return self._update_menu(self.menu.RIGHT)
 
     def _handle_select(self):
-        '''Handle the select user event'''
+        '''Handle the select user event.'''
         album = self.menu.get_current_menuitem().get_userdata()
         kwargs = { 'album' : album }
         self.callback("album", kwargs)
         return False
 
-    def _on_activate(self, event=None):
-        '''Tab activation'''
+    def _on_activated(self, event=None):
+        '''Tab activated.'''
         self._update_album_info()
         self.menu.set_active(True)
         self.menu.set_opacity(255)
         return False
 
     def _on_deactivated(self, event=None):
-        '''Tab deactivated'''
+        '''Tab deactivated.'''
         self._update_album_info()
         self.menu.set_active(False)
         self.menu.set_opacity(128)

=== modified file 'entertainerlib/frontend/gui/tabs/artists_tab.py'
--- entertainerlib/frontend/gui/tabs/artists_tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/artists_tab.py	2009-05-11 20:08:27 +0000
@@ -39,7 +39,6 @@
         self.menu.set_row_count(7)
         self.menu.set_visible_column_count(3)
         self.menu.set_position(self.get_abs_x(0.057), self.get_abs_y(0.208))
-        self.menu.set_active(False)
         if self.config.show_effects():
             self.menu.set_animate(True)
         self.add(self.menu)
@@ -65,12 +64,11 @@
         self.li.set_current(1)
         self.add(self.li)
 
-        gobject.timeout_add(25, self._async_menu_build, artists)
-
-        self._selected_artist = None
-        gobject.timeout_add(25, self._init_selected_artist)
-
-        self.connect('activated', self._on_activate)
+        self._async_menu_build(artists)
+
+        self._selected_artist = self.menu.get_current_menuitem().get_userdata()
+
+        self.connect('activated', self._on_activated)
         self.connect('deactivated', self._on_deactivated)
 
     def _async_menu_build(self, artists):
@@ -95,27 +93,15 @@
             # Done loading artists, notify gobject to stop calling menu build
             return False
 
-    def _init_selected_artist(self):
-        '''The menu is loaded dynamically so the current menu item may not be
-        available initially. Keep trying until the value is available'''
-        try:
-            self._selected_artist = \
-                self.menu.get_current_menuitem().get_userdata()
-            # Stop gobject
-            return False
-        except IndexError:
-            # Item must not be available yet, continue to call
-            return True
-
     def _get_selected_artist(self):
-        '''Get the currently selected artist from the menu'''
+        '''Get the currently selected artist from the menu.'''
         return self.menu.get_current_menuitem().get_userdata()
 
     selected_artist = property(_get_selected_artist)
 
     def can_activate(self):
         '''Albums tab will always be created from an existing artist with at
-        least one album'''
+        least one album.'''
         return True
 
     def _update_artist_info(self):
@@ -133,47 +119,47 @@
             self.artist_tracks.set_text("")
 
     def _update_menu(self, direction):
-        '''Update the menu based on the provided menu direction'''
+        '''Update the menu based on the provided menu direction.'''
         self.menu.move(direction)
         self._update_artist_info()
         self.li.set_current(self.menu.get_current_position() + 1)
         return False
 
     def _handle_up(self):
-        '''Handle the up user event'''
+        '''Handle the up user event.'''
         if self.menu.get_cursor_position()[1] == 0:
             return True # Move control back to tab bar
         else:
             return self._update_menu(self.menu.UP)
 
     def _handle_down(self):
-        '''Handle the down user event'''
+        '''Handle the down user event.'''
         return self._update_menu(self.menu.DOWN)
 
     def _handle_left(self):
-        '''Handle the left user event'''
+        '''Handle the left user event.'''
         return self._update_menu(self.menu.LEFT)
 
     def _handle_right(self):
-        '''Handle the right user event'''
+        '''Handle the right user event.'''
         return self._update_menu(self.menu.RIGHT)
 
     def _handle_select(self):
-        '''Handle the select user event'''
+        '''Handle the select user event.'''
         artist = self.menu.get_current_menuitem().get_userdata()
         kwargs = { 'artist' : artist }
         self.callback("artist", kwargs)
         return False
 
-    def _on_activate(self, event=None):
-        '''Tab activation'''
+    def _on_activated(self, event=None):
+        '''Tab activated.'''
         self._update_artist_info()
         self.menu.set_active(True)
         self.menu.set_opacity(255)
         return False
 
     def _on_deactivated(self, event=None):
-        '''Tab deactivated'''
+        '''Tab deactivated.'''
         self._update_artist_info()
         self.menu.set_active(False)
         self.menu.set_opacity(128)

=== modified file 'entertainerlib/frontend/gui/tabs/lyrics_tab.py'
--- entertainerlib/frontend/gui/tabs/lyrics_tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/lyrics_tab.py	2009-05-11 19:58:35 +0000
@@ -19,7 +19,7 @@
         self.track = track
         self.lyrics_area = None
         self.library = music_library
-        self.lyrics_text = None
+        self.lyrics_text = ""
 
         if self.track.has_lyrics():
             self.lyrics_text = self.track.get_lyrics()
@@ -30,7 +30,7 @@
             self.lyrics_area = ScrollArea(0.4, 0.57, 0.5, 0.23, lyrics)
             self.add(self.lyrics_area)
 
-            self.connect('activated', self._on_activate)
+            self.connect('activated', self._on_activated)
             self.connect('deactivated', self._on_deactivated)
         else:
             # Display throbber animation while searching for lyrics
@@ -43,10 +43,9 @@
             self.track.fetch_lyrics(self._lyrics_search_callback)
 
     def _lyrics_search_callback(self, lyrics_text):
-        """
+        '''
         This function is called when lyrics search is over.
-        @param lyrics: Lyrics or None if lyrics weren't found
-        """
+        '''
         self.throbber.hide()
 
         # Save the results to help determine if the tab can activate
@@ -66,31 +65,33 @@
             self.library.save_lyrics(self.track, lyrics_text)
 
     def can_activate(self):
-        '''Lyrics tab can scroll the lyrics listing only if lyrics were found'''
-        if self.lyrics_text is None:
+        '''
+        Lyrics tab can scroll the lyrics listing only if lyrics were found.
+        '''
+        if self.lyrics_text == "":
             return False
         else:
             return True
 
     def _handle_up(self):
-        '''Handle the up user event'''
+        '''Handle the up user event.'''
         if self.lyrics_area.get_offset() == 0:
             return True # Move control back to tab bar
         else:
             return self.lyrics_area.scroll_up()
 
     def _handle_down(self):
-        '''Handle the down user event'''
+        '''Handle the down user event.'''
         return self.lyrics_area.scroll_down()
 
-    def _on_activate(self, event=None):
-        '''Tab activation'''
+    def _on_activated(self, event=None):
+        '''Tab activated.'''
         self.lyrics_area.set_active(True)
         self.lyrics_area.set_opacity(255)
         return False
 
     def _on_deactivated(self, event=None):
-        '''Tab deactivated'''
+        '''Tab deactivated.'''
         self.lyrics_area.set_active(False)
         self.lyrics_area.set_opacity(128)
         return False

=== modified file 'entertainerlib/frontend/gui/tabs/movies_tab.py'
--- entertainerlib/frontend/gui/tabs/movies_tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/movies_tab.py	2009-05-11 19:59:44 +0000
@@ -37,7 +37,7 @@
         else:
             self._create_menu()
 
-            self.connect('activated', self._on_activate)
+            self.connect('activated', self._on_activated)
             self.connect('deactivated', self._on_deactivated)
 
     def can_activate(self):
@@ -91,7 +91,6 @@
             self.menu.add_actor(item)
 
         self.menu.set_position(self.get_abs_x(0.11), self.get_abs_y(0.2))
-        self.menu.set_active(False)
         self.menu.show()
 
         if self.config.show_effects():
@@ -126,7 +125,7 @@
         self.add(self.movie_plot)
 
     def _update_movie_info(self):
-        '''Update the movie information labels'''
+        '''Update the movie information labels.'''
         if self.active:
             movie = self.menu.get_current_menuitem().get_userdata()
             genres = movie.get_genres()
@@ -145,47 +144,47 @@
             self.movie_plot.set_text("")
 
     def _update_menu(self, direction):
-        '''Update the menu based on the provided menu direction'''
+        '''Update the menu based on the provided menu direction.'''
         self.menu.move(direction)
         self._update_movie_info()
         self.list_indicator.set_current(self.menu.get_current_position() + 1)
         return False
 
     def _handle_up(self):
-        '''Handle the up user event'''
+        '''Handle the up user event.'''
         if self.menu.get_current_position() <= 6:
             return True # Move control back to tab bar
         else:
             return self._update_menu(self.menu.UP)
 
     def _handle_down(self):
-        '''Handle the down user event'''
+        '''Handle the down user event.'''
         return self._update_menu(self.menu.DOWN)
 
     def _handle_left(self):
-        '''Handle the left user event'''
+        '''Handle the left user event.'''
         return self._update_menu(self.menu.LEFT)
 
     def _handle_right(self):
-        '''Handle the right user event'''
+        '''Handle the right user event.'''
         return self._update_menu(self.menu.RIGHT)
 
     def _handle_select(self):
-        '''Handle the select user event'''
+        '''Handle the select user event.'''
         movie = self.menu.get_current_menuitem().get_userdata()
         kwargs = { 'movie' : movie }
         self.callback("movie", kwargs)
         return False
 
-    def _on_activate(self, event=None):
-        '''Tab activation'''
+    def _on_activated(self, event=None):
+        '''Tab activated.'''
         self._update_movie_info()
         self.menu.set_active(True)
         self.menu.set_opacity(255)
         return False
 
     def _on_deactivated(self, event=None):
-        '''Tab deactivated'''
+        '''Tab deactivated.'''
         self._update_movie_info()
         self.menu.set_active(False)
         self.menu.set_opacity(128)

=== modified file 'entertainerlib/frontend/gui/tabs/series_tab.py'
--- entertainerlib/frontend/gui/tabs/series_tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/series_tab.py	2009-05-11 20:00:00 +0000
@@ -36,7 +36,7 @@
         else:
             self._create_menu()
 
-            self.connect('activated', self._on_activate)
+            self.connect('activated', self._on_activated)
             self.connect('deactivated', self._on_deactivated)
 
     def can_activate(self):
@@ -120,7 +120,7 @@
         self.add(self.series_info)
 
     def _update_series_info(self):
-        '''Update the TV-Serie information labels'''
+        '''Update the series information labels.'''
         if self.active:
             series = self.menu.get_current_menuitem().get_userdata()
             info = _("%(season)d Seasons\n%(episode)d Episodes") % {'season':
@@ -134,47 +134,47 @@
             self.series_title.set_text("")
 
     def _update_menu(self, direction):
-        '''Update the menu based on the provided menu direction'''
+        '''Update the menu based on the provided menu direction.'''
         self.menu.move(direction)
         self._update_series_info()
         self.list_indicator.set_current(self.menu.get_current_position() + 1)
         return False
 
     def _handle_up(self):
-        '''Handle the up user event'''
+        '''Handle the up user event.'''
         if self.menu.get_current_position() <= 6:
             return True # Move control back to tab bar
         else:
             return self._update_menu(self.menu.UP)
 
     def _handle_down(self):
-        '''Handle the down user event'''
+        '''Handle the down user event.'''
         return self._update_menu(self.menu.DOWN)
 
     def _handle_left(self):
-        '''Handle the left user event'''
+        '''Handle the left user event.'''
         return self._update_menu(self.menu.LEFT)
 
     def _handle_right(self):
-        '''Handle the right user event'''
+        '''Handle the right user event.'''
         return self._update_menu(self.menu.RIGHT)
 
     def _handle_select(self):
-        '''Handle the select user event'''
+        '''Handle the select user event.'''
         series = self.menu.get_current_menuitem().get_userdata()
         kwargs = { 'tv_series' : series }
         self.callback("tv_series", kwargs)
         return False
 
-    def _on_activate(self, event=None):
-        '''Tab activation'''
+    def _on_activated(self, event=None):
+        '''Tab activated.'''
         self._update_series_info()
         self.menu.set_active(True)
         self.menu.set_opacity(255)
         return False
 
     def _on_deactivated(self, event=None):
-        '''Tab deactivated'''
+        '''Tab deactivated.'''
         self._update_series_info()
         self.menu.set_active(False)
         self.menu.set_opacity(128)

=== modified file 'entertainerlib/frontend/gui/tabs/tab.py'
--- entertainerlib/frontend/gui/tabs/tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/tab.py	2009-05-11 19:36:59 +0000
@@ -1,5 +1,5 @@
 # Copyright (c) 2009 Entertainer Developers - See COPYING - GPLv2
-'''Tab - Tab is group of objects that are displayed on one tab'''
+'''Tab - Tab is group of objects that are displayed on one tab.'''
 
 import clutter
 import gobject
@@ -9,14 +9,13 @@
 from entertainerlib.frontend.gui.widgets.label import Label
 from entertainerlib.frontend.gui.widgets.texture import Texture
 
-class Tab(Base, clutter.Group, object):
-    """
-    Tab can be used as part of the TabGroup
+class Tab(Base, clutter.Group):
+    '''
+    Tab can be used as part of the TabGroup.
 
     Tab is a very simple container that contains all the widgets and logic
     of the tab page.
-    """
-    __gtype_name__ = 'Tab'
+    '''
     __gsignals__ = {
         'activated' : ( gobject.SIGNAL_RUN_LAST, gobject.TYPE_NONE, () ),
         'deactivated' : ( gobject.SIGNAL_RUN_LAST, gobject.TYPE_NONE, () ),
@@ -46,16 +45,19 @@
         self.behaviour.apply(self)
 
         # Tabs are created deactivated and invisible
-        self._active = False
+        self._active = None
         self._visible = False
         self.set_opacity(0)
 
     def _get_active(self):
-        """active property getter."""
+        '''active property getter.'''
         return self._active
 
     def _set_active(self, boolean):
-        """active property setter."""
+        '''active property setter.'''
+        if self._active == boolean:
+            return
+
         self._active = boolean
 
         if boolean:
@@ -66,11 +68,14 @@
     active = property(_get_active, _set_active)
 
     def _get_visible(self):
-        """visible property getter."""
+        '''visible property getter.'''
         return self._visible
 
     def _set_visible(self, boolean):
-        """visible property setter."""
+        '''visible property setter.'''
+        if self._visible == boolean:
+            return
+
         self._visible = boolean
 
         if boolean:
@@ -84,20 +89,20 @@
     visible = property(_get_visible, _set_visible)
 
     def can_activate(self):
-        """
+        '''
         Returns a boolean whether or not this tab can be activated. If tab
         doesn't have any controlling widgets it shouldn't activate ever.
         Default value is False. Override this method when implementing a new
         tab and add logic if needed.
-        """
+        '''
         return False
 
     def show_empty_tab_notice(self, title=_("Empty tab"),
             message_body=_("This tab doesn't contain any elements.")):
-        """
+        '''
         Create an information box that is displayed if there is no widgets in
         this tab. This method should be called only from child class as needed.
-        """
+        '''
 
         # Create warning icon
         info_icon = Texture(self.theme.getImage("warning_icon"), 0.28, 0.27)
@@ -113,30 +118,27 @@
         self.add(info)
 
     def _handle_up(self):
-        '''Dummy method for unimplemented handlers in subclass'''
+        '''Dummy method for unimplemented handlers in subclass.'''
         return False
 
     def _handle_down(self):
-        '''Dummy method for unimplemented handlers in subclass'''
+        '''Dummy method for unimplemented handlers in subclass.'''
         return False
 
     def _handle_left(self):
-        '''Dummy method for unimplemented handlers in subclass'''
+        '''Dummy method for unimplemented handlers in subclass.'''
         return False
 
     def _handle_right(self):
-        '''Dummy method for unimplemented handlers in subclass'''
+        '''Dummy method for unimplemented handlers in subclass.'''
         return False
 
     def _handle_select(self):
-        '''Dummy method for unimplemented handlers in subclass'''
+        '''Dummy method for unimplemented handlers in subclass.'''
         return False
 
     def handle_user_event(self, event):
-        '''Dispatch specific user events to the appropriate method
-
-        @return: Boolean, Should control be moved back to tab bar from tab
-        '''
+        '''Dispatch specific user events to the appropriate method.'''
         try:
             handle_method = self.dispatch[event.get_type()]
         except KeyError:

=== modified file 'entertainerlib/frontend/gui/tabs/tracks_tab.py'
--- entertainerlib/frontend/gui/tabs/tracks_tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/tracks_tab.py	2009-05-11 20:00:05 +0000
@@ -64,12 +64,12 @@
         self.li.show()
         self.add(self.li)
 
-        self.connect('activated', self._on_activate)
+        self.connect('activated', self._on_activated)
         self.connect('deactivated', self._on_deactivated)
 
     def can_activate(self):
         '''Tracks tab will always be created from an existing artist with at
-        least one track'''
+        least one track.'''
         return True
 
     def _update_track_info(self):
@@ -88,47 +88,47 @@
             self.track_number.set_text("")
 
     def _update_menu(self, direction):
-        '''Update the menu based on the provided menu direction'''
+        '''Update the menu based on the provided menu direction.'''
         self.menu.move(direction)
         self._update_track_info()
         self.li.set_current(self.menu.get_current_position() + 1)
         return False
 
     def _handle_up(self):
-        '''Handle the up user event'''
+        '''Handle the up user event.'''
         if self.menu.get_cursor_position()[1] == 0:
             return True # Move control back to tab bar
         else:
             return self._update_menu(self.menu.UP)
 
     def _handle_down(self):
-        '''Handle the down user event'''
+        '''Handle the down user event.'''
         return self._update_menu(self.menu.DOWN)
 
     def _handle_left(self):
-        '''Handle the left user event'''
+        '''Handle the left user event.'''
         return self._update_menu(self.menu.LEFT)
 
     def _handle_right(self):
-        '''Handle the right user event'''
+        '''Handle the right user event.'''
         return self._update_menu(self.menu.RIGHT)
 
     def _handle_select(self):
-        '''Handle the select user event'''
+        '''Handle the select user event.'''
         track = self.menu.get_current_menuitem().get_userdata()
         kwargs = { 'track' : track }
         self.callback("audio_play", kwargs)
         return False
 
-    def _on_activate(self, event=None):
-        '''Tab activation'''
+    def _on_activated(self, event=None):
+        '''Tab activated.'''
         self._update_track_info()
         self.menu.set_active(True)
         self.menu.set_opacity(255)
         return False
 
     def _on_deactivated(self, event=None):
-        '''Tab deactivated'''
+        '''Tab deactivated.'''
         self._update_track_info()
         self.menu.set_active(False)
         self.menu.set_opacity(128)

=== modified file 'entertainerlib/frontend/gui/tabs/video_clips_tab.py'
--- entertainerlib/frontend/gui/tabs/video_clips_tab.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/tabs/video_clips_tab.py	2009-05-11 20:00:10 +0000
@@ -37,7 +37,7 @@
         else:
             self._create_menu()
 
-            self.connect('activated', self._on_activate)
+            self.connect('activated', self._on_activated)
             self.connect('deactivated', self._on_deactivated)
 
     def can_activate(self):
@@ -112,7 +112,7 @@
         self.add(self.clip_info)
 
     def _update_clip_info(self):
-        '''Update the VideoClip information labels'''
+        '''Update the VideoClip information labels.'''
         if self.active:
             clip = self.menu.get_current_menuitem().get_userdata()
             (folder, filename) = os.path.split(clip.get_filename())
@@ -123,48 +123,48 @@
             self.clip_info.set_text("")
 
     def _update_menu(self, direction):
-        '''Update the menu based on the provided menu direction'''
+        '''Update the menu based on the provided menu direction.'''
         self.menu.move(direction)
         self._update_clip_info()
         self.list_indicator.set_current(self.menu.get_current_position() + 1)
         return False
 
     def _handle_up(self):
-        '''Handle the up user event'''
+        '''Handle the up user event.'''
         if self.menu.get_current_position() <= 2:
             return True # Move control back to tab bar
         else:
             return self._update_menu(self.menu.UP)
 
     def _handle_down(self):
-        '''Handle the down user event'''
+        '''Handle the down user event.'''
         return self._update_menu(self.menu.DOWN)
 
     def _handle_left(self):
-        '''Handle the left user event'''
+        '''Handle the left user event.'''
         return self._update_menu(self.menu.LEFT)
 
     def _handle_right(self):
-        '''Handle the right user event'''
+        '''Handle the right user event.'''
         return self._update_menu(self.menu.RIGHT)
 
     def _handle_select(self):
-        '''Handle the select user event'''
+        '''Handle the select user event.'''
         clip = self.menu.get_current_menuitem().get_userdata()
         self.media_player.set_media(clip)
         self.media_player.play()
         self.callback("video_osd")
         return False
 
-    def _on_activate(self, event=None):
-        '''Tab activation'''
+    def _on_activated(self, event=None):
+        '''Tab activated.'''
         self._update_clip_info()
         self.menu.set_active(True)
         self.menu.set_opacity(255)
         return False
 
     def _on_deactivated(self, event=None):
-        '''Tab deactivated'''
+        '''Tab deactivated.'''
         self._update_clip_info()
         self.menu.set_active(False)
         self.menu.set_opacity(128)

=== modified file 'entertainerlib/frontend/gui/widgets/tab_group.py'
--- entertainerlib/frontend/gui/widgets/tab_group.py	2009-05-10 06:35:30 +0000
+++ entertainerlib/frontend/gui/widgets/tab_group.py	2009-05-11 20:26:02 +0000
@@ -2,34 +2,29 @@
 '''TabGroup is a group of tabs. It draws the tab bar.'''
 
 import clutter
+import gobject
 import pango
 
 from entertainerlib.frontend.gui.widgets.base import Base
 from entertainerlib.frontend.gui.widgets.label import Label
 from entertainerlib.frontend.gui.user_event import UserEvent
-from entertainerlib.utils.configuration import Configuration
 
-class TabGroup(Base, clutter.Group, object):
-    """
+class TabGroup(Base, clutter.Group):
+    '''
     This is a top level container for tabs.
 
     This widget contains graphics for tab bar, but content of the tabs are
     separated into Tab objects.
-    """
+    '''
+    __gsignals__ = {
+        'activated' : ( gobject.SIGNAL_RUN_LAST, gobject.TYPE_NONE, () ),
+        }
 
     def __init__(self, width, height, color):
-        """
-        Initialize empty tab group.
-        @param width: Width of the tab bar in percents (0-1)
-        @param height: height of the tab bar in percents (0-1)
-        @param color: color string to call from the theme
-        """
         Base.__init__(self)
         clutter.Group.__init__(self)
 
-        self.config = Configuration()
-
-        self.active = False
+        self._active = None
 
         # Size
         self.stage_w = self.config.get_stage_width()
@@ -63,50 +58,36 @@
         self.current_tab = None
 
     def tab(self, name):
-        '''Return the given tab that matches the provided name'''
+        '''Return the given tab that matches the provided name.'''
         return self._tabs[name]
 
     def __len__(self):
-        """
-        Get TagGroup length
-        @return: Integer, Number of tabs in this TabGroup
-        """
+        '''Get TagGroup length.'''
         return len(self.tabs_list)
 
-    def resize_widget(self, stage_size):
-        """
-        Update widget size for new stage size. This is called when stage size
-        has changed.
-        @param stage_size: Tuple with width and height in pixels (1028x1024)
-        """
-        stage = clutter.Stage()
-        self.stage_w = stage.get_width()
-        self.stage_h = stage.get_height()
-
-    def set_active(self, boolean):
-        """
-        Set widget status active / inactive
-        @param boolean: True to set active, False to set inactive
-        """
-        self.active = boolean
+    def _get_active(self):
+        '''active property getter.'''
+        return self._active
+
+    def _set_active(self, boolean):
+        '''active property setter.'''
+        if self._active == boolean:
+            return
+
+        self._active = boolean
+
         if boolean:
+            self.emit('activated')
             self.set_opacity(255)
+            if self.current_tab != None:
+                self.tabs_list[self.current_tab].active = False
         else:
             self.set_opacity(128)
 
-    def is_active(self):
-        """
-        Is this widget active?
-        @return: boolean
-        """
-        return self.active
+    active = property(_get_active, _set_active)
 
     def add_tab(self, tab):
-        """
-        Add new tab to this tab group.
-        @param tab: Tab object to be added
-        """
-
+        '''Add new tab to this tab group.'''
         # Add to the dictionary so that the tab can be called by name
         self._tabs[tab.name] = tab
 
@@ -136,13 +117,13 @@
         self.timelines.append((timeline, alpha, behaviour))
 
     def _reposition_labels(self):
-        """
+        '''
         This is called every time we need to calculate new positions to the
         labels.
 
         This is when we add or remove tabs from this TabGroup. This method
         calculates label positions and sizes and updates UI accordingly
-        """
+        '''
         self.font_size = self.get_height() * 0.3
         count = len(self.labels)
         width = int(self.stage_w * self.width_factor)
@@ -159,50 +140,41 @@
             label.set_ellipsize(pango.ELLIPSIZE_END)
 
     def _activate_tab(self, new_tab):
-        """Activation of new selected Tab"""
+        '''Activation of new selected Tab.'''
+        # Fade out of the previously selected title label.
         self.timelines[self.current_tab][0].set_direction(
             clutter.TIMELINE_FORWARD)
         self.timelines[self.current_tab][0].start()
 
+        # Fade in of the selected title label.
         self.timelines[new_tab][0].set_direction(clutter.TIMELINE_BACKWARD)
         self.timelines[new_tab][0].start()
 
         # Change visible tab
         self.tabs_list[self.current_tab].visible = False
-        self.tabs_list[self.current_tab].active = False
         self.tabs_list[new_tab].visible = True
-        self.tabs_list[new_tab].active = False
 
     def _switch_tab_to_left(self):
-        """
-        Switch one tab to left if possible. Do nothing if there is no tab
-        """
+        '''Switch one tab to left or nothing if there is no tab.'''
         if self.current_tab > 0 and self.current_tab is not None:
             self._activate_tab(self.current_tab - 1)
             self.current_tab = self.current_tab - 1
 
     def _switch_tab_to_right(self):
-        """
-        Switch one tab to right if possible. Do nothing if there is no tab
-        """
-        if self.current_tab  < len(self.tabs_list) - 1 and (
+        '''Switch one tab to right or nothing if there is no tab.'''
+        if self.current_tab < len(self.tabs_list) - 1 and (
             self.current_tab is not None):
             self._activate_tab(self.current_tab + 1)
             self.current_tab = self.current_tab + 1
 
     def _switch_tab_to_down(self):
-        """
-        Switch down from current tab if allowed
-        """
+        '''Switch down from current tab if allowed.'''
         if self.tabs_list[self.current_tab].can_activate():
-            self.set_active(False)
+            self.active = False
             self.tabs_list[self.current_tab].active = True
 
     def handle_user_event(self, event):
-        """
-        Handle screen specific user events
-        @param event: UserEvent object
-        """
+        '''Handle screen specific user events.'''
         if self.active:
             event_type = event.get_type()
             if event_type == UserEvent.NAVIGATE_LEFT:
@@ -214,14 +186,18 @@
         else:
             result = self.tabs_list[self.current_tab].handle_user_event(event)
             if result:
-                self.set_active(True)
+                self.active = True
                 self.tabs_list[self.current_tab].active = False
 
     def _on_tab_title_button_press(self, actor, event):
-        '''Title Label button-press handler'''
-        clicked_tab = self._tabs[actor.get_name()]
-        clicked_tab_index = self.tabs_list.index(clicked_tab)
+        '''Title Label button-press handler.'''
+        if self.active:
+            clicked_tab = self._tabs[actor.get_name()]
+            clicked_tab_index = self.tabs_list.index(clicked_tab)
 
-        self._activate_tab(clicked_tab_index)
-        self.current_tab = clicked_tab_index
+            if self.current_tab != clicked_tab_index:
+                self._activate_tab(clicked_tab_index)
+                self.current_tab = clicked_tab_index
+        else:
+            self.active = True

=== modified file 'entertainerlib/tests/mock.py'
--- entertainerlib/tests/mock.py	2009-05-09 17:03:51 +0000
+++ entertainerlib/tests/mock.py	2009-05-11 11:12:18 +0000
@@ -218,6 +218,14 @@
         '''See `MusicLibrary.number_of_tracks`.'''
         return 0
 
+    def get_all_artists(self):
+        '''See `MusicLibrary.get_all_artists`.'''
+        return []
+
+    def get_all_albums(self):
+        '''See `MusicLibrary.get_all_albums`.'''
+        return []
+
     def __del__(self):
         '''See `MusicLibrary.__del__`.'''

=== added file 'entertainerlib/tests/test_tabgroup.py'
--- entertainerlib/tests/test_tabgroup.py	1970-01-01 00:00:00 +0000
+++ entertainerlib/tests/test_tabgroup.py	2009-05-11 20:17:02 +0000
@@ -0,0 +1,62 @@
+# Copyright (c) 2009 Entertainer Developers - See COPYING - GPLv2
+'''Tests TabGroup'''
+
+from entertainerlib.frontend.gui.tabs.tab import Tab
+from entertainerlib.frontend.gui.user_event import UserEvent
+from entertainerlib.frontend.gui.widgets.tab_group import TabGroup
+from entertainerlib.tests import EntertainerTest
+
+class TabGroupTest(EntertainerTest):
+    '''Test for entertainerlib.frontend.gui.widgets.tab_goup.'''
+
+    def setUp(self):
+        '''Set up the test.'''
+        EntertainerTest.setUp(self)
+
+        self.tab_group = TabGroup(0.95, 0.13, 'title')
+
+        tab1 = Tab("tab1", "title1", None)
+        self.tab2 = Tab("foo", "title2", None)
+
+        self.tab_group.add_tab(tab1)
+        self.tab_group.add_tab(self.tab2)
+
+    def tearDown(self):
+        '''Clean up after the test.'''
+        EntertainerTest.tearDown(self)
+
+    def test_create(self):
+        '''Test correct TabGroup initialization.'''
+        self.assertTrue(isinstance(self.tab_group, TabGroup))
+
+    def test_active(self):
+        '''Test the active property.'''
+        self.tab_group.active = False
+        self.assertEqual(self.tab_group.active, False)
+        self.tab_group.active = True
+        self.assertEqual(self.tab_group.active, True)
+
+    def test_len(self):
+        '''Test the use of len on a TabGroup.'''
+        self.assertEqual(len(self.tab_group), 2)
+
+    def test_tab(self):
+        '''Test the use of the tab method.'''
+        self.assertEqual(self.tab_group.tab("foo"), self.tab2)
+
+    def test_add_tab(self):
+        '''Test the use of the add_tab method.'''
+        tabs = len(self.tab_group)
+        self.tab_group.add_tab(Tab("tab3", "title3", None))
+        self.assertEqual(len(self.tab_group), tabs + 1)
+
+    def test_keyboard_navigation(self):
+        '''Test the effect of keyboard navigation events.'''
+        self.tab_group.active = True
+        start_tab = self.tab_group.current_tab
+        right_event = UserEvent(UserEvent.NAVIGATE_RIGHT)
+        left_event = UserEvent(UserEvent.NAVIGATE_LEFT)
+        self.tab_group.handle_user_event(right_event)
+        self.tab_group.handle_user_event(left_event)
+        self.assertEqual(self.tab_group.current_tab, start_tab)
+

Revision history for this message

Matt Layman (mblayman) wrote on 2009-05-11:

Samuel,

Here are my last few follow up comments after reviewing your branch again following all of my initial input.

albums_tab.py
* redundant gobject.timeout_add was removed from artists_tab so it should probably be removed from albums_tab too.

movies_tab.py
* In _update_movie_info, the text for movie_info should be translatable, but it currently isn't. Can you put the value of the "set_text" into translatable format?

tab.py
* self._active = None seem problematic for init. What happens if something tries to call the active property before you set the value? Shouldn't we default to False initially?

tab_group.py
* Same comment as tab.py

test_tabgroup.py
* tearDown isn't necessary since you aren't doing anything that the superclass isn't already doing.

lp://staging/~samuel-buffet/entertainer/reactive_tabs updated on 2009-05-12

401. By Samuel Buffet on 2009-05-12: removed the tearDown method of test_tabgroup.py
402. By Samuel Buffet on 2009-05-12: small modification in albums_tab.py to avoid a useless _init_selected_album.
403. By Samuel Buffet on 2009-05-12: movies_tab.py, improvments in _update_movie_info.
404. By Samuel Buffet on 2009-05-12: The MockMusicLibrary is modified in order to provide a MockAlbum in return of get_albums_by_artist.
This was mandatory to fix the test__generate_artist
405. By Samuel Buffet on 2009-05-12: - artist screen pass test as I've added a protection against empty results
- Tab & TabGroup Classes will return False on the first attempt to read the active property
- RAZ of what I've done in mock.py

Revision history for this message

Samuel Buffet (samuel-buffet) wrote on 2009-05-12:

Download full text (6.8 KiB)

Hi Matt,

All your points are very good questions. See bellow my fixes and explanations.

> albums_tab.py
> * redundant gobject.timeout_add was removed from artists_tab so it should
> probably be removed from albums_tab too.

Fixed.

> movies_tab.py
> * In _update_movie_info, the text for movie_info should be translatable, but
> it currently isn't. Can you put the value of the "set_text" into translatable
> format?

Fixed using the gettext macro _() and using a formatted string.

> tab.py
> * self._active = None seem problematic for init. What happens if something
> tries to call the active property before you set the value? Shouldn't we
> default to False initially?
>
> tab_group.py
> * Same comment as tab.py

Here, there's an explanation to have None in the __init__ and not False (but the point really makes sense). Indeed, if self._active is set to False then if we try to do Tab.active = False later on then we wont run the entire setter's code because of the IF statement at the beginning.

The IF statement is needed to run the setter's code only when there's a change in the property's value. This is really frequent on every language using object's properties.

So look the new incremental diff, I've fixed the point keeping that None initialization of self._active

> test_tabgroup.py
> * tearDown isn't necessary since you aren't doing anything that the
> superclass isn't already doing.

Removed.

the new new incremental diff (very short this time).
Thanks for the review.
Samuel-

inc_diff :

# Tabs
albums = music_library.get_albums_by_artist(artist)
- tab1 = AlbumsTab(albums, move_to_new_screen_callback)
+ if albums:
+ tab1 = AlbumsTab(albums, move_to_new_screen_callback)
+ self.add_tab(tab1)

tracks = music_library.get_tracks_by_artist(artist)
- tab2 = TracksTab(tracks, move_to_new_screen_callback)
-
- self.add_tab(tab1)
- self.add_tab(tab2)
+ if tracks:
+ tab2 = TracksTab(tracks, move_to_new_screen_callback)
+ self.add_tab(tab2)

def is_interested_in_play_action(self):
"""

- gobject.timeout_add(50, self._async_menu_build, albums, x_percent)
+ self._async_menu_build(albums, x_percent)
+ self._selected_album = self.menu.get_current_menuitem().get_userdata()

         self.menu.set_position(self.get_abs_x(0.0586), self.get_abs_y(0.1823))
         self.menu.show()
@@ -95,9 +96,6 @@
         self.album_tracks = Label(0.037, "subtitle", 0.22, 0.91, "")
         self.add(self.album_tracks)

- self._sel...

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:

Joshua Scotton

Matt Layman

Paul Hummer

Samuel Buffet

rockinrandy1965

 === modified file 'entertainerlib/frontend/gui/screens/artist.py'
 --- entertainerlib/frontend/gui/screens/artist.py	2009-04-27 03:32:30 +0000
 +++ entertainerlib/frontend/gui/screens/artist.py	2009-05-05 11:18:31 +0000
@@ -33,7 +33,6 @@
          tracks = music_library.get_tracks_by_artist(artist)
          tab2 = TracksTab(tracks, move_to_new_screen_callback)
--        tab2.hide_tab()
          self.add_tab(tab1)
          self.add_tab(tab2)
 === modified file 'entertainerlib/frontend/gui/screens/music.py'
 --- entertainerlib/frontend/gui/screens/music.py	2009-04-27 03:32:30 +0000
 +++ entertainerlib/frontend/gui/screens/music.py	2009-05-05 11:18:31 +0000
@@ -34,7 +34,6 @@
                  move_to_new_screen_callback)
              tab2 = AlbumsTab(music_library.get_all_albums(),
                  move_to_new_screen_callback)
--            tab2.hide_tab()
              self.add_tab(tab1)
              self.add_tab(tab2)
 === modified file 'entertainerlib/frontend/gui/tabs/albums_tab.py'
 --- entertainerlib/frontend/gui/tabs/albums_tab.py	2009-02-03 23:37:57 +0000
 +++ entertainerlib/frontend/gui/tabs/albums_tab.py	2009-04-22 10:28:29 +0000
@@ -102,6 +102,9 @@
          self._selected_album = None
          gobject.timeout_add(50, self._init_selected_album)
++        self.connect('activated', self._on_activate)
++        self.connect('deactivated', self._on_deactivated)
++
      def _async_menu_build(self, albums, item_size_percent):
          """
          This function is called as an async callback. It is also a recursive
@@ -152,30 +155,23 @@
          least one album'''
          return True
--    def set_active(self, boolean):
--        """
--        Override method from parent class.
--        @param boolean: True to set this tab active, False to set inactive
--        """
--        Tab.set_active(self, boolean)
--        if boolean:
--            self._update_album_info(
--                self.menu.get_current_menuitem().get_userdata())
--            self.menu.set_active(True)
--        else:
--            self.menu.set_active(False)
--
--    def _update_album_info(self, album):
++    def _update_album_info(self):
          '''Update the album information labels'''
--        self.album_title.set_text(album.get_title())
--        self.album_artist.set_text(album.get_artist())
--        self.album_tracks.set_text(_("%(total)s tracks") % \
--            {'total': album.get_number_of_tracks()})
++        if self.active:
++            album = self.menu.get_current_menuitem().get_userdata()
++            self.album_title.set_text(album.get_title())
++            self.album_artist.set_text(album.get_artist())
++            self.album_tracks.set_text(_("%(total)s tracks") % \
++                {'total': album.get_number_of_tracks()})
++        else:
++            self.album_title.set_text("")
++            self.album_artist.set_text("")
++            self.album_tracks.set_text("")
      def _update_menu(self, direction):
          '''Update the menu based on the provided menu direction'''
          self.menu.move(direction)
--        self._update_album_info(self.menu.get_current_menuitem().get_userdata())
++        self._update_album_info()
          self.li.set_current(self.menu.get_current_position() + 1)
          return False
@@ -205,3 +201,17 @@
          self.callback("album", kwargs)
          return False
++    def _on_activate(self, event=None):
++        '''Tab activation'''
++        self._update_album_info()
++        self.menu.set_active(True)
++        self.menu.set_opacity(255)
++        return False
++
++    def _on_deactivated(self, event=None):
++        '''Tab deactivated'''
++        self._update_album_info()
++        self.menu.set_active(False)
++        self.menu.set_opacity(128)
++        return False
++
 === modified file 'entertainerlib/frontend/gui/tabs/artists_tab.py'
 --- entertainerlib/frontend/gui/tabs/artists_tab.py	2009-02-03 23:37:57 +0000
 +++ entertainerlib/frontend/gui/tabs/artists_tab.py	2009-04-22 10:28:29 +0000
@@ -73,6 +73,9 @@
          self._selected_artist = None
          gobject.timeout_add(25, self._init_selected_artist)
++        self.connect('activated', self._on_activate)
++        self.connect('deactivated', self._on_deactivated)
++
      def _async_menu_build(self, artists):
          """
          This function is called as an async callback. It is also a recursive
@@ -118,32 +121,24 @@
          least one album'''
          return True
--    def set_active(self, boolean):
--        """
--        Override method from parent class.
--        @param boolean: True to set this tab active, False to set inactive
--        """
--        Tab.set_active(self, boolean)
--        if boolean:
--            self._update_artist_info(
--                self.menu.get_current_menuitem().get_userdata())
--            self.menu.set_active(True)
--        else:
--            self.menu.set_active(False)
--
--    def _update_artist_info(self, artist):
++    def _update_artist_info(self):
          '''Update the artist information labels'''
--        self.artist_title.set_text(artist)
--        self.artist_albums.set_text(_("%(albums)d albums") %
--            {'albums': self.library.number_of_albums_by_artist(artist)})
--        self.artist_tracks.set_text(_("%(tracks)d tracks") %
--            {'tracks': self.library.number_of_tracks_by_artist(artist)})
++        if self.active:
++            artist = self.menu.get_current_menuitem().get_userdata()
++            self.artist_title.set_text(artist)
++            self.artist_albums.set_text(_("%(albums)d albums") %
++                {'albums': self.library.number_of_albums_by_artist(artist)})
++            self.artist_tracks.set_text(_("%(tracks)d tracks") %
++                {'tracks': self.library.number_of_tracks_by_artist(artist)})
++        else:
++            self.artist_title.set_text("")
++            self.artist_albums.set_text("")
++            self.artist_tracks.set_text("")
      def _update_menu(self, direction):
          '''Update the menu based on the provided menu direction'''
          self.menu.move(direction)
--        self._update_artist_info(
--            self.menu.get_current_menuitem().get_userdata())
++        self._update_artist_info()
          self.li.set_current(self.menu.get_current_position() + 1)
          return False
@@ -173,3 +168,17 @@
          self.callback("artist", kwargs)
          return False
++    def _on_activate(self, event=None):
++        '''Tab activation'''
++        self._update_artist_info()
++        self.menu.set_active(True)
++        self.menu.set_opacity(255)
++        return False
++
++    def _on_deactivated(self, event=None):
++        '''Tab deactivated'''
++        self._update_artist_info()
++        self.menu.set_active(False)
++        self.menu.set_opacity(128)
++        return False
++
 === modified file 'entertainerlib/frontend/gui/tabs/lyrics_tab.py'
 --- entertainerlib/frontend/gui/tabs/lyrics_tab.py	2009-04-21 21:04:56 +0000
 +++ entertainerlib/frontend/gui/tabs/lyrics_tab.py	2009-05-07 16:39:54 +0000
@@ -32,6 +32,9 @@
              self.lyrics_area = ScrollArea(0.4, 0.57, 0.5, 0.23, lyrics)
              self.add(self.lyrics_area)
++
++            self.connect('activated', self._on_activate)
++            self.connect('deactivated', self._on_deactivated)
          else:
              # Display throbber animation while searching for lyrics
              white = clutter.Color(255, 255, 255, 255)
@@ -42,8 +45,6 @@
              self.add(self.throbber)
              self.track.fetch_lyrics(self._lyrics_search_callback)
--        self.hide_tab()
--
      def _lyrics_search_callback(self, lyrics_text):
          """
          This function is called when lyrics search is over.
@@ -75,17 +76,6 @@
          else:
              return True
--    def set_active(self, boolean):
--        """
--        Override method from parent class.
--        @param boolean: True to set this tab active, False to set inactive
--        """
--        Tab.set_active(self, boolean)
--        if boolean:
--            self.lyrics_area.set_active(True)
--        else:
--            self.lyrics_area.set_active(False)
--
      def _handle_up(self):
          '''Handle the up user event'''
          if self.lyrics_area.get_offset() == 0:
@@ -97,3 +87,15 @@
          '''Handle the down user event'''
          return self.lyrics_area.scroll_down()
++    def _on_activate(self, event=None):
++        '''Tab activation'''
++        self.lyrics_area.set_active(True)
++        self.lyrics_area.set_opacity(255)
++        return False
++
++    def _on_deactivated(self, event=None):
++        '''Tab deactivated'''
++        self.lyrics_area.set_active(False)
++        self.lyrics_area.set_opacity(128)
++        return False
++
 === modified file 'entertainerlib/frontend/gui/tabs/movies_tab.py'
 --- entertainerlib/frontend/gui/tabs/movies_tab.py	2009-02-03 23:37:57 +0000
 +++ entertainerlib/frontend/gui/tabs/movies_tab.py	2009-04-22 10:28:29 +0000
@@ -40,6 +40,9 @@
          else:
              self._create_menu()
++            self.connect('activated', self._on_activate)
++            self.connect('deactivated', self._on_deactivated)
++
      def can_activate(self):
          """
          Allow if we have some movies indexed.
@@ -49,19 +52,6 @@
          else:
              return True
--    def set_active(self, boolean):
--        """
--        Override method from parent class.
--        @param boolean: True to set this tab active, False to set inactive
--        """
--        Tab.set_active(self, boolean)
--        if boolean:
--            self._update_movie_info(
--                self.menu.get_current_menuitem().get_userdata())
--            self.menu.set_active(True)
--        else:
--            self.menu.set_active(False)
--
      def _create_empty_library_notice(self):
          """
          Create an information box that is displayed if there are no indexed
@@ -138,26 +128,29 @@
          self.movie_plot.width = 0.7
          self.add(self.movie_plot)
--    def _update_movie_info(self, movie):
--        """
--        Update movie information. Updates movie specific labels.
--        @param movie: Currently selected movie (Movie object)
--        """
--        genres = movie.get_genres()
--        if len(genres) > 1:
--            genre = genres[0] + "/" + genres[1]
++    def _update_movie_info(self):
++        '''Update the movie information labels'''
++        if self.active:
++            movie = self.menu.get_current_menuitem().get_userdata()
++            genres = movie.get_genres()
++            if len(genres) > 1:
++                genre = genres[0] + "/" + genres[1]
++            else:
++                genre = genres[0]
++
++            self.movie_title.set_text(movie.get_title() + \
++                " (" + str(movie.get_year()) + ")")
++            self.movie_info.set_text(str(movie.get_runtime()) + "min, " + genre)
++            self.movie_plot.set_text(movie.get_short_plot())
          else:
--            genre = genres[0]
--
--        self.movie_title.set_text(movie.get_title() + \
--            " (" + str(movie.get_year()) + ")")
--        self.movie_info.set_text(str(movie.get_runtime()) + "min, " + genre)
--        self.movie_plot.set_text(movie.get_short_plot())
++            self.movie_title.set_text("")
++            self.movie_info.set_text("")
++            self.movie_plot.set_text("")
      def _update_menu(self, direction):
          '''Update the menu based on the provided menu direction'''
          self.menu.move(direction)
--        self._update_movie_info(self.menu.get_current_menuitem().get_userdata())
++        self._update_movie_info()
          self.list_indicator.set_current(self.menu.get_current_position() + 1)
          return False
@@ -187,3 +180,17 @@
          self.callback("movie", kwargs)
          return False
++    def _on_activate(self, event=None):
++        '''Tab activation'''
++        self._update_movie_info()
++        self.menu.set_active(True)
++        self.menu.set_opacity(255)
++        return False
++
++    def _on_deactivated(self, event=None):
++        '''Tab deactivated'''
++        self._update_movie_info()
++        self.menu.set_active(False)
++        self.menu.set_opacity(128)
++        return False
++
 === modified file 'entertainerlib/frontend/gui/tabs/series_tab.py'
 --- entertainerlib/frontend/gui/tabs/series_tab.py	2009-02-03 23:37:57 +0000
 +++ entertainerlib/frontend/gui/tabs/series_tab.py	2009-04-22 10:28:29 +0000
@@ -39,7 +39,8 @@
          else:
              self._create_menu()
--        self.hide_tab()
++            self.connect('activated', self._on_activate)
++            self.connect('deactivated', self._on_deactivated)
      def can_activate(self):
          """
@@ -50,19 +51,6 @@
          else:
              return True
--    def set_active(self, boolean):
--        """
--        Override method from parent class.
--        @param boolean: True to set this tab active, False to set inactive
--        """
--        Tab.set_active(self, boolean)
--        if boolean:
--            self._update_series_info(
--                self.menu.get_current_menuitem().get_userdata())
--            self.menu.set_active(True)
--        else:
--            self.menu.set_active(False)
--
      def _create_empty_library_notice(self):
          """
          Create an information box that is displayed if there are no indexed
@@ -134,23 +122,24 @@
          self.series_info = Label(0.034, "subtitle", 0.2, 0.82, "")
          self.add(self.series_info)
--    def _update_series_info(self, series):
--        """
--        Update TV-Serie information. Updates clutter labels on TV-series tab
--        @param series: Currently selected TV-Serie (TVSeries object)
--        """
--        info = _("%(season)d Seasons\n%(episode)d Episodes") % {'season':
--            series.get_number_of_seasons(), 'episode':
--            series.get_number_of_episodes()}
++    def _update_series_info(self):
++        '''Update the TV-Serie information labels'''
++        if self.active:
++            series = self.menu.get_current_menuitem().get_userdata()
++            info = _("%(season)d Seasons\n%(episode)d Episodes") % {'season':
++                series.get_number_of_seasons(), 'episode':
++                series.get_number_of_episodes()}
--        self.series_info.set_text(info)
--        self.series_title.set_text(series.get_title())
++            self.series_info.set_text(info)
++            self.series_title.set_text(series.get_title())
++        else:
++            self.series_info.set_text("")
++            self.series_title.set_text("")
      def _update_menu(self, direction):
          '''Update the menu based on the provided menu direction'''
          self.menu.move(direction)
--        self._update_series_info(
--            self.menu.get_current_menuitem().get_userdata())
++        self._update_series_info()
          self.list_indicator.set_current(self.menu.get_current_position() + 1)
          return False
@@ -180,3 +169,17 @@
          self.callback("tv_series", kwargs)
          return False
++    def _on_activate(self, event=None):
++        '''Tab activation'''
++        self._update_series_info()
++        self.menu.set_active(True)
++        self.menu.set_opacity(255)
++        return False
++
++    def _on_deactivated(self, event=None):
++        '''Tab deactivated'''
++        self._update_series_info()
++        self.menu.set_active(False)
++        self.menu.set_opacity(128)
++        return False
++
 === modified file 'entertainerlib/frontend/gui/tabs/tab.py'
 --- entertainerlib/frontend/gui/tabs/tab.py	2009-02-28 17:22:46 +0000
 +++ entertainerlib/frontend/gui/tabs/tab.py	2009-05-07 19:25:47 +0000
@@ -5,28 +5,33 @@
  __author__ = "Lauri Taimila <lauri@taimila.com>"
  import clutter
++import gobject
  from entertainerlib.frontend.gui.user_event import UserEvent
  from entertainerlib.frontend.gui.widgets.base import Base
  from entertainerlib.frontend.gui.widgets.label import Label
  from entertainerlib.frontend.gui.widgets.texture import Texture
--class Tab(Base, clutter.Group):
++class Tab(Base, clutter.Group, object):
      """
      Tab can be used as part of the TabGroup
      Tab is a very simple container that contains all the widgets and logic
      of the tab page.
      """
++    __gtype_name__ = 'Tab'
++    __gsignals__ = {
++        'activated' : ( gobject.SIGNAL_RUN_LAST, gobject.TYPE_NONE, () ),
++        'deactivated' : ( gobject.SIGNAL_RUN_LAST, gobject.TYPE_NONE, () ),
++        }
      def __init__(self, name, title="Untitled tab", callback=None):
--
          Base.__init__(self)
          clutter.Group.__init__(self)
--        self._name = name
--        self._title = title
++
++        self.name = name
++        self.title = title
          self.callback = callback
--        self.active = False
          self.theme = self.config.theme
          self.dispatch = {
@@ -37,20 +42,49 @@
              UserEvent.NAVIGATE_SELECT : self._handle_select,
+         }
--        self.timeline = clutter.Timeline(15, 60)
++        # show/hide animation on the Tab
++        self.timeline = clutter.Timeline(30, 60)
          self.alpha = clutter.Alpha(self.timeline, clutter.smoothstep_inc_func)
          self.behaviour = clutter.BehaviourOpacity(0, 255, self.alpha)
          self.behaviour.apply(self)
--    @property
--    def name(self):
--        '''Internal string name that can be used as a key for the tab group'''
--        return self._name
--
--    @property
--    def title(self):
--        '''User string title displayed in the interface'''
--        return self._title
++        # Tabs are created deactivated and invisible
++        self._active = False
++        self._visible = False
++        self.set_opacity(0)
++
++    def _get_active(self):
++        """active property getter."""
++        return self._active
++
++    def _set_active(self, boolean):
++        """active property setter."""
++        self._active = boolean
++
++        if boolean:
++            self.emit('activated')
++        else:
++            self.emit('deactivated')
++
++    active = property(_get_active, _set_active)
++
++    def _get_visible(self):
++        """visible property getter."""
++        return self._visible
++
++    def _set_visible(self, boolean):
++        """visible property setter."""
++        self._visible = boolean
++
++        if boolean:
++            self.timeline.set_direction(clutter.TIMELINE_FORWARD)
++            self.timeline.rewind()
++            self.timeline.start()
++        else:
++            self.timeline.set_direction(clutter.TIMELINE_BACKWARD)
++            self.timeline.start()
++
++    visible = property(_get_visible, _set_visible)
      def can_activate(self):
          """
@@ -61,20 +95,6 @@
          """
          return False
--    def is_active(self):
--        """
--        Is this tab active
--        @return: boolean
--        """
--        return self.active
--
--    def set_active(self, boolean):
--        """
--        Set this tab to active or inactive state
--        @param boolean: True to set active, False to set inactive
--        """
--        self.active = boolean
--
      def show_empty_tab_notice(self, title=_("Empty tab"),
              message_body=_("This tab doesn't contain any elements.")):
          """
@@ -95,23 +115,6 @@
          info.width = 0.57
          self.add(info)
--    def show_tab(self):
--        """
--        Activate this tab. This is called when tab is changed and this tab
--        is becoming active. This method should activate animations.
--        """
--        self.set_opacity(0)
--        self.show()
--        self.timeline.set_direction(clutter.TIMELINE_FORWARD)
--        self.timeline.start()
--
--    def hide_tab(self):
--        """
--        Deactivate this tab. This is called when tab is changed and this tab
--        is becoming non-active. This method should disable animations.
--        """
--        self.hide()
--
      def _handle_up(self):
          '''Dummy method for unimplemented handlers in subclass'''
          return False
 === modified file 'entertainerlib/frontend/gui/tabs/tracks_tab.py'
 --- entertainerlib/frontend/gui/tabs/tracks_tab.py	2009-02-03 23:37:57 +0000
 +++ entertainerlib/frontend/gui/tabs/tracks_tab.py	2009-04-22 10:28:29 +0000
@@ -67,37 +67,33 @@
          self.li.show()
          self.add(self.li)
++        self.connect('activated', self._on_activate)
++        self.connect('deactivated', self._on_deactivated)
++
      def can_activate(self):
          '''Tracks tab will always be created from an existing artist with at
          least one track'''
          return True
--    def set_active(self, boolean):
--        """
--        Override method from parent class.
--        @param boolean: True to set this tab active, False to set inactive
--        """
--        Tab.set_active(self, boolean)
--        if boolean:
--            self._update_track_info(
--                self.menu.get_current_menuitem().get_userdata())
--            self.menu.set_active(True)
--        else:
--            self.menu.set_active(False)
--
--    def _update_track_info(self, track):
++    def _update_track_info(self):
          '''Update the track information labels'''
--        self.track_title.set_text(track.get_title())
--        self.track_length.set_text(str(track.get_length() / 60) + ":" + \
--            str(track.get_length() % 60).zfill(2))
--        self.track_number.set_text(_("Track %(track)s from %(album)s") % \
--            {'track': track.get_tracknumber(),
--             'album': track.get_album().get_title()})
++        if self.active:
++            track = self.menu.get_current_menuitem().get_userdata()
++            self.track_title.set_text(track.get_title())
++            self.track_length.set_text(str(track.get_length() / 60) + ":" + \
++                str(track.get_length() % 60).zfill(2))
++            self.track_number.set_text(_("Track %(track)s from %(album)s") % \
++                {'track': track.get_tracknumber(),
++                'album': track.get_album().get_title()})
++        else:
++            self.track_title.set_text("")
++            self.track_length.set_text("")
++            self.track_number.set_text("")
      def _update_menu(self, direction):
          '''Update the menu based on the provided menu direction'''
          self.menu.move(direction)
--        self._update_track_info(self.menu.get_current_menuitem().get_userdata())
++        self._update_track_info()
          self.li.set_current(self.menu.get_current_position() + 1)
          return False
@@ -127,3 +123,17 @@
          self.callback("audio_play", kwargs)
          return False
++    def _on_activate(self, event=None):
++        '''Tab activation'''
++        self._update_track_info()
++        self.menu.set_active(True)
++        self.menu.set_opacity(255)
++        return False
++
++    def _on_deactivated(self, event=None):
++        '''Tab deactivated'''
++        self._update_track_info()
++        self.menu.set_active(False)
++        self.menu.set_opacity(128)
++        return False
++
 === modified file 'entertainerlib/frontend/gui/tabs/video_clips_tab.py'
 --- entertainerlib/frontend/gui/tabs/video_clips_tab.py	2009-02-03 23:37:57 +0000
 +++ entertainerlib/frontend/gui/tabs/video_clips_tab.py	2009-04-22 10:28:29 +0000
@@ -40,7 +40,8 @@
          else:
              self._create_menu()
--        self.hide_tab()
++            self.connect('activated', self._on_activate)
++            self.connect('deactivated', self._on_deactivated)
      def can_activate(self):
          """
@@ -51,19 +52,6 @@
          else:
              return True
--    def set_active(self, boolean):
--        """
--        Override method from parent class.
--        @param boolean: True to set this tab active, False to set inactive
--        """
--        Tab.set_active(self, boolean)
--        if boolean:
--            self._update_clip_info(
--                self.menu.get_current_menuitem().get_userdata())
--            self.menu.set_active(True)
--        else:
--            self.menu.set_active(False)
--
      def _create_empty_library_notice(self):
          """
          Create an information box that is displayed if there are no indexed
@@ -126,19 +114,21 @@
          self.clip_info.width = 0.5
          self.add(self.clip_info)
--    def _update_clip_info(self, clip):
--        """
--        Update VideoClip information. Updates clutter labels on Clips tab
--        @param clip: Currently selected VideoClip (VideoClip object)
--        """
--        (folder, filename) = os.path.split(clip.get_filename())
--        self.clip_title.set_text(filename)
--        self.clip_info.set_text(folder)
++    def _update_clip_info(self):
++        '''Update the VideoClip information labels'''
++        if self.active:
++            clip = self.menu.get_current_menuitem().get_userdata()
++            (folder, filename) = os.path.split(clip.get_filename())
++            self.clip_title.set_text(filename)
++            self.clip_info.set_text(folder)
++        else:
++            self.clip_title.set_text("")
++            self.clip_info.set_text("")
      def _update_menu(self, direction):
          '''Update the menu based on the provided menu direction'''
          self.menu.move(direction)
--        self._update_clip_info(self.menu.get_current_menuitem().get_userdata())
++        self._update_clip_info()
          self.list_indicator.set_current(self.menu.get_current_position() + 1)
          return False
@@ -169,3 +159,17 @@
          self.callback("video_osd")
          return False
++    def _on_activate(self, event=None):
++        '''Tab activation'''
++        self._update_clip_info()
++        self.menu.set_active(True)
++        self.menu.set_opacity(255)
++        return False
++
++    def _on_deactivated(self, event=None):
++        '''Tab deactivated'''
++        self._update_clip_info()
++        self.menu.set_active(False)
++        self.menu.set_opacity(128)
++        return False
++
 === modified file 'entertainerlib/frontend/gui/widgets/tab_group.py'
 --- entertainerlib/frontend/gui/widgets/tab_group.py	2009-02-27 00:01:53 +0000
 +++ entertainerlib/frontend/gui/widgets/tab_group.py	2009-04-16 18:52:06 +0000
@@ -7,11 +7,12 @@
  import clutter
  import pango
++from entertainerlib.frontend.gui.widgets.base import Base
  from entertainerlib.frontend.gui.widgets.label import Label
  from entertainerlib.frontend.gui.user_event import UserEvent
  from entertainerlib.utils.configuration import Configuration
--class TabGroup(clutter.Group):
++class TabGroup(Base, clutter.Group, object):
      """
      This is a top level container for tabs.
@@ -26,7 +27,9 @@
          @param height: height of the tab bar in percents (0-1)
          @param color: color string to call from the theme
          """
++        Base.__init__(self)
          clutter.Group.__init__(self)
++
          self.config = Configuration()
          self.active = False
@@ -112,16 +115,23 @@
          self.tabs_list.append(tab)
--        # Set first tab active
++        tab.active = False
++
++        # Set first tab visible
          if self.current_tab == None:
              self.current_tab = 0
--            self.tabs_list[0].show_tab()
++            self.tabs_list[0].visible = True
          tab_title = Label(0.01, self.color, 0, 0, tab.title)
++        self.labels.append(tab_title)
++        tab_title.set_name(tab.name)
++
          self.add(tab_title)
--        self.labels.append(tab_title)
          self._reposition_labels()
++        tab_title.set_reactive(True)
++        tab_title.connect('button-press-event', self._on_tab_title_button_press)
++
          timeline = clutter.Timeline(25, 60)
          alpha = clutter.Alpha(timeline, clutter.smoothstep_inc_func)
          behaviour = clutter.BehaviourOpacity(255, 96, alpha)
@@ -151,44 +161,45 @@
              label.set_line_wrap(False)
              label.set_ellipsize(pango.ELLIPSIZE_END)
++    def _activate_tab(self, new_tab):
++        """Activation of new selected Tab"""
++        self.timelines[self.current_tab][0].set_direction(
++            clutter.TIMELINE_FORWARD)
++        self.timelines[self.current_tab][0].start()
++
++        self.timelines[new_tab][0].set_direction(clutter.TIMELINE_BACKWARD)
++        self.timelines[new_tab][0].start()
++
++        # Change visible tab
++        self.tabs_list[self.current_tab].visible = False
++        self.tabs_list[self.current_tab].active = False
++        self.tabs_list[new_tab].visible = True
++        self.tabs_list[new_tab].active = False
++
      def _switch_tab_to_left(self):
          """
          Switch one tab to left if possible. Do nothing if there is no tab
          """
          if self.current_tab > 0 and self.current_tab is not None:
++            self._activate_tab(self.current_tab - 1)
              self.current_tab = self.current_tab - 1
--            # Animate labels
--            self.timelines[self.current_tab + 1][0].set_direction(
--                clutter.TIMELINE_FORWARD)
--            self.timelines[self.current_tab + 1][0].start()
--            self.timelines[self.current_tab][0].set_direction(
--                clutter.TIMELINE_BACKWARD)
--            self.timelines[self.current_tab][0].start()
--
--            # Change visible tab
--            self.tabs_list[self.current_tab + 1].hide_tab()
--            self.tabs_list[self.current_tab].show_tab()
--
      def _switch_tab_to_right(self):
          """
          Switch one tab to right if possible. Do nothing if there is no tab
          """
          if self.current_tab  < len(self.tabs_list) - 1 and (
              self.current_tab is not None):
++            self._activate_tab(self.current_tab + 1)
              self.current_tab = self.current_tab + 1
--            # Animate labels
--            self.timelines[self.current_tab - 1][0].set_direction(
--                clutter.TIMELINE_FORWARD)
--            self.timelines[self.current_tab - 1][0].start()
--            self.timelines[self.current_tab][0].set_direction(
--                clutter.TIMELINE_BACKWARD)
--            self.timelines[self.current_tab][0].start()
--
--            # Change visible tab
--            self.tabs_list[self.current_tab - 1].hide_tab()
--            self.tabs_list[self.current_tab].show_tab()
++    def _switch_tab_to_down(self):
++        """
++        Switch down from current tab if allowed
++        """
++        if self.tabs_list[self.current_tab].can_activate():
++            self.set_active(False)
++            self.tabs_list[self.current_tab].active = True
      def handle_user_event(self, event):
          """
@@ -201,13 +212,19 @@
                  self._switch_tab_to_left()
              elif event_type == UserEvent.NAVIGATE_RIGHT:
                  self._switch_tab_to_right()
--            elif event_type == UserEvent.NAVIGATE_DOWN and (
--                self.tabs_list[self.current_tab].can_activate()):
--                self.set_active(False)
--                self.tabs_list[self.current_tab].set_active(True)
++            elif event_type == UserEvent.NAVIGATE_DOWN:
++                self._switch_tab_to_down()
          else:
              result = self.tabs_list[self.current_tab].handle_user_event(event)
              if result:
                  self.set_active(True)
--                self.tabs_list[self.current_tab].set_active(False)
++                self.tabs_list[self.current_tab].active = False
++
++    def _on_tab_title_button_press(self, actor, event):
++        '''Title Label button-press handler'''
++        clicked_tab = self._tabs[actor.get_name()]
++        clicked_tab_index = self.tabs_list.index(clicked_tab)
++
++        self._activate_tab(clicked_tab_index)
++        self.current_tab = clicked_tab_index

Entertainer Media Center

Merge lp://staging/~samuel-buffet/entertainer/reactive_tabs into lp://staging/entertainer

Commit message

Description of the change

Preview Diff

Subscribers