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



commit ace751a53cf92e072db3fecaa0c2af2a87a7c685
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 acd626a9..546e2852 100644
--- a/gnomemusic/grilowrappers/grltrackerplaylists.py
+++ b/gnomemusic/grilowrappers/grltrackerplaylists.py
@@ -145,7 +145,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 a1e2dce9..9794ce05 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)
-        first_row.emit("activate")
-
     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)
+            first_row.emit("activate")
+            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)
             row_next.emit("activate")
+            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]