From 00516581dcf325029702b4485ad14a1d6bf8aa98 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Sat, 16 May 2020 23:05:54 -0600 Subject: [PATCH] Closes #192 Play queue resume is non-modal Also added infrastructure for showing other non-modal notifications --- CHANGELOG.rst | 3 ++ sublime/adapters/subsonic/api_objects.py | 13 ++++- sublime/app.py | 26 ++++----- sublime/config.py | 1 - sublime/ui/albums.py | 1 + sublime/ui/main.py | 53 +++++++++++++++++-- sublime/ui/state.py | 16 ++++-- .../adapter_tests/filesystem_adapter_tests.py | 36 +++++++------ 8 files changed, 111 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 29cf55d..04ea01e 100644 --- a/CHANGELOG.rst +++ b/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. diff --git a/sublime/adapters/subsonic/api_objects.py b/sublime/adapters/subsonic/api_objects.py index d7f4d10..78052d0 100644 --- a/sublime/adapters/subsonic/api_objects.py +++ b/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 diff --git a/sublime/app.py b/sublime/app.py index 7f8358a..4be16f0 100644 --- a/sublime/app.py +++ b/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) - result = dialog.run() - dialog.destroy() - if result == Gtk.ResponseType.YES: + def on_resume_click(): 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"{resume_text}", + 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)) diff --git a/sublime/config.py b/sublime/config.py index 6f30fe6..cbdb0d1 100644 --- a/sublime/config.py +++ b/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 diff --git a/sublime/ui/albums.py b/sublime/ui/albums.py index 2e13344..36d9327 100644 --- a/sublime/ui/albums.py +++ b/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) diff --git a/sublime/ui/main.py b/sublime/ui/main.py index a0ee084..26f944e 100644 --- a/sublime/ui/main.py +++ b/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( diff --git a/sublime/ui/state.py b/sublime/ui/state.py index 98c111f..f103951 100644 --- a/sublime/ui/state.py +++ b/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: diff --git a/tests/adapter_tests/filesystem_adapter_tests.py b/tests/adapter_tests/filesystem_adapter_tests.py index cbb73b0..e7ba830 100644 --- a/tests/adapter_tests/filesystem_adapter_tests.py +++ b/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_2 = cache_adapter.get_cover_art_uri("pl_2", "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", 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_cover_art_1 = cache_adapter.get_cover_art_uri("1", "file") - stale_cover_art_2 = cache_adapter.get_cover_art_uri("2", "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", size=300) + 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