Code review part 3

3 files changed, 67 insertions, 42 deletions

diff --git a/activity.py b/activity.py
index 851e627..7b9bec3 100644
--- a/activity.py
+++ b/activity.py
@@ -38,7 +38,7 @@ from StringIO import StringIO
 
 import animation
 from frames_list import FramesList
-from toolbars import AnimateToolbarbox
+from toolbars import AnimateToolbarBox
 from toolbars import FramesToolbar
 
 
@@ -49,7 +49,7 @@ class AnimateActivity(activity.Activity):
 
         self.max_participants = 1
 
-        toolbarbox = AnimateToolbarbox()
+        toolbarbox = AnimateToolbarBox(self)
 
         self.set_toolbar_box(toolbarbox)
         toolbarbox.show_all()
@@ -58,10 +58,15 @@ class AnimateActivity(activity.Activity):
 
         self._animation = None
         self._animation_frames = []
-        self.animation_mode = animation.RETURN_MODE
+        self.animation_mode = animation.MODE_RETURN
         self.modes_buttons = toolbarbox.modes_buttons
 
         self._frames_list = FramesList()
+        self._frames_list.connect("current-frame-changed",
+                                  self._current_frame_changed_cb)
+        self._frames_list.connect("move", self._move_cb)
+        self._frames_list.connect('get-current-frame',
+                                lambda w: self._animation.get_current_frame())
         self._frames_list.show()
 
         vbox = gtk.VBox()
@@ -78,6 +83,12 @@ class AnimateActivity(activity.Activity):
 
         self.show()
 
+    def _move_cb(self, index, new_index):
+        self._animation.move(index, new_index)
+
+    def _current_frame_changed_cb(self, index):
+        self._animation.set_current_frame(index)
+
     def write_file(self, file_path):
         self.metadata["mime_type"] = "application/x-animate-activity"
         zfile = zipfile.ZipFile(file_path, 'w')
@@ -190,13 +201,19 @@ class AnimateActivity(activity.Activity):
                 self._animation.add_frame(pixbuf)
                 self._frames_list.add_frame(pixbuf)
             self._animation.set_fps(self._fpsbutton.get_value())
+            self._animation.connect("current-frame-updated",
+                                    self._current_frame_updated_cb)
             self._animation.show_all()
 
             del self._animation_mode
 
-        self._frames_list._animation = self._animation
         canvas.pack_start(self._animation, True, True, 0)
 
+    def _current_frame_updated_cb(self, widget, index):
+        if widget.frames_reverse:
+            index = len(widget.frames) - 1 - index
+        self._frames_list.select_frame(index)
+
     def _view_fullscreen(self, widget):
         self._frames_list.hide()
         self._animation.set_size(gtk.gdk.screen_width(),
diff --git a/frames_list.py b/frames_list.py
index cbd5fec..298eaaa 100644
--- a/frames_list.py
+++ b/frames_list.py
@@ -21,11 +21,22 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 # MA 02110-1301, USA.
 
+import gobject
 import gtk
 
 
 class FramesList(gtk.ScrolledWindow):
 
+    __gsignals__ = {"current-frame-changed": (gobject.SIGNAL_RUN_LAST,
+                                              gobject.TYPE_NONE,
+                                              (gobject.TYPE_INT,)),
+                    "move": (gobject.SIGNAL_RUN_LAST,
+                                gobject.TYPE_NONE,
+                                (gobject.TYPE_INT, gobject.TYPE_INT,)),
+                    "get-current-frame": (gobject.SIGNAL_RUN_LAST,
+                                          gobject.TYPE_INT,
+                                          tuple())}
+
     def __init__(self):
         super(FramesList, self).__init__()
 
@@ -74,9 +85,7 @@ class FramesList(gtk.ScrolledWindow):
             count += 1
         self.frames = count - 1
 
-    def _select_frame(self, widget, index):
-        if widget.frames_reverse:
-            index = len(widget.frames) - 1 - index
+    def select_frame(self, index):
         self.selection.select_iter(self.store.get_iter(index))
         selected_path = self.store.get_path(self.selection.get_selected()[1])
         self.treeview.scroll_to_cell(selected_path)
@@ -92,19 +101,16 @@ class FramesList(gtk.ScrolledWindow):
 
         if self.frames == 1:
             self.selection.select_iter(_iter)
-        if self._animation and not self._animation_connected:
-            self._animation.connect("current-frame-updated", self._select_frame)
-            self._animation_connected = True
 
     def _selection_changed_cb(self, selection):
         model, _iter = selection.get_selected()
         try:
             if self._animation.frames_reverse:
-                self._animation.set_current_frame(
-                                        self._animation.get_current_frame())
+                self.emit('current-frame-changed',
+                          self.emit('get-current-frame'))
             else:
-                self._animation.set_current_frame(self.store.get_value(_iter,
-                                                                        0) - 1)
+                self.emit('current-frame-changed',
+                          self.store.get_value(_iter, 0) - 1)
         except:
             pass
 
@@ -116,7 +122,8 @@ class FramesList(gtk.ScrolledWindow):
 
         selected_path = self.store.get_path(selected_iter)[0]
         new_position_path = self.store.get_path(position)[0]
-        self._animation.move(selected_path, new_position_path)
+        #self._animation.move(selected_path, new_position_path)
+        self.emit('move', selected_path, new_position_path)
 
     def move_down(self, widget):
         selected_iter = self.selection.get_selected()[1]
@@ -126,4 +133,5 @@ class FramesList(gtk.ScrolledWindow):
 
         selected_path = self.store.get_path(selected_iter)[0]
         new_position_path = self.store.get_path(position)[0]
-        self._animation.move(selected_path, new_position_path)
+        #self._animation.move(selected_path, new_position_path)
+        self.emit('move', selected_path, new_position_path)
diff --git a/toolbars.py b/toolbars.py
index 9b2c2d4..5e9a316 100644
--- a/toolbars.py
+++ b/toolbars.py
@@ -26,19 +26,19 @@ import gtk
 from sugar.activity.widgets import StopButton
 from sugar.activity.widgets import ToolbarButton
 from sugar.activity.widgets import ActivityToolbarButton
-from sugar.graphics.toolbarbox import Toolbarbox
+from sugar.graphics.toolbarbox import ToolbarBox
 from sugar.graphics.toolbutton import ToolButton
 from sugar.graphics.radiotoolbutton import RadioToolButton
 
 from gettext import gettext as _
 
 
-class AnimateToolbarbox(Toolbarbox):
+class AnimateToolbarBox(ToolbarBox):
 
-    def __init__(self):
-        super(AnimateToolbarbox, self).__init__()
+    def __init__(self, activity):
+        super(AnimateToolbarBox, self).__init__()
 
-        activity_button = ActivityToolbarButton(self)
+        activity_button = ActivityToolbarButton(activity)
         self.toolbar.insert(activity_button, 0)
 
         separator = gtk.SeparatorToolItem()
@@ -46,12 +46,12 @@ class AnimateToolbarbox(Toolbarbox):
 
         add_frame = ToolButton('list-add')
         add_frame.set_tooltip(_('Add a frame'))
-        add_frame.connect('clicked', self._add_frame)
+        #add_frame.connect('clicked', self._add_frame)
         self.toolbar.insert(add_frame, -1)
 
         remove_frame = ToolButton('list-remove')
         remove_frame.set_tooltip(_('Remove the selected frame'))
-        remove_frame.connect('clicked', self._remove_frame)
+        #remove_frame.connect('clicked', self._remove_frame)
         self.toolbar.insert(remove_frame, -1)
 
         separator = gtk.SeparatorToolItem()
@@ -59,17 +59,17 @@ class AnimateToolbarbox(Toolbarbox):
 
         back_btn = ToolButton('media-seek-backward')
         back_btn.set_tooltip(_('Previous Frame'))
-        back_btn.connect('clicked', self._previous_frame)
+        #back_btn.connect('clicked', self._previous_frame)
         self.toolbar.insert(back_btn, -1)
 
         run_btn = ToolButton('media-playback-start')
         run_btn.set_tooltip(_('Run animation'))
-        run_btn.connect('clicked', self._run_pause_animation)
+        #run_btn.connect('clicked', self._run_pause_animation)
         self.toolbar.insert(run_btn, -1)
 
         next_btn = ToolButton('media-seek-forward')
         next_btn.set_tooltip(_('Next Frame'))
-        next_btn.connect('clicked', self._next_frame)
+        #next_btn.connect('clicked', self._next_frame)
         self.toolbar.insert(next_btn, -1)
 
         separator = gtk.SeparatorToolItem()
@@ -78,22 +78,22 @@ class AnimateToolbarbox(Toolbarbox):
         options_button = ToolbarButton(icon_name='preferences-system')
         options_toolbar = gtk.Toolbar()
 
-        returnbutton = RadioToolButton(icon_name="media-playlist-repeat")
-        returnbutton.set_tooltip(_("Repeat Mode"))
+        returnbutton = RadioToolButton(icon_name='media-playlist-repeat')
+        returnbutton.set_tooltip(_('Repeat Mode'))
         returnbutton.set_active(True)
-        returnbutton.connect("clicked", self._set_return_mode)
+        #returnbutton.connect('clicked', self._set_return_mode)
         options_toolbar.insert(returnbutton, -1)
 
         normalmode = RadioToolButton(icon_name='normal-mode')
         normalmode.props.group = returnbutton
         normalmode.set_tooltip(_('Normal Mode'))
-        normalmode.connect("clicked", self._set_normal_mode)
+        #normalmode.connect('clicked', self._set_normal_mode)
         options_toolbar.insert(normalmode, -1)
 
         pingpong = RadioToolButton(icon_name='ping-pong')
         pingpong.props.group = returnbutton
-        pingpong.set_tooltip(_("Ping Pong Mode"))
-        pingpong.connect("clicked", self._set_pingpong_mode)
+        pingpong.set_tooltip(_('Ping Pong Mode'))
+        #pingpong.connect('clicked', self._set_pingpong_mode)
         options_toolbar.insert(pingpong, -1)
 
         self.modes_buttons = [normalmode, returnbutton, pingpong]
@@ -102,7 +102,7 @@ class AnimateToolbarbox(Toolbarbox):
         options_toolbar.insert(separator, -1)
 
         toollabel = gtk.ToolItem()
-        toollabel.add(gtk.Label(_("Frames per second:")))
+        toollabel.add(gtk.Label(_('Frames per second:')))
         options_toolbar.insert(toollabel, -1)
 
         separator = gtk.SeparatorToolItem()
@@ -112,7 +112,7 @@ class AnimateToolbarbox(Toolbarbox):
         toolspin = gtk.ToolItem()
         adjustment = gtk.Adjustment(2, 1, 24, 1, 1)
         self._fpsbutton = gtk.SpinButton(adjustment, 0, 0)
-        self._fpsbutton.connect('value-changed', self._fps_changed_cb)
+        #self._fpsbutton.connect('value-changed', self._fps_changed_cb)
         toolspin.add(self._fpsbutton)
         options_toolbar.insert(toolspin, -1)
 
@@ -126,7 +126,7 @@ class AnimateToolbarbox(Toolbarbox):
 
         fullscreen_btn = ToolButton('view-fullscreen')
         fullscreen_btn.set_tooltip(_('Fullscreen'))
-        fullscreen_btn.connect("clicked", self._view_fullscreen)
+        #fullscreen_btn.connect('clicked', self._view_fullscreen)
 
         self.toolbar.insert(fullscreen_btn, -1)
 
@@ -135,7 +135,7 @@ class AnimateToolbarbox(Toolbarbox):
         separator.set_draw(False)
         self.toolbar.insert(separator, -1)
 
-        stopbtn = StopButton(self)
+        stopbtn = StopButton(activity)
         self.toolbar.insert(stopbtn, -1)
 
 
@@ -144,12 +144,12 @@ class FramesToolbar(gtk.Toolbar):
     def __init__(self):
         super(FramesToolbar, self).__init__()
 
-        move_up = ToolButton("go-up")
-        move_up.set_tooltip(_("Move up"))
-        move_up.connect('clicked', self._frames_list.move_up)
+        move_up = ToolButton('go-up')
+        move_up.set_tooltip(_('Move up'))
+        #move_up.connect('clicked', self._frames_list.move_up)
         self.insert(move_up, -1)
 
-        move_down = ToolButton("go-down")
-        move_down.set_tooltip(_("Move down"))
-        move_down.connect('clicked', self._frames_list.move_down)
+        move_down = ToolButton('go-down')
+        move_down.set_tooltip(_('Move down'))
+        #move_down.connect('clicked', self._frames_list.move_down)
         self.insert(move_down, -1)