diff --git a/backend/config/config.go b/backend/config/config.go index 2f6430e..3e5b7b0 100644 --- a/backend/config/config.go +++ b/backend/config/config.go @@ -544,7 +544,7 @@ func (c *Config) SetDefaultPage(page string) error { c.General.ApplyDefaults() } - c.General.DefaultPage = DefaultPage(page) + c.General.DefaultPage = View(page) if err := c.General.Validate(); err != nil { return fmt.Errorf( @@ -666,6 +666,81 @@ func (c *Config) SetAllowMeteredCatalogDownload(allow bool) error { return nil } +// GetViewVisibility reports which primary views the sidebar should +// show, answered for every known view rather than only the ones the +// config mentions -- so the frontend filters on a value and never has +// to hold a second copy of the defaults. +func (c *Config) GetViewVisibility() map[string]bool { + if c.General == nil { + general := &GeneralConfig{} + general.ApplyDefaults() + + return general.ResolvedViewVisibility() + } + + return c.General.ResolvedViewVisibility() +} + +// SetViewVisible shows or hides one primary view. +// +// Two refusals, both about a state the user cannot get out of from the +// UI they would be left with: Settings is never hideable, and the +// launch page is never hideable while it is the launch page (change it +// first). Hiding a view does not make it unreachable -- `navigate` +// still resolves it, which detail views depend on -- it only takes the +// nav item away. +func (c *Config) SetViewVisible(view string, visible bool) error { + spec, known := LookupView(view) + if !known { + return fmt.Errorf("%w: %q", errUnknownView, view) + } + + if c.General == nil { + c.General = &GeneralConfig{} + c.General.ApplyDefaults() + } + + if !visible { + if !spec.Hideable { + return fmt.Errorf("%w: %q", errViewNotHideable, view) + } + + if spec.ID == c.General.DefaultPage { + return fmt.Errorf("%w: %q", errViewIsLaunchPage, view) + } + } + + if c.General.ViewVisibility == nil { + c.General.ViewVisibility = make(map[string]bool, len(Views)) + } + + c.General.ViewVisibility[view] = visible + + if err := c.General.Validate(); err != nil { + return fmt.Errorf("invalid view visibility: %w", err) + } + + if err := c.Save(); err != nil { + return fmt.Errorf("could not save config: %w", err) + } + + events.Emit( + c.ctx, + events.GeneralConfigChanged, + map[string]any{ + "ViewVisibility": c.General.ResolvedViewVisibility(), + }, + ) + + c.logger.Info( + "view visibility updated", + "view", view, + "visible", visible, + ) + + return nil +} + // GetTrackListColumns returns the configured track-list columns. func (c *Config) GetTrackListColumns() []tracklist.Column { if c.TrackList == nil { diff --git a/backend/config/general.go b/backend/config/general.go index 895b391..750ec65 100644 --- a/backend/config/general.go +++ b/backend/config/general.go @@ -5,27 +5,13 @@ import ( "fmt" ) -// DefaultPage identifies which view the app opens to on launch. -type DefaultPage string - -// Valid DefaultPage values, matching the frontend's top-level route ids. -const ( - DefaultPageHome DefaultPage = "home" - DefaultPageTracks DefaultPage = "tracks" - DefaultPageAlbums DefaultPage = "albums" - DefaultPageArtists DefaultPage = "artists" - DefaultPageGenres DefaultPage = "genres" - DefaultPagePlaylists DefaultPage = "playlists" - DefaultPageExplore DefaultPage = "explore" - DefaultPageDownloads DefaultPage = "downloads" - DefaultPageAutotag DefaultPage = "autotag" - DefaultPageJobs DefaultPage = "jobs" -) - // DefaultDefaultPage is the launch page for a fresh install. -const DefaultDefaultPage = DefaultPageHome +const DefaultDefaultPage = ViewHome -var errUnknownDefaultPage = errors.New("unknown default page") +var ( + errUnknownDefaultPage = errors.New("unknown default page") + errViewCannotLaunch = errors.New("view cannot be the launch page") +) // QueueFallback identifies what plays, if anything, once the queue // runs out with nothing left to auto-advance to. @@ -46,8 +32,22 @@ var errUnknownQueueFallback = errors.New("unknown queue fallback") // GeneralConfig holds general application preferences that don't // belong to a more specific subsystem. type GeneralConfig struct { - DefaultPage DefaultPage `toml:"DefaultPage"` + DefaultPage View `toml:"DefaultPage"` QueueFallback QueueFallback `toml:"QueueFallback"` + // ViewVisibility says which sidebar destinations are shown, keyed by + // view id. + // + // **An absent key means that view's own default** (`Views`), and that + // is the whole reason this is a map rather than a `HiddenViews + // []string` or a struct of booleans. A list's zero value is "hide + // nothing", which cannot express Autotag being off by default without + // a migration; a struct field for a view that later stops existing is + // stored garbage somebody has to deprecate. Here a view added later + // gets its own default rather than being invisible or forcibly + // visible, an unknown key is dropped on load, and no install needs + // migrating in either direction. Same polarity rule as + // AllowMeteredCatalogDownload: the zero value is the intended answer. + ViewVisibility map[string]bool `toml:"ViewVisibility"` // AllowMeteredCatalogDownload permits the ~0.6 GB Explore catalog to // be fetched on a connection the platform calls cellular. It defaults // to false, which is the whole point: the zero value is the safe one, @@ -71,15 +71,17 @@ func (c *GeneralConfig) ApplyDefaults() { func (c *GeneralConfig) Validate() error { c.ApplyDefaults() - switch c.DefaultPage { - case DefaultPageHome, DefaultPageTracks, DefaultPageAlbums, DefaultPageArtists, - DefaultPageGenres, DefaultPagePlaylists, DefaultPageExplore, DefaultPageDownloads, - DefaultPageAutotag, DefaultPageJobs: - // Valid. - default: + spec, known := LookupView(string(c.DefaultPage)) + if !known { return fmt.Errorf("%w: %q", errUnknownDefaultPage, c.DefaultPage) } + if !spec.CanLaunch { + return fmt.Errorf("%w: %q", errViewCannotLaunch, c.DefaultPage) + } + + c.normalizeViewVisibility() + switch c.QueueFallback { case QueueFallbackStop, QueueFallbackFavorites, QueueFallbackDynamicMix: // Valid. @@ -89,3 +91,51 @@ func (c *GeneralConfig) Validate() error { return nil } + +// normalizeViewVisibility drops what the stored map may not say, and +// repairs the one invariant the shell depends on. +// +// Three things are dropped or forced, and all three are reachable only +// from a hand-edited config or from a version that knew different +// views: an unknown id (a view removed since, e.g. when #27 folds Jobs +// into Settings) says nothing to anybody; a view that is not Hideable +// cannot be false; and **the launch page is always visible**, because +// otherwise an install lands on a page with no nav item pointing at it. +// +// That last one is a *repair* here and an *error* at the setter +// (SetViewVisible), deliberately. On load there is nobody to tell and +// the honest reading of "my launch page is Autotag" is that this user +// wants Autotag, so it is un-hidden rather than the launch page being +// silently reset to something they did not choose. At the setter the +// user is right there and can act, so it refuses and says why. +func (c *GeneralConfig) normalizeViewVisibility() { + for id := range c.ViewVisibility { + spec, known := LookupView(id) + if !known || !spec.Hideable { + delete(c.ViewVisibility, id) + } + } + + if visible, ok := c.ViewVisibility[string(c.DefaultPage)]; ok && !visible { + c.ViewVisibility[string(c.DefaultPage)] = true + } +} + +// ResolvedViewVisibility answers for every known view, so no caller has +// to know the defaults -- the frontend included, which is why the +// binding returns this rather than the stored map. +func (c *GeneralConfig) ResolvedViewVisibility() map[string]bool { + resolved := make(map[string]bool, len(Views)) + + for _, v := range Views { + visible := v.VisibleByDefault + + if stored, ok := c.ViewVisibility[string(v.ID)]; ok && v.Hideable { + visible = stored + } + + resolved[string(v.ID)] = visible + } + + return resolved +} diff --git a/backend/config/views.go b/backend/config/views.go new file mode 100644 index 0000000..87eb47b --- /dev/null +++ b/backend/config/views.go @@ -0,0 +1,89 @@ +package config + +import "errors" + +var ( + errUnknownView = errors.New("unknown view") + errViewNotHideable = errors.New("view cannot be hidden") + errViewIsLaunchPage = errors.New("view is the launch page") +) + +// View identifies one of the shell's primary destinations -- the +// things the sidebar lists and `index.ts` knows as `VIEW_TAGS`. +type View string + +// The primary views, in no particular order: the sidebar owns the order +// it draws them in, because that is presentation. +const ( + ViewHome View = "home" + ViewPlaylists View = "playlists" + ViewArtists View = "artists" + ViewGenres View = "genres" + ViewAlbums View = "albums" + ViewTracks View = "tracks" + ViewExplore View = "explore" + ViewDownloads View = "downloads" + ViewAutotag View = "autotag" + ViewJobs View = "jobs" + ViewSettings View = "settings" +) + +// ViewSpec is what the backend knows about a destination. The label and +// the icon are deliberately absent: those are presentation, they live +// beside the rest of the app's icon vocabulary in +// `frontend/src/utils/icon-language.ts`, and a Go copy of them would be +// a second thing to keep in step for nothing. +type ViewSpec struct { + // ID is the view name the frontend navigates by. + ID View + // VisibleByDefault is what an install gets when the config says + // nothing about this view -- which is every install until somebody + // changes it, and every view added after this one shipped. + VisibleByDefault bool + // Hideable is false for Settings alone. It is a property of the + // view rather than a check in the setter because `config.toml` is + // hand-editable, and an app that can be locked out of its own + // Settings by a typo is a support problem nobody can debug + // remotely. + Hideable bool + // CanLaunch reports whether the view may be the launch page. + // Settings is the only one that may not, which is the shape the + // DefaultPage enum already had. + CanLaunch bool +} + +// Views is the one list of primary destinations, in the order Settings +// offers them. +// +// It is the single source for three things that used to be written down +// separately: which views exist, which of them may be the launch page +// (`DefaultPage`'s validation reads it), and what an unconfigured +// install shows. +// +// Autotag is the one view hidden by default: it rewrites tags on disk, +// which is not what most libraries want on day one, and #25 asks for it +// to be turned on deliberately. +var Views = []ViewSpec{ + {ID: ViewHome, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewPlaylists, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewArtists, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewGenres, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewAlbums, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewTracks, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewExplore, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewDownloads, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewAutotag, VisibleByDefault: false, Hideable: true, CanLaunch: true}, + {ID: ViewJobs, VisibleByDefault: true, Hideable: true, CanLaunch: true}, + {ID: ViewSettings, VisibleByDefault: true, Hideable: false, CanLaunch: false}, +} + +// LookupView returns the spec for a view id. +func LookupView(id string) (ViewSpec, bool) { + for _, v := range Views { + if string(v.ID) == id { + return v, true + } + } + + return ViewSpec{}, false +} diff --git a/backend/config/views_test.go b/backend/config/views_test.go new file mode 100644 index 0000000..f14b14f --- /dev/null +++ b/backend/config/views_test.go @@ -0,0 +1,255 @@ +package config + +import ( + "errors" + "log/slog" + "path/filepath" + "testing" +) + +// newViewTestConfig builds a Config backed by a temp file, which is all +// SetViewVisible needs: it saves and emits, and the emit is a no-op +// without a running app. +func newViewTestConfig(t *testing.T) *Config { + t.Helper() + + c := &Config{ + logger: slog.Default(), + filePath: filepath.Join(t.TempDir(), "config.toml"), + } + + // Load a file that is not there: that is what marks the config + // loaded, without which Save refuses on the *second* write. + if err := c.Load(); err != nil { + t.Fatalf("Load() error: %v", err) + } + + return c +} + +// A view the config says nothing about takes its own default, which is +// what makes this need no migration in either direction: an existing +// install gets Autotag hidden without a key, and a view added later +// gets its own answer rather than the list's. +func TestViewVisibilityDefaults(t *testing.T) { + t.Parallel() + + general := &GeneralConfig{} + general.ApplyDefaults() + + resolved := general.ResolvedViewVisibility() + + if len(resolved) != len(Views) { + t.Fatalf("resolved %d views, want %d", len(resolved), len(Views)) + } + + if resolved[string(ViewAutotag)] { + t.Error("autotag should be hidden by default") + } + + for _, v := range Views { + if v.ID == ViewAutotag { + continue + } + + if !resolved[string(v.ID)] { + t.Errorf("%s should be visible by default", v.ID) + } + } +} + +// A stored answer wins over the default, in both directions -- turning +// Autotag on is the whole user-facing point. +func TestViewVisibilityStoredWins(t *testing.T) { + t.Parallel() + + general := &GeneralConfig{ + ViewVisibility: map[string]bool{ + string(ViewAutotag): true, + string(ViewJobs): false, + }, + } + general.ApplyDefaults() + + resolved := general.ResolvedViewVisibility() + + if !resolved[string(ViewAutotag)] { + t.Error("autotag was switched on and should be visible") + } + + if resolved[string(ViewJobs)] { + t.Error("jobs was switched off and should be hidden") + } +} + +// A key for a view that no longer exists is discarded rather than +// migrated. This is the property the #25-before-#27 ordering rests on: +// when Jobs folds into Settings, `jobs = true` in somebody's config is +// a key nothing asks about, not a cleanup task. +func TestValidateDropsUnknownAndUnhideableViews(t *testing.T) { + t.Parallel() + + general := &GeneralConfig{ + ViewVisibility: map[string]bool{ + "a-view-that-was-removed": true, + string(ViewSettings): false, + string(ViewAutotag): true, + }, + } + + if err := general.Validate(); err != nil { + t.Fatalf("Validate() error: %v", err) + } + + if _, ok := general.ViewVisibility["a-view-that-was-removed"]; ok { + t.Error("an unknown view id should be dropped on load") + } + + if _, ok := general.ViewVisibility[string(ViewSettings)]; ok { + t.Error("settings is not hideable and should not be stored") + } + + if !general.ResolvedViewVisibility()[string(ViewSettings)] { + t.Error("settings must resolve visible whatever the file said") + } +} + +// On load there is nobody to tell, so a launch page hidden by a +// hand-edited file is un-hidden rather than the launch page being +// reset to something the user did not choose. +func TestValidateRevealsAHiddenLaunchPage(t *testing.T) { + t.Parallel() + + general := &GeneralConfig{ + DefaultPage: ViewAutotag, + ViewVisibility: map[string]bool{ + string(ViewAutotag): false, + }, + } + + if err := general.Validate(); err != nil { + t.Fatalf("Validate() error: %v", err) + } + + if !general.ResolvedViewVisibility()[string(ViewAutotag)] { + t.Error("the launch page must be visible") + } +} + +// Settings may not be the launch page, which is the shape the old +// DefaultPage enum had and is now read off the same table. +func TestValidateRejectsAnUnlaunchablePage(t *testing.T) { + t.Parallel() + + general := &GeneralConfig{DefaultPage: ViewSettings} + + err := general.Validate() + if !errors.Is(err, errViewCannotLaunch) { + t.Fatalf("Validate() error = %v, want errViewCannotLaunch", err) + } +} + +// At the setter the user is present and can act, so the two states +// they could not get out of are refused rather than repaired. +func TestSetViewVisibleRefusals(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + view string + visible bool + want error + }{ + {"settings is never hideable", string(ViewSettings), false, errViewNotHideable}, + {"the launch page is not hideable", string(ViewHome), false, errViewIsLaunchPage}, + {"an unknown view is not a setting", "nonsense", false, errUnknownView}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + c := newViewTestConfig(t) + + err := c.SetViewVisible(tt.view, tt.visible) + if !errors.Is(err, tt.want) { + t.Fatalf("SetViewVisible() error = %v, want %v", err, tt.want) + } + }) + } +} + +// Showing a view is never refused, including Settings and the launch +// page -- there is no state to be stuck in. +func TestSetViewVisibleShowsAnything(t *testing.T) { + t.Parallel() + + c := newViewTestConfig(t) + + for _, v := range Views { + if err := c.SetViewVisible(string(v.ID), true); err != nil { + t.Fatalf("SetViewVisible(%q, true) error: %v", v.ID, err) + } + } + + if !c.GetViewVisibility()[string(ViewAutotag)] { + t.Error("autotag was switched on and should be visible") + } +} + +// The stored map survives a save/load round trip, which is what a +// map-valued TOML key is worth checking for. +func TestViewVisibilityRoundTrips(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "config.toml") + + original := &Config{logger: slog.Default(), filePath: path} + if err := original.Load(); err != nil { + t.Fatalf("Load() error: %v", err) + } + + if err := original.SetViewVisible(string(ViewAutotag), true); err != nil { + t.Fatalf("SetViewVisible() error: %v", err) + } + + if err := original.SetViewVisible(string(ViewJobs), false); err != nil { + t.Fatalf("SetViewVisible() error: %v", err) + } + + loaded := &Config{logger: slog.Default(), filePath: path} + if err := loaded.Load(); err != nil { + t.Fatalf("Load() error: %v", err) + } + + resolved := loaded.GetViewVisibility() + + if !resolved[string(ViewAutotag)] { + t.Error("autotag should have loaded as visible") + } + + if resolved[string(ViewJobs)] { + t.Error("jobs should have loaded as hidden") + } +} + +// Every view the shell can launch into is a view the sidebar can show, +// or an install could land on a page with no nav item and no setting +// pointing at it. +func TestEveryLaunchableViewIsAView(t *testing.T) { + t.Parallel() + + for _, v := range Views { + if !v.CanLaunch { + continue + } + + if !v.Hideable { + continue + } + + if _, ok := LookupView(string(v.ID)); !ok { + t.Errorf("%s is launchable but not a known view", v.ID) + } + } +} diff --git a/frontend/bindings/yellowjacket/backend/config/config.ts b/frontend/bindings/yellowjacket/backend/config/config.ts index ddd06ac..169ae09 100644 --- a/frontend/bindings/yellowjacket/backend/config/config.ts +++ b/frontend/bindings/yellowjacket/backend/config/config.ts @@ -112,6 +112,16 @@ export function GetTrackListColumns(): $CancellablePromise { + return $Call.ByID(2798108026); +} + /** * Load reads and parses the config file from disk. */ @@ -247,6 +257,20 @@ export function SetTrackListColumns(columns: tracklist$0.Column[] | null): $Canc return $Call.ByID(4226159685, columns); } +/** + * SetViewVisible shows or hides one primary view. + * + * Two refusals, both about a state the user cannot get out of from the + * UI they would be left with: Settings is never hideable, and the + * launch page is never hideable while it is the launch page (change it + * first). Hiding a view does not make it unreachable -- `navigate` + * still resolves it, which detail views depend on -- it only takes the + * nav item away. + */ +export function SetViewVisible(view: string, visible: boolean): $CancellablePromise { + return $Call.ByID(1751982648, view, visible); +} + /** * Validate returns errors if there is a breaking issue with the config. */