Refactor Application Settings and Support for Packaged/Unpackaged Mode #1924 #2014

ghost1372 · 2025-08-07T14:32:37Z

This PR replaces the legacy ApplicationData-based settings system with a new SettingsHelper class. It adds support for both packaged and unpackaged app scenarios.

Closes #1924.

Old Settings System:

appData.LocalSettings.Values[SettingsKeys.IsLeftMode] = true;
New Settings System:

SettingsHelper.Current.IsLeftMode = true;

Note

Settings are automatically saved when changed.

SettingsHelper Architecture:

Uses an ISettingsProvider abstraction.
In packaged mode: uses ApplicationDataContainer.
In unpackaged mode: uses JsonSettingsProvider.

Adding a New Setting:

public bool IsEnabled
{
    get => GetOrCreateDefault<bool>(true);
    set => Set(value);
}

…centralize settings keys (#1900) ## Description - Replace `Windows.Storage.ApplicationData` with `Microsoft.Windows.Storage.ApplicationData`. - Centralize Settings Keys in a `SettingsKeys` Class. - define key constants in `ScratchPadPage`. ## Motivation and Context - Closes #1892 - Closes #1893 ## How Has This Been Tested? **Manually tested** ## Screenshots (if appropriate): ## Types of changes  - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) Co-authored-by: Niels Laute <niels.laute@live.nl>

Zakariathr22

Just a few points that I think are important, but of course after hearing @marcelwgn’s opinion about them.

WinUIGallery/Helpers/SettingsHelper/Internals/ListExtensions.cs

WinUIGallery/Helpers/SettingsHelper/Internals/Vectors.cs

WinUIGallery/Helpers/SettingsHelper/Providers/ApplicationDataSettingsProvider.cs

WinUIGallery/Pages/HomePage.xaml.cs

WinUIGallery/Helpers/SettingsHelper/ObservableSettings.cs

WinUIGallery/Pages/SettingsPage.xaml.cs

…r Usage

Copilot

Pull Request Overview

This PR refactors the application settings system by replacing the legacy ApplicationData-based approach with a new SettingsHelper class that provides type-safe property access and supports both packaged and unpackaged app scenarios.

Introduces a new SettingsHelper class with an ISettingsProvider abstraction
Replaces string-based settings keys with strongly-typed properties
Adds JSON-based settings storage for unpackaged apps while maintaining ApplicationData support for packaged apps

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
SettingsHelper/SettingsHelper.cs	New settings class with type-safe properties and update methods
SettingsHelper/Providers/*	Provider abstraction with ApplicationData and JSON implementations
SettingsHelper/ObservableSettings.cs	Base class providing property change notifications and storage operations
SettingsHelper/Internals/*	Support classes for JSON serialization and list operations
SettingsKeys.cs	Removed legacy string-based settings keys
Various pages/helpers	Updated to use new SettingsHelper.Current API instead of legacy methods

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-30T13:00:58Z

WinUIGallery/Helpers/SettingsHelper/Providers/JsonSettingsProvider.cs

+        catch (Exception)
+        {
+            HandleCorruptedFile();
+        }


[nitpick] Catching generic Exception can hide unexpected errors. Consider catching specific exceptions like JsonException or IOException for better error handling.

Suggested change

catch (Exception)

{

HandleCorruptedFile();

}

catch (JsonException)

{

HandleCorruptedFile();

}

catch (IOException)

{

HandleCorruptedFile();

}

Having multiple specific exceptions can be useful for clarity, but in this case, it may not be necessary. We could handle multiple error cases with a single exception type, which would allow us to catch all errors uniformly instead of only the predefined ones.

Copilot · 2025-08-30T13:00:58Z

WinUIGallery/Helpers/SettingsHelper/Providers/ApplicationDataSettingsProvider.cs

+        catch (Exception)
+        {
+            HandleCorruptedKey(key);
+            return default;
+        }


[nitpick] Catching generic Exception can hide unexpected errors. Consider catching specific exceptions like JsonException or InvalidCastException for better error handling.

Suggested change

catch (Exception)

{

HandleCorruptedKey(key);

return default;

}

catch (JsonException)

{

HandleCorruptedKey(key);

return default;

}

catch (InvalidCastException)

{

HandleCorruptedKey(key);

return default;

}

catch (FormatException)

{

HandleCorruptedKey(key);

return default;

}

WinUIGallery/Helpers/SettingsHelper/Providers/ApplicationDataSettingsProvider.cs

WinUIGallery/Helpers/SettingsHelper/Providers/JsonSettingsProvider.cs

…ider.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ettingsProvider.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Zakariathr22

Looks like there’s an issue when running the app unpackaged:

Navigate to App notification page
Click Show notification button

The app crashes with an unhandled exception.

ghost1372 · 2025-09-10T12:29:10Z

Looks like there’s an issue when running the app unpackaged:

Navigate to App notification page

Click Show notification button

The app crashes with an unhandled exception.

The AppNotificationManager throws an "Element Not Found" exception and is not related to this PR!
This crash can be easily fixed but should be addressed in a separate PR.

As described here, we need to register it. I tested it, and it works fine in the unpackaged scenario. So, this requires additional work and should be done in a separate PR (we can keep the current logic for packaged mode and add a registration/scenario for unpackaged mode). Here are the Microsoft docs and samples for unpackaged scenarios.
https://learn.microsoft.com/en-us/windows/apps/develop/notifications/app-notifications/app-notifications-quickstart?tabs=cs
https://github.com/microsoft/WindowsAppSDK-Samples/tree/main/Samples/Notifications/App/CsUnpackagedAppNotifications/CsUnpackagedAppNotifications

marcelwgn

Thank you for working on this! There are some comments and one issue surrounding lists.

Previously, we used a string and comma separated the entries in there to represent that. The new code doesnt account for that which results in people losing their favorites and recently viewed as the old field (string) does not match the expected type (list of strings).
We need some conversion code for those so we can migrate users.

WinUIGallery/Helpers/SettingsHelper/Internals/ListExtensions.cs

WinUIGallery/Helpers/SettingsHelper/Providers/SettingsProviderFactory.cs

marcelwgn · 2025-09-28T11:48:24Z

WinUIGallery/MainWindow.xaml.cs

    private void SetWindowProperties()
    {
-#if DEBUG
+#if DEBUG || DEBUG_UNPACKAGED


Is Debug_Unpackaged not also ifdeffing DEBUG?

In Unpackaged Mode with the Debug profile, I always encounter COM registration exceptions.

However, if I use Debug-Unpackaged, the app works fine, but the 'Dev' text does not appear unless I add || DEBUG_UNPACKAGED

marcelwgn · 2025-09-28T11:50:01Z

WinUIGallery/Samples/ControlPages/Fundamentals/ScratchPadPage.xaml.cs

-        }
-
-        m_oldText = xamlStr;
+        m_oldText = GetDefaultScratchXAML();


Is it intentional we are no longer storing the xaml scratchpad content?

Yes, more info here
#2014 (comment)

WinUIGallery/Helpers/SettingsHelper/Providers/ApplicationDataSettingsProvider.cs

marcelwgn · 2025-09-28T11:59:00Z

WinUIGallery/Helpers/SettingsHelper/Providers/ApplicationDataSettingsProvider.cs

+                return (T)JsonSerializer.Deserialize(str, typeInfo);
+            }
+
+            return (T)Convert.ChangeType(value, typeof(T));


This is a bit suspicious, are we actually hitting this?

Since it’s been a long time since these lines were added, I don’t remember the exact reason (I am very busy these days.😅), but it was probably to prevent crashes and handle settings stored by the previous system. If you prefer, I can replace it with an InvalidCastException.

Renamed AddToFirst and AddToLast extension methods to AddAsFirst and AddAsLast for improved clarity and consistency.

Replaces usage of ProductNameAndVersion with ProductName

…tingsHelper to new SettingsHelper

ghost1372 · 2025-09-28T16:19:42Z

Hi @marcelwgn
Thank you for your review.
I’ve addressed the suggestions you made earlier.
I’ve added migration methods to preserve the user’s Favorites and Recently Visited items.

ghost1372 added 8 commits August 7, 2025 16:02

Implemented new settings system

9a9ae94

Replace Old Settings with New Settings

d5b5b9c

Remove Old Settings

8def68c

Rename Settings to SettingsHelper

c42d4d0

Remove unused codes

ec0bacf

Fix ScratchPadPage

51c3447

Update MainWindow.xaml.cs

b3da70f

use Try/Catch in Get methods

5bf1266

ghost1372 mentioned this pull request Aug 7, 2025

Refactor Application Settings with AppConfig and JSON Storage Support for Packaged/Unpackaged Apps #1924 #2001

Closed

3 tasks

Zakariathr22 reviewed Aug 10, 2025

View reviewed changes

ghost1372 added 4 commits August 10, 2025 21:32

Make ListExtensions More Generic

181fc43

Add Common Methods for Favorites and RecentlyVisited to Simplify Thei…

40e76e0

…r Usage

Remove Unsued Vector Wrapper

7f04f89

Remove ScratchPadXaml Setting

d8c7891

niels9001 requested review from Copilot and marcelwgn August 30, 2025 13:00

Copilot AI reviewed Aug 30, 2025

View reviewed changes

ghost1372 and others added 2 commits August 30, 2025 16:50

Update WinUIGallery/Helpers/SettingsHelper/Providers/JsonSettingsProv…

9bcced4

…ider.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update WinUIGallery/Helpers/SettingsHelper/Providers/ApplicationDataS…

b14fbb0

…ettingsProvider.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

marcelwgn self-assigned this Sep 8, 2025

Merge branch 'main' into AppSettings

b5a7e56

Zakariathr22 reviewed Sep 9, 2025

View reviewed changes

marcelwgn requested changes Sep 28, 2025

View reviewed changes

ghost1372 added 5 commits September 28, 2025 16:15

Merge branch 'main' into AppSettings

0e36b3f

Rename ListExtensions methods for clarity

e523217

Renamed AddToFirst and AddToLast extension methods to AddAsFirst and AddAsLast for improved clarity and consistency.

Use product name for settings folder path

425e96e

Replaces usage of ProductNameAndVersion with ProductName

Refactor Get<T>: restrict try/catch to JSON deserialization only

0361951

Add one-time migration for Favorites and RecentlyVisited from old Set…

1982f07

…tingsHelper to new SettingsHelper

Merge branch 'main' into AppSettings

90a12b8

Refactor Application Settings and Support for Packaged/Unpackaged Mode #1924 #2014

Are you sure you want to change the base?

Refactor Application Settings and Support for Packaged/Unpackaged Mode #1924 #2014

Uh oh!

Conversation

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

Uh oh!

SettingsHelper Architecture:

Adding a New Setting:

Uh oh!

Zakariathr22 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Zakariathr22 left a comment

Choose a reason for hiding this comment

Uh oh!

ghost1372 commented Sep 10, 2025

Uh oh!

marcelwgn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost1372 commented Sep 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghost1372 commented Aug 7, 2025 •

edited

Loading