[gnome-music/wip/mschraal/remove-playlists-loaded-signal: 11/11] Let consumers handle playlists availability



commit f0339a190ea2b7e616778ff9c5bdbf7826b3665b
Author: Marinus Schraal <mschraal gnome org>
Date:   Thu Jan 9 16:06:39 2020 +0100

    Let consumers handle playlists availability
    
    PlaylistsView and MPRIS want to know when playlists become available to
    setup some actions. This was previously done by emitting a signal after
    the initial load of playlists. However, in certain  conditions this
    signal could already have fired before the consumer was even read, so no
    initial action would be taken.
    
    For example, PlaylistsView by default selects a row for intial display
    when the list is filled. However, without a signal being received, no
    row is selected to display.
    
    Now PlaylistsView instead directly listens to the playlists model for
    changes and uses an initial state variable to check if it is needed to
    activate a row.
    
    Closes: #348

 gnomemusic/coremodel.py                         |  1 -
 gnomemusic/grilowrappers/grltrackerplaylists.py |  1 -
 gnomemusic/mpris.py                             | 37 +++++++++----------------
 gnomemusic/views/playlistsview.py               | 27 +++++++++++-------
 4 files changed, 30 insertions(+), 36 deletions(-)
---
diff --git a/gnomemusic/coremodel.py b/gnomemusic/coremodel.py
index a8579159..37c8f230 100644
--- a/gnomemusic/coremodel.py
+++ b/gnomemusic/coremodel.py
@@ -68,7 +68,6 @@ class CoreModel(GObject.GObject):
     __gsignals__ = {
         "artists-loaded": (GObject.SignalFlags.RUN_FIRST, None, ()),
         "playlist-loaded": (GObject.SignalFlags.RUN_FIRST, None, (int,)),
-        "playlists-loaded": (GObject.SignalFlags.RUN_FIRST, None, ()),
     }
 
     active_playlist = GObject.Property(type=Playlist, default=None)
diff --git a/gnomemusic/grilowrappers/grltrackerplaylists.py b/gnomemusic/grilowrappers/grltrackerplaylists.py
index 25acde5e..8c582f97 100644
--- a/gnomemusic/grilowrappers/grltrackerplaylists.py
+++ b/gnomemusic/grilowrappers/grltrackerplaylists.py
@@ -148,7 +148,6 @@ class GrlTrackerPlaylists(GObject.GObject):
             self._window.notifications_popup.pop_loading()
             return
         if not media:
-            self._coremodel.emit("playlists-loaded")
             self._window.notifications_popup.pop_loading()
             return
 
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index 03408fa8..60205d07 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -303,8 +303,11 @@ class MPRIS(DBusInterface):
             "playlist-loaded", self._on_player_playlist_changed)
 
         self._playlists_model = self._coremodel.props.playlists_sort
-        self._playlists_loaded_id = self._coremodel.connect(
-            "playlists-loaded", self._on_playlists_loaded)
+        n_items = self._playlists_model.get_n_items()
+        self._on_playlists_items_changed(
+            self._playlists_model, 0, n_items, n_items)
+        self._playlists_model.connect(
+            "items-changed", self._on_playlists_items_changed)
 
         self._player_playlist_type = None
         self._path_list = []
@@ -607,33 +610,19 @@ class MPRIS(DBusInterface):
         self._properties_changed(
             MPRIS.MEDIA_PLAYER2_PLAYLISTS_IFACE, properties, [])
 
-    def _on_playlists_loaded(self, klass):
-        self._coremodel.disconnect(self._playlists_loaded_id)
-        for playlist in self._playlists_model:
-            playlist.connect("notify::title", self._on_playlist_renamed)
-            playlist.connect(
-                "notify::active", self._on_player_playlist_changed)
-
-        self._playlists_model.connect(
-            "items-changed", self._on_playlists_count_changed)
-        self._on_playlists_count_changed(None, None, 0, 0)
-
-    @log
-    def _on_playlists_count_changed(self, model, position, removed, added):
-        playlist_count = self._playlists_model.get_n_items()
-        if playlist_count == self._previous_playlist_count:
-            return
+    def _on_playlists_items_changed(self, model, position, removed, added):
+        if added > 0:
+            for i in list(range(added)):
+                playlist = model[position + i]
+                playlist.connect("notify::title", self._on_playlist_renamed)
+                playlist.connect(
+                    "notify::active", self._on_player_playlist_changed)
 
-        self._previous_playlist_count = playlist_count
+        playlist_count = model.get_n_items()
         properties = {"PlaylistCount": GLib.Variant("u", playlist_count)}
         self._properties_changed(
             MPRIS.MEDIA_PLAYER2_PLAYLISTS_IFACE, properties, [])
 
-        if added == 0:
-            return
-
-        model[position].connect("notify::title", self._on_playlist_renamed)
-
     @log
     def _on_playlist_renamed(self, playlist, param):
         mpris_playlist = self._get_mpris_playlist_from_playlist(playlist)
diff --git a/gnomemusic/views/playlistsview.py b/gnomemusic/views/playlistsview.py
index 3a272670..6656b1cb 100644
--- a/gnomemusic/views/playlistsview.py
+++ b/gnomemusic/views/playlistsview.py
@@ -62,6 +62,8 @@ class PlaylistsView(BaseView):
         self._window = application.props.window
         self._player = player
 
+        self._clean_fill = True
+
         self._song_popover = PlaylistContextMenu(application, self._view)
 
         play_song = self._window.lookup_action("play_song")
@@ -91,11 +93,12 @@ class PlaylistsView(BaseView):
 
         self._sidebar.bind_model(self._model, self._add_playlist_to_sidebar)
 
-        self._loaded_id = self._coremodel.connect(
-            "playlists-loaded", self._on_playlists_loaded)
         self._active_playlist_id = self._coremodel.connect(
             "notify::active-playlist", self._on_active_playlist_changed)
 
+        self._model.connect("items-changed", self._on_playlists_model_changed)
+        self._on_playlists_model_changed(self._model, 0, 0, 0)
+
         # Selection is only possible from the context menu
         self.disconnect(self._selection_mode_id)
 
@@ -130,15 +133,17 @@ class PlaylistsView(BaseView):
         row = PlaylistTile(playlist)
         return row
 
-    def _on_playlists_loaded(self, klass):
-        self._coremodel.disconnect(self._loaded_id)
-        self._model.connect("items-changed", self._on_playlists_model_changed)
-
-        first_row = self._sidebar.get_row_at_index(0)
-        self._sidebar.select_row(first_row)
-        self._on_playlist_activated(self._sidebar, first_row)
-
     def _on_playlists_model_changed(self, model, position, removed, added):
+        if model.get_n_items() == 0:
+            self._clean_fill = True
+            return
+        elif self._clean_fill:
+            first_row = self._sidebar.get_row_at_index(0)
+            self._sidebar.select_row(first_row)
+            self._on_playlist_activated(self._sidebar, first_row)
+            self._clean_fill = True
+            return
+
         if removed == 0:
             return
 
@@ -147,6 +152,7 @@ class PlaylistsView(BaseView):
         if row_next:
             self._sidebar.select_row(row_next)
             self._on_playlist_activated(self._sidebar, row_next)
+            self._clean_fill = True
 
     @log
     def _on_view_right_clicked(self, gesture, n_press, x, y):
@@ -201,6 +207,7 @@ class PlaylistsView(BaseView):
     @log
     def _on_playlist_activated(self, sidebar, row, data=None):
         """Update view with content from selected playlist"""
+        self._clean_fill = False
         playlist = row.props.playlist
 
         self._view.bind_model(


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]