[pitivi] timeline: Update the layer widgets when a layer is added

[Alexandru Băluț <alexbalut src gnome org>]

commit 879e7eb98d0eabe149496436a583be320767da45
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Wed Feb 8 01:52:36 2017 +0100

    timeline: Update the layer widgets when a layer is added
    
    When undoing a layer removal, first the layer prios were being restored,
    then the layer was being added. The timeline was updating the layer
    widgets only when the priority of a layer changed.
    
    Now the layer widgets are updated also when a layer is added.
    
    Fixes https://phabricator.freedesktop.org/T7695
    
    Reviewed-by: Thibault Saunier <tsaunier gnome org>
    Differential Revision: https://phabricator.freedesktop.org/D1650

 pitivi/timeline/timeline.py |   25 +++++++++++++++--------
 tests/test_undo_timeline.py |   45 ++++++++++++++++++++++++------------------
 2 files changed, 42 insertions(+), 28 deletions(-)
---
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index e70bf8b..51e9e36 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -442,7 +442,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
             self.disconnect_by_func(self._motion_notify_event_cb)
 
             self.ges_timeline.disconnect_by_func(self._durationChangedCb)
-            self.ges_timeline.disconnect_by_func(self._layerAddedCb)
+            self.ges_timeline.disconnect_by_func(self._layer_added_cb)
             self.ges_timeline.disconnect_by_func(self._layerRemovedCb)
             self.ges_timeline.disconnect_by_func(self._snapCb)
             self.ges_timeline.disconnect_by_func(self._snapEndedCb)
@@ -463,10 +463,10 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         self.ges_timeline.ui = self
 
         for ges_layer in self.ges_timeline.get_layers():
-            self._addLayer(ges_layer)
+            self._add_layer(ges_layer)
 
         self.ges_timeline.connect("notify::duration", self._durationChangedCb)
-        self.ges_timeline.connect("layer-added", self._layerAddedCb)
+        self.ges_timeline.connect("layer-added", self._layer_added_cb)
         self.ges_timeline.connect("layer-removed", self._layerRemovedCb)
         self.ges_timeline.connect("snapping-started", self._snapCb)
         self.ges_timeline.connect("snapping-ended", self._snapEndedCb)
@@ -938,8 +938,9 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
                 self.dropDataReady = True
 
     # Handle layers
-    def _layerAddedCb(self, timeline, ges_layer):
-        self._addLayer(ges_layer)
+    def _layer_added_cb(self, unused_ges_timeline, ges_layer):
+        self._add_layer(ges_layer)
+        self.__update_layers()
 
     def moveLayer(self, ges_layer, index):
         self.debug("Moving layer %s to %s", ges_layer.props.priority, index)
@@ -950,7 +951,8 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
             if ges_layer.props.priority != i:
                 ges_layer.props.priority = i
 
-    def _addLayer(self, ges_layer):
+    def _add_layer(self, ges_layer):
+        """Adds widgets for controlling and showing the specified layer."""
         layer = Layer(ges_layer, self)
         ges_layer.ui = layer
 
@@ -970,7 +972,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
 
         self.__add_separators()
 
-        ges_layer.connect("notify::priority", self.__layerPriorityChangedCb)
+        ges_layer.connect("notify::priority", self.__layer_priority_changed_cb)
 
     def __add_separators(self):
         """Adds separators to separate layers."""
@@ -984,7 +986,12 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
 
         self._separators.append((controls_separator, separator))
 
-    def __layerPriorityChangedCb(self, ges_layer, pspec):
+    def __layer_priority_changed_cb(self, unused_ges_layer, unused_pspec):
+        """Handles the changing of a layer's priority."""
+        self.__update_layers()
+
+    def __update_layers(self):
+        """Updates the layer widgets if their priorities are in good order."""
         ges_layers = self.ges_timeline.get_layers()
         priorities = [ges_layer.props.priority for ges_layer in ges_layers]
         if priorities != list(range(len(priorities))):
@@ -998,7 +1005,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         self.info("Removing layer: %s", ges_layer.props.priority)
         self.layout.layers_vbox.remove(ges_layer.ui)
         self._layers_controls_vbox.remove(ges_layer.control_ui)
-        ges_layer.disconnect_by_func(self.__layerPriorityChangedCb)
+        ges_layer.disconnect_by_func(self.__layer_priority_changed_cb)
 
         # Remove extra separators.
         controls_separator, separator = self._separators.pop()
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index 2a4d833..a8d1776 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -37,6 +37,7 @@ from pitivi.undo.undo import PropertyChangedAction
 from pitivi.utils.ui import LAYER_HEIGHT
 from pitivi.utils.ui import URI_TARGET_ENTRY
 from tests import common
+from tests.test_timeline_timeline import TestLayers
 
 
 class BaseTestUndoTimeline(TestCase):
@@ -91,6 +92,10 @@ class BaseTestUndoTimeline(TestCase):
                     if isinstance(element, GES.VideoTransition):
                         return element
 
+    def check_layers(self, layers):
+        self.assertEqual(self.timeline.get_layers(), layers)
+        TestLayers.check_priorities_and_positions(self, self.timeline.ui, layers, list(range(len(layers))))
+
 
 class TestTimelineObserver(BaseTestUndoTimeline):
 
@@ -100,26 +105,28 @@ class TestTimelineObserver(BaseTestUndoTimeline):
         layer1 = self.layer
         layer2 = self.timeline.append_layer()
         layer3 = self.timeline.append_layer()
+        self.check_layers([layer1, layer2, layer3])
+        self.check_removal(self.timeline.get_layers())
 
-        self.assertEqual([layer1, layer2, layer3], self.timeline.get_layers())
-        self.assertEqual([l.props.priority for l in [layer1, layer2, layer3]],
-                         list(range(3)))
+    def check_removal(self, ges_layers):
+        for ges_layer in ges_layers:
+            remaining_layers = list(ges_layers)
+            remaining_layers.remove(ges_layer)
 
-        with self.action_log.started("layer removed"):
-            self.timeline.remove_layer(layer2)
+            with self.action_log.started("layer removed"):
+                self.timeline.remove_layer(ges_layer)
+            self.check_layers(remaining_layers)
 
-        self.assertEqual([layer1, layer3], self.timeline.get_layers())
-        self.assertEqual([l.props.priority for l in [layer1, layer3]],
-                         list(range(2)))
+            self.action_log.undo()
+            self.check_layers(ges_layers)
 
-        self.action_log.undo()
-        self.assertEqual([layer1, layer2, layer3], self.timeline.get_layers())
-        self.assertEqual([l.props.priority for l in [layer1, layer2, layer3]],
-                         list(range(3)))
-        self.action_log.redo()
-        self.assertEqual([layer1, layer3], self.timeline.get_layers())
-        self.assertEqual([l.props.priority for l in [layer1, layer3]],
-                         list(range(2)))
+            self.action_log.redo()
+            self.check_layers(remaining_layers)
+
+            self.check_removal(remaining_layers)
+
+            self.action_log.undo()
+            self.check_layers(ges_layers)
 
     def test_group_ungroup_clips(self):
         self.setup_timeline_container()
@@ -250,13 +257,13 @@ class TestLayerObserver(BaseTestUndoTimeline):
                     timeline_ui._motion_notify_event_cb(None, event=event)
 
             timeline_ui._button_release_event_cb(None, event=event)
-        self.assertEqual(self.timeline.get_layers(), [layer2, layer3, layer1])
+        self.check_layers([layer2, layer3, layer1])
 
         self.action_log.undo()
-        self.assertEqual(self.timeline.get_layers(), [layer1, layer2, layer3])
+        self.check_layers([layer1, layer2, layer3])
 
         self.action_log.redo()
-        self.assertEqual(self.timeline.get_layers(), [layer2, layer3, layer1])
+        self.check_layers([layer2, layer3, layer1])
 
     def test_layer_renamed(self):
         layer = Layer(self.layer, timeline=mock.Mock())