Closes #192 Play queue resume is non-modal

Also added infrastructure for showing other non-modal notifications

CHANGELOG.rst

@@ -5,6 +5,9 @@ v0.9.3
 
     * The Albums tab is now paginated.
     * You can sort the Albums tab ascending or descending.
+    * The amount of the song that is cached is now shown while streaming a song.
+    * The dialog for resuming a play queue from another device has been greatly
+      improved and is now less intrusive.
 
 * This release has a ton of under-the-hood changes to make things more robust
   and performant.

sublime/adapters/subsonic/api_objects.py

@@ -2,6 +2,7 @@
 These are the API objects that are returned by Subsonic.
 """
 
+import hashlib
 from dataclasses import asdict, dataclass, field
 from datetime import datetime, timedelta
 from typing import Any, Dict, List, Optional, Union
@@ -81,7 +82,8 @@ class Album(SublimeAPI.Album):
 @dataclass_json(letter_case=LetterCase.CAMEL)
 @dataclass
 class ArtistAndArtistInfo(SublimeAPI.Artist):
-    id: str
+    id: str = field(init=False)
+    _id: Optional[str] = field(metadata=config(field_name="id"))
     name: str
     albums: List[Album] = field(
         default_factory=list, metadata=config(field_name="album")
@@ -99,7 +101,16 @@ class ArtistAndArtistInfo(SublimeAPI.Artist):
     music_brainz_id: Optional[str] = None
     last_fm_url: Optional[str] = None
 
+    @staticmethod
+    def _strhash(string: str) -> str:
+        return hashlib.sha1(bytes(string, "utf8")).hexdigest()
+
     def __post_init__(self):
+        self.id = (
+            self._id
+            if self._id is not None
+            else ArtistAndArtistInfo._strhash(self.name)
+        )
         self.album_count = self.album_count or len(self.albums)
         if not self.artist_image_url:
             self.artist_image_url = self.cover_art

sublime/app.py

@@ -40,7 +40,7 @@ from .players import ChromecastPlayer, MPVPlayer, Player, PlayerEvent
 from .ui.configure_servers import ConfigureServersDialog
 from .ui.main import MainWindow
 from .ui.settings import SettingsDialog
-from .ui.state import RepeatType
+from .ui.state import RepeatType, UIState
 
 
 class SublimeMusicApp(Gtk.Application):
@@ -128,6 +128,7 @@ class SublimeMusicApp(Gtk.Application):
         self.window.connect("song-clicked", self.on_song_clicked)
         self.window.connect("songs-removed", self.on_songs_removed)
         self.window.connect("refresh-window", self.on_refresh_window)
+        self.window.connect("notification-closed", self.on_notification_closed)
         self.window.connect("go-to", self.on_window_go_to)
         self.window.connect("key-press-event", self.on_window_key_press)
         self.window.player_controls.connect("song-scrub", self.on_song_scrub)
@@ -496,6 +497,10 @@ class SublimeMusicApp(Gtk.Application):
             setattr(self.app_config.state, k, v)
         self.update_window(force=force)
 
+    def on_notification_closed(self, _):
+        self.app_config.state.current_notification = None
+        self.update_window()
+
     def on_configure_servers(self, *args):
         self.show_configure_servers_dialog()
 
@@ -915,13 +920,6 @@ class SublimeMusicApp(Gtk.Application):
                         return
 
                 # TODO (#167): info bar here (maybe pop up above the player controls?)
-                dialog = Gtk.MessageDialog(
-                    transient_for=self.window,
-                    message_type=Gtk.MessageType.INFO,
-                    buttons=Gtk.ButtonsType.YES_NO,
-                    text="Resume Playback?",
-                )
-
                 resume_text = "Do you want to resume the play queue"
                 if play_queue.changed_by or play_queue.changed:
                     resume_text += " saved"
@@ -934,21 +932,25 @@ class SublimeMusicApp(Gtk.Application):
                         resume_text += f" at {changed_str}"
                 resume_text += "?"
 
-                dialog.format_secondary_markup(resume_text)
+                def on_resume_click():
-                result = dialog.run()
-                dialog.destroy()
-                if result == Gtk.ResponseType.YES:
                     self.app_config.state.play_queue = new_play_queue
                     self.app_config.state.song_progress = play_queue.position
                     self.app_config.state.current_song_index = (
                         play_queue.current_index or 0
                     )
                     self.player.reset()
+                    self.app_config.state.current_notification = None
                     self.update_window()
 
                     if was_playing:
                         self.on_play_pause()
 
+                self.app_config.state.current_notification = UIState.UINotification(
+                    markup=f"<b>{resume_text}</b>",
+                    actions=(("Resume", on_resume_click),),
+                )
+                self.update_window()
+
         play_queue_future = AdapterManager.get_play_queue()
         play_queue_future.add_done_callback(lambda f: GLib.idle_add(do_update, f))
 

sublime/config.py

@@ -158,7 +158,6 @@ class AppConfiguration:
 
         # Do the import in the function to avoid circular imports.
         from sublime.adapters import AdapterManager
-
         AdapterManager.reset(self)
 
     @property

sublime/ui/albums.py

@@ -557,6 +557,7 @@ class AlbumsGrid(Gtk.Overlay):
         self.detail_box = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)
         self.detail_box.pack_start(Gtk.Box(), True, True, 0)
 
+        # TODO wrap in revealer?
         self.detail_box_inner = Gtk.Box()
         self.detail_box.pack_start(self.detail_box_inner, False, False, 0)
 

sublime/ui/main.py

@@ -6,7 +6,7 @@ from gi.repository import Gdk, Gio, GLib, GObject, Gtk, Pango
 from sublime.adapters import AdapterManager, api_objects as API, Result
 from sublime.config import AppConfiguration
 from sublime.ui import albums, artists, browse, player_controls, playlists, util
-from sublime.ui.common import SpinnerImage
+from sublime.ui.common import IconButton, SpinnerImage
 
 
 class MainWindow(Gtk.ApplicationWindow):
@@ -24,6 +24,7 @@ class MainWindow(Gtk.ApplicationWindow):
             GObject.TYPE_NONE,
             (object, bool),
         ),
+        "notification-closed": (GObject.SignalFlags.RUN_FIRST, GObject.TYPE_NONE, (),),
         "go-to": (GObject.SignalFlags.RUN_FIRST, GObject.TYPE_NONE, (str, str),),
     }
 
@@ -43,6 +44,33 @@ class MainWindow(Gtk.ApplicationWindow):
         self.titlebar = self._create_headerbar(self.stack)
         self.set_titlebar(self.titlebar)
 
+        flowbox = Gtk.Box(orientation=Gtk.Orientation.VERTICAL)
+        notification_container = Gtk.Overlay()
+
+        notification_container.add(self.stack)
+
+        self.notification_revealer = Gtk.Revealer(
+            valign=Gtk.Align.END, halign=Gtk.Align.CENTER
+        )
+
+        notification_box = Gtk.Box(can_focus=False, valign="start", spacing=10)
+        notification_box.get_style_context().add_class("app-notification")
+
+        self.notification_text = Gtk.Label(use_markup=True)
+        notification_box.pack_start(self.notification_text, True, False, 0)
+
+        self.notification_actions = Gtk.Box()
+        notification_box.pack_start(self.notification_actions, True, False, 0)
+
+        notification_box.add(close_button := IconButton("window-close-symbolic"))
+        close_button.connect("clicked", lambda _: self.emit("notification-closed"))
+
+        self.notification_revealer.add(notification_box)
+
+        notification_container.add_overlay(self.notification_revealer)
+        flowbox.pack_start(notification_container, True, True, 0)
+
+        # Player Controls
         self.player_controls = player_controls.PlayerControls()
         self.player_controls.connect(
             "song-clicked", lambda _, *a: self.emit("song-clicked", *a)
@@ -53,15 +81,32 @@ class MainWindow(Gtk.ApplicationWindow):
         self.player_controls.connect(
             "refresh-window", lambda _, *args: self.emit("refresh-window", *args),
         )
-
-        flowbox = Gtk.Box(orientation=Gtk.Orientation.VERTICAL)
-        flowbox.pack_start(self.stack, True, True, 0)
         flowbox.pack_start(self.player_controls, False, True, 0)
+
         self.add(flowbox)
 
         self.connect("button-release-event", self._on_button_release)
 
+    current_notification_hash = None
+
     def update(self, app_config: AppConfiguration, force: bool = False):
+        notification = app_config.state.current_notification
+        if notification and hash(notification) != self.current_notification_hash:
+            self.notification_text.set_markup(notification.markup)
+
+            for c in self.notification_actions.get_children():
+                self.notification_actions.remove(c)
+
+            for label, fn in notification.actions:
+                self.notification_actions.add(action_button := Gtk.Button(label=label))
+                action_button.connect("clicked", lambda _: fn())
+
+            self.notification_revealer.show_all()
+            self.notification_revealer.set_reveal_child(True)
+
+        if notification is None:
+            self.notification_revealer.set_reveal_child(False)
+
         # Update the Connected to label on the popup menu.
         if app_config.server:
             self.connected_to_label.set_markup(

sublime/ui/state.py

@@ -1,7 +1,7 @@
 from dataclasses import dataclass, field
 from datetime import timedelta
 from enum import Enum
-from typing import Any, Dict, Optional, Tuple
+from typing import Any, Callable, Dict, Optional, Tuple
 
 from sublime.adapters import AlbumSearchQuery
 from sublime.adapters.api_objects import Genre, Song
@@ -35,6 +35,13 @@ class RepeatType(Enum):
 class UIState:
     """Represents the UI state of the application."""
 
+    @dataclass(unsafe_hash=True)
+    class UINotification:
+        markup: str
+        actions: Tuple[Tuple[str, Callable[[], None]], ...] = field(
+            default_factory=tuple
+        )
+
     version: int = 1
     playing: bool = False
     current_song_index: int = -1
@@ -55,15 +62,18 @@ class UIState:
     album_sort_direction: str = "ascending"
     album_page_size: int = 30
     album_page: int = 0
+    current_notification: Optional[UINotification] = None
 
     def __getstate__(self):
         state = self.__dict__.copy()
         del state["song_stream_cache_progress"]
+        del state["current_notification"]
         return state
 
     def __setstate__(self, state: Dict[str, Any]):
         self.__dict__.update(state)
         self.song_stream_cache_progress = None
+        self.current_notification = None
 
     class _DefaultGenre(Genre):
         def __init__(self):
@@ -83,11 +93,11 @@ class UIState:
 
     @property
     def current_song(self) -> Optional[Song]:
-        from sublime.adapters import AdapterManager
-
         if not self.play_queue or self.current_song_index < 0:
             return None
 
+        from sublime.adapters import AdapterManager
+
         current_song_id = self.play_queue[self.current_song_index]
 
         if not self._current_song or self._current_song.id != current_song_id:

tests/adapter_tests/filesystem_adapter_tests.py

@@ -267,11 +267,13 @@ def test_caching_get_playlist_then_details(cache_adapter: FilesystemAdapter):
 
 def test_cache_cover_art(cache_adapter: FilesystemAdapter):
     with pytest.raises(CacheMissError):
-        cache_adapter.get_cover_art_uri("pl_test1", "file")
+        cache_adapter.get_cover_art_uri("pl_test1", "file", size=300)
 
     # After ingesting the data, reading from the cache should give the exact same file.
     cache_adapter.ingest_new_data(KEYS.COVER_ART_FILE, "pl_test1", MOCK_ALBUM_ART)
-    with open(cache_adapter.get_cover_art_uri("pl_test1", "file"), "wb+") as cached:
+    with open(
+        cache_adapter.get_cover_art_uri("pl_test1", "file", size=300), "wb+"
+    ) as cached:
         with open(MOCK_ALBUM_ART, "wb+") as expected:
             assert cached.read() == expected.read()
 
@@ -294,8 +296,8 @@ def test_invalidate_playlist(cache_adapter: FilesystemAdapter):
         KEYS.COVER_ART_FILE, "pl_2", MOCK_ALBUM_ART2,
     )
 
-    stale_uri_1 = cache_adapter.get_cover_art_uri("pl_test1", "file")
+    stale_uri_1 = cache_adapter.get_cover_art_uri("pl_test1", "file", size=300)
-    stale_uri_2 = cache_adapter.get_cover_art_uri("pl_2", "file")
+    stale_uri_2 = cache_adapter.get_cover_art_uri("pl_2", "file", size=300)
 
     cache_adapter.invalidate_data(KEYS.PLAYLISTS, None)
     cache_adapter.invalidate_data(KEYS.PLAYLIST_DETAILS, "2")
@@ -311,7 +313,7 @@ def test_invalidate_playlist(cache_adapter: FilesystemAdapter):
         assert len(e.partial_data) == 2
 
     try:
-        cache_adapter.get_cover_art_uri("pl_test1", "file")
+        cache_adapter.get_cover_art_uri("pl_test1", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data
@@ -326,7 +328,7 @@ def test_invalidate_playlist(cache_adapter: FilesystemAdapter):
     # Even though the pl_2 cover art file wasn't explicitly invalidated, it should have
     # been invalidated with the playlist details invalidation.
     try:
-        cache_adapter.get_cover_art_uri("pl_2", "file")
+        cache_adapter.get_cover_art_uri("pl_2", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data
@@ -349,7 +351,7 @@ def test_invalidate_song_file(cache_adapter: FilesystemAdapter):
         cache_adapter.get_song_uri("1", "file")
 
     with pytest.raises(CacheMissError):
-        cache_adapter.get_cover_art_uri("s1", "file")
+        cache_adapter.get_cover_art_uri("s1", "file", size=300)
 
     # Make sure it didn't delete the other song.
     assert cache_adapter.get_song_uri("2", "file").endswith("song2.mp3")
@@ -430,7 +432,7 @@ def test_delete_playlists(cache_adapter: FilesystemAdapter):
     # Deleting a playlist with associated cover art should get rid the cover art too.
     cache_adapter.delete_data(KEYS.PLAYLIST_DETAILS, "1")
     try:
-        cache_adapter.get_cover_art_uri("pl_1", "file")
+        cache_adapter.get_cover_art_uri("pl_1", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data is None
@@ -440,7 +442,7 @@ def test_delete_playlists(cache_adapter: FilesystemAdapter):
         MOCK_ALBUM_ART, str(cache_adapter.cover_art_dir.joinpath(MOCK_ALBUM_ART_HASH)),
     )
     try:
-        cache_adapter.get_cover_art_uri("pl_1", "file")
+        cache_adapter.get_cover_art_uri("pl_1", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data is None
@@ -454,7 +456,7 @@ def test_delete_song_data(cache_adapter: FilesystemAdapter):
     )
 
     music_file_path = cache_adapter.get_song_uri("1", "file")
-    cover_art_path = cache_adapter.get_cover_art_uri("s1", "file")
+    cover_art_path = cache_adapter.get_cover_art_uri("s1", "file", size=300)
 
     cache_adapter.delete_data(KEYS.SONG_FILE, "1")
     cache_adapter.delete_data(KEYS.COVER_ART_FILE, "s1")
@@ -469,7 +471,7 @@ def test_delete_song_data(cache_adapter: FilesystemAdapter):
         assert e.partial_data is None
 
     try:
-        cache_adapter.get_cover_art_uri("s1", "file")
+        cache_adapter.get_cover_art_uri("s1", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data is None
@@ -798,9 +800,9 @@ def test_caching_invalidate_artist(cache_adapter: FilesystemAdapter):
     stale_artist = cache_adapter.get_artist("artist1")
     stale_album_1 = cache_adapter.get_album("1")
     stale_album_2 = cache_adapter.get_album("2")
-    stale_artist_artwork = cache_adapter.get_cover_art_uri("image", "file")
+    stale_artist_artwork = cache_adapter.get_cover_art_uri("image", "file", size=300)
-    stale_cover_art_1 = cache_adapter.get_cover_art_uri("1", "file")
+    stale_cover_art_1 = cache_adapter.get_cover_art_uri("1", "file", size=300)
-    stale_cover_art_2 = cache_adapter.get_cover_art_uri("2", "file")
+    stale_cover_art_2 = cache_adapter.get_cover_art_uri("2", "file", size=300)
 
     cache_adapter.invalidate_data(KEYS.ARTIST, "artist1")
 
@@ -827,21 +829,21 @@ def test_caching_invalidate_artist(cache_adapter: FilesystemAdapter):
         assert e.partial_data == stale_album_2
 
     try:
-        cache_adapter.get_cover_art_uri("image", "file")
+        cache_adapter.get_cover_art_uri("image", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data
         assert e.partial_data == stale_artist_artwork
 
     try:
-        cache_adapter.get_cover_art_uri("1", "file")
+        cache_adapter.get_cover_art_uri("1", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data
         assert e.partial_data == stale_cover_art_1
 
     try:
-        cache_adapter.get_cover_art_uri("2", "file")
+        cache_adapter.get_cover_art_uri("2", "file", size=300)
         assert 0, "DID NOT raise CacheMissError"
     except CacheMissError as e:
         assert e.partial_data