Add launch page settings #634

adrianghc · 2025-06-27T19:37:32Z

Add a setting and surrounding infrastructure to let users pick which page should appear when they launch the app (except when launching via file). Any page besides the settings page is supported, including second-level pages like the albums or artists pages of the music library.

Add a LaunchPageOption setting and surrounding enums, getters and setters.

United600 · 2025-07-16T15:43:37Z

Nice change. I am not so sure about the subpages, we might want to consider adding only the navigation view items. That approach would allow us to reuse them for a eventual jumplist support.

Also you were only required to add the English string, as Crowdin manages the rest. Could you please revert those changes?

adrianghc · 2025-07-16T21:02:18Z

I did consider adding only the navigation view items, but I found that I didn't like this approach because it effectively forces on the user an arbitrary opinion of the app about which subpage is more important. For me personally, the default songs view of the music page is not useful, and I always switch to the albums view. This is an arbitrary preference on my side, but so it would be for the app to only let me launch at the songs view. It would leave the idea of this feature, to let the user control where to launch the app, only half-baked. If you don't share the app's opinion on which subpage is the most important, you're left with an extra navigation step to do each time.

Finally, on the default Media Player app of Windows, the behavior is that while there is no setting like I'm implementing, the app does remember the last page you had open, down to the subpage level. I didn't want Screenbox to be less-than in this regard. :)

Regarding localization, should I just revert the changes to all Resources.resw files except for en-US? I'm a native German speaker so I already added de-DE translations, would it make sense to keep those rather than going through Crowdin for them?

United600 · 2025-07-17T06:21:50Z

I did consider adding only the navigation view items, but I found that I didn't like this approach because it effectively forces on the user an arbitrary opinion of the app about which subpage is more important.

It’s cool, I was just throwing my opinion out there.

Finally, on the default Media Player app of Windows, the behavior is that while there is no setting like I'm implementing, the app does remember the last page you had open, down to the subpage level. I didn't want Screenbox to be less-than in this regard. :)

We should do the same at some point, same for the jumplist support.

Regarding localization, should I just revert the changes to all Resources.resw files except for en-US? I'm a native German speaker so I already added de-DE translations, would it make sense to keep those rather than going through Crowdin for them?

Yes, the de-DE translation should be reverted as well, and yeah it's a bit of a pain. Crowdin will overwrite any Resources.resw except for the en-US. Sorry about that.

Add an item to the settings GUI page to view and modify the LaunchPageOption setting, plus locale resources (en-US and de-DE).

Add a EnumExtensions.cs file to Screenbox\Helpers, which includes two helper methods for the LaunchPageOption enum type: - GetNavPageFirstLevel() returns the name of the first navigation level (left NavigationView) for each LaunchPageOption setting. - GetNavPageSecondLevel() returns the name of the second navigation level (top NavigationView) for each LaunchPageOption setting.

Navigate to the first (left) NavigationView level on app launch, as indicated by the LaunchPageOption setting.

Navigate to the second (top) NavigationView level on app launch, as indicated by the LaunchPageOption setting, and applicable for the music and video library pages.

adrianghc · 2025-07-17T18:16:16Z

I have reverted all the translations. I suppose I can only add the de-DE translation back through Crowdin after this PR has been merged?

United600 · 2025-07-17T21:18:33Z

I have reverted all the translations. I suppose I can only add the de-DE translation back through Crowdin after this PR has been merged?

Thanks. Yes, and if you don't have an account, or don't want to create one, we can translate it.

Copilot

Pull Request Overview

This PR adds functionality to allow users to select which page should appear when they launch the app. The implementation includes a new settings UI, underlying enum and service changes, and navigation logic updates to respect the user's chosen launch page preference.

Introduces a new LaunchPageOption enum with 8 different page options
Adds settings UI with a ComboBox to select the preferred launch page
Updates navigation logic in main pages to use the selected launch page setting

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
Screenbox/Strings/en-US/Resources.resw	Adds localized strings for launch page settings UI
Screenbox/Screenbox.csproj	Registers new EnumExtensions helper class
Screenbox/Pages/SettingsPage.xaml	Adds ComboBox UI for launch page selection
Screenbox/Helpers/EnumExtensions.cs	Provides navigation tag mapping for launch page options
Screenbox/Pages/MainPage.xaml.cs	Updates main navigation to respect launch page setting
Screenbox/Pages/MusicPage.xaml.cs	Updates music page navigation for launch page setting
Screenbox/Pages/VideosPage.xaml.cs	Updates videos page navigation for launch page setting
Screenbox.Core/ViewModels/SettingsPageViewModel.cs	Adds view model property and change handling for launch page
Screenbox.Core/Services/SettingsService.cs	Implements persistent storage for launch page setting
Screenbox.Core/Services/ISettingsService.cs	Adds interface definition for launch page property
Screenbox.Core/Enums/SettingsOptions.cs	Defines LaunchPageOption enum with available page options

Copilot · 2025-08-08T04:58:09Z

Screenbox/Helpers/EnumExtensions.cs

+        };
+    }
+
+    public static string GetNavPageSecondLevel(this LaunchPageOption launchPageOption)


The GetNavPageSecondLevel method doesn't handle all LaunchPageOption enum values. Home, Network, and PlayQueue options will throw ArgumentOutOfRangeException when called, but these methods are used in navigation logic where any enum value could be passed.

Copilot · 2025-08-08T04:58:09Z

Screenbox/Pages/MusicPage.xaml.cs

+                    libraryNavItem = LibraryNavView.MenuItems
+                        .OfType<muxc.NavigationViewItem>()
+                        .FirstOrDefault(item => item.Tag?.ToString() == launchPageTag);
+                }


Calling GetNavPageSecondLevel() without checking if the launch page actually has a second level will cause an exception for Home, Network, and PlayQueue options.

Suggested change

}

// Only call GetNavPageSecondLevel if the first level is "music"

if (launchPage.GetNavPageFirstLevel() == "music")

{

var launchPageTag = launchPage.GetNavPageSecondLevel();

libraryNavItem = LibraryNavView.MenuItems

.OfType<muxc.NavigationViewItem>()

.FirstOrDefault(item => item.Tag?.ToString() == launchPageTag);

}

else

{

libraryNavItem = (muxc.NavigationViewItem?)LibraryNavView.MenuItems[0];

}

Calling GetNavPageSecondLevel() without checking if the launch page actually has a second level will cause an exception for Home, Network, and PlayQueue options.

The Copilot comment is valid. The methods GetNavPageFirstLevel and GetNavPageSecondLevel have an implicit dependency, making the logic fragile. Despite the name, we want to avoid exceptions when calling methods out of order.

Copilot · 2025-08-08T04:58:10Z

Screenbox/Pages/VideosPage.xaml.cs

+                    var launchPageTag = launchPage.GetNavPageSecondLevel();
+                    libraryNavItem = LibraryNavView.MenuItems
+                        .OfType<muxc.NavigationViewItem>()
+                        .FirstOrDefault(item => item.Tag?.ToString() == launchPageTag);


Calling GetNavPageSecondLevel() without checking if the launch page actually has a second level will cause an exception for Home, Network, and PlayQueue options.

Suggested change

.FirstOrDefault(item => item.Tag?.ToString() == launchPageTag);

// Only call GetNavPageSecondLevel if the launch page actually has a second level

string firstLevel = launchPage.GetNavPageFirstLevel();

if (firstLevel != "home" && firstLevel != "network" && firstLevel != "playqueue")

{

var launchPageTag = launchPage.GetNavPageSecondLevel();

libraryNavItem = LibraryNavView.MenuItems

.OfType<muxc.NavigationViewItem>()

.FirstOrDefault(item => item.Tag?.ToString() == launchPageTag);

}

else

{

libraryNavItem = (muxc.NavigationViewItem?)LibraryNavView.MenuItems[0];

}

adrianghc · 2025-08-08T08:37:01Z

@huynhsontung Would you like me to implement Copilot's suggested changes? Other usages of ArgumentOutOfRangeException in the code base do not set an error message so I didn't implement this either, but I can do it if you'd like.

United600 · 2025-08-08T14:55:28Z

@huynhsontung Would you like me to implement Copilot's suggested changes? Other usages of ArgumentOutOfRangeException in the code base do not set an error message so I didn't implement this either, but I can do it if you'd like.

Can't speak for him but (IMHO) the ArgumentOutOfRangeException changes aren't strictly necessary, don't go out of your way to implement them.

huynhsontung · 2025-08-09T17:30:24Z

Please ignore the Copilot comments and suggestions. I was just testing the feature.

I noticed that you also include subpage in the settings. I want to implement subpage persistence so when a nav page is selected it goes to the last subpage, as you noted with the Media Player. In the meantime, this setting will do the job.

Sorry for the delay in reviewing. I have been extremely busy.

adrianghc · 2025-09-01T21:26:58Z

Hi @huynhsontung, any chance we could revisit this? I think the check failed only because it used the deprecated Windows Server image, which I saw you fixed in the meantime.

huynhsontung

Sorry for the very late review! 😓

After reviewing the code, this solution appears somewhat messy for the problem it aims to address. Especially having the settings page as an enum, once released, it will be tough to change in the future. It might be more effective to implement a way to persist the Frame history across sessions, so that the user sees the last opened page when the app is launched. We probably can persist the navigation state using Frame.GetNavigationState and Frame.SetNavigationState.

huynhsontung · 2025-09-22T07:17:35Z

Screenbox/Pages/MainPage.xaml.cs


        private readonly Dictionary<string, Type> _pages;

+        private ISettingsService Settings => Ioc.Default.GetRequiredService<ISettingsService>();


Getting the service directly from the control breaks the MVVM pattern. Ideally, we want only the ViewModels to have access to the services. Then the control will get the value from the ViewModel.

huynhsontung · 2025-09-22T07:25:11Z

Screenbox/Pages/MusicPage.xaml.cs

+                    libraryNavItem = LibraryNavView.MenuItems
+                        .OfType<muxc.NavigationViewItem>()
+                        .FirstOrDefault(item => item.Tag?.ToString() == launchPageTag);
+                }


Calling GetNavPageSecondLevel() without checking if the launch page actually has a second level will cause an exception for Home, Network, and PlayQueue options.

The Copilot comment is valid. The methods GetNavPageFirstLevel and GetNavPageSecondLevel have an implicit dependency, making the logic fragile. Despite the name, we want to avoid exceptions when calling methods out of order.

huynhsontung · 2025-09-22T07:31:30Z

Screenbox/Pages/MusicPage.xaml.cs

+                {
+                    var launchPageTag = launchPage.GetNavPageSecondLevel();
+                    libraryNavItem = LibraryNavView.MenuItems
+                        .OfType<muxc.NavigationViewItem>()
+                        .FirstOrDefault(item => item.Tag?.ToString() == launchPageTag);
+                }


With this flow, the second-level page is set each time the user navigates to the parent page. Is this expected? The settings text suggests that this behaviour only applies to app launches. However, in reality, it is set on every page navigation.

From my testing, it actually only applies to app launches, not every page navigation.

huynhsontung · 2025-09-22T07:34:35Z

Screenbox.Core/Enums/SettingsOptions.cs

+public enum LaunchPageOption
+{
+    Home,
+    Songs,
+    Albums,
+    Artists,
+    VideoFolders,
+    AllVideos,
+    Network,
+    PlayQueue
+}


Having the launch page name as an enum is also problematic in the future. If we add new page types to the app, we have to add the page at the bottom of the enum. The order of this enum is fixed once it is released. We cannot change this order (only append to it) because doing so will break the user's settings.

This is true, but we could also hardcode the enum values, then we could insert new ones in the middle. Though that would not be all that elegant either.

adrianghc · 2025-09-29T20:38:23Z

I understand your concerns. So, if I understand you right, you'd rather scrap this implementation and go with an entirely different approach, correct? In that case I would close this. I might be able to get around to the approach you propose if I find the time and then I can create a separate PR for that.

Add LaunchPageOption setting and infrastructure

3a31faa

Add a LaunchPageOption setting and surrounding enums, getters and setters.

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. enhancement New feature or request l10n Localization labels Jun 27, 2025

huynhsontung mentioned this pull request Jul 11, 2025

some improvements w/ love⭐ #639

Closed

United600 requested a review from Copilot July 16, 2025 15:37

This comment was marked as outdated.

Sign in to view

adrianghc added 4 commits July 17, 2025 19:28

Add LaunchPageOption setting to GUI

15155db

Add an item to the settings GUI page to view and modify the LaunchPageOption setting, plus locale resources (en-US and de-DE).

Navigate to first (left) NavigationView level

95ab5a7

Navigate to the first (left) NavigationView level on app launch, as indicated by the LaunchPageOption setting.

Navigate to second (top) NavigationView level

a09a2d8

Navigate to the second (top) NavigationView level on app launch, as indicated by the LaunchPageOption setting, and applicable for the music and video library pages.

adrianghc force-pushed the launch-page-settings branch from b4c0e72 to a09a2d8 Compare July 17, 2025 17:41

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 17, 2025

huynhsontung requested a review from Copilot August 8, 2025 04:57

Copilot AI reviewed Aug 8, 2025

View reviewed changes

huynhsontung requested a review from Copilot August 8, 2025 05:09

This comment was marked as duplicate.

Sign in to view

huynhsontung reviewed Sep 22, 2025

View reviewed changes


		private readonly Dictionary<string, Type> _pages;

		private ISettingsService Settings => Ioc.Default.GetRequiredService<ISettingsService>();

Uh oh!

Add launch page settings #634

Are you sure you want to change the base?

Add launch page settings #634

Uh oh!

Conversation

adrianghc commented Jun 27, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

United600 commented Jul 16, 2025

Uh oh!

adrianghc commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

United600 commented Jul 17, 2025

Uh oh!

adrianghc commented Jul 17, 2025

Uh oh!

United600 commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

huynhsontung Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as duplicate.

Uh oh!

adrianghc commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

United600 commented Aug 8, 2025

Uh oh!

huynhsontung commented Aug 9, 2025

Uh oh!

adrianghc commented Sep 1, 2025

Uh oh!

huynhsontung left a comment

Choose a reason for hiding this comment

Uh oh!

huynhsontung Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

huynhsontung Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huynhsontung Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

adrianghc Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

huynhsontung Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

adrianghc Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

adrianghc commented Sep 29, 2025

Uh oh!

Uh oh!

adrianghc commented Jul 16, 2025 •

edited

Loading

United600 commented Jul 17, 2025 •

edited

Loading

huynhsontung Sep 22, 2025 •

edited

Loading

adrianghc commented Aug 8, 2025 •

edited

Loading

huynhsontung Sep 22, 2025 •

edited

Loading