-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Profile settings support #47
Profile settings support #47
Conversation
Right now it somewhat matches what is required from the StackPanel ordering orientation. CellVisualizer and Cell.API updated to match.
trying to do all at once and each manage their own timeouts.
Add ability to hit escape to cancel/get out of the AddTabFlyout UI. Also remove our kb event listener on close.
…ing WMI. Added to our hwndhost wrapper too.
Major additions/changes see PR for breakdown Developer-Sanctuary#47
2f6b6e8
to
8849240
Compare
Major additions/changes see PR for breakdown
8849240
to
8649ae0
Compare
So "Change Cell Orientation in code to better match the logical expectations" commit doesn't work properly. Everything seems to be fine except for the drag over. When you drag over the drop regions are opposite orientation (so if you have a vertical split with 2 tall cells when you drag over it had the entire bottom half of the window drop for the right cell and the entire top half for the left). I looked over what I could find in UE CellVisualizer and Cell API calls and all the orientation terms seem to be correct to me. That would lead me to think it may be something in EasyXAMLTools doing this. I looked again at reverting this but the terms in my mind seem to make sense with this change. A vertical split has a vertical orientation for a cell, it results in a horizontal stack panel (aka cells stacked left to right) and a vertical panel resizer between cells. |
CurrentCell = null; | ||
CurrentCell = null!; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll fix the source generator for this might be better than just ignoring the fact that the value can be null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
w/e you like:) When something breaks generated code often doesn't get compiled the error window is full of useless things. Compiler output's first error is generally good but sometimes it was so many warnings tough to read. Goal was just reduce the null warning spam.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fix the generator on latest commit in master branch
@@ -78,7 +78,7 @@ | |||
</Rectangle> | |||
</Grid> | |||
</DataTemplate> | |||
<DataTemplate x:Name="VerticalCellDataTemplate" x:DataType="class:Cell"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I guess that works but I would rename them to HorizontalSplitCellDataTemplate
and VerticalSplitCellDataTemplate
after merging to avoid confusion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment in the PR itself, as this is still not quite working properly and I don't know why.
async void HandleCLICmds() { | ||
var toAdd = CLI.GetArrVal("add-window-by-exe"); | ||
var editLastAddedWindow = CLI.GetFlag("edit-last-added"); | ||
LeftFlyout.NoAutoClose = CLI.GetFlag("edit-no-autoclose"); | ||
var profile = CLI.GetVal("profile"); | ||
if (!String.IsNullOrWhiteSpace(profile)) { | ||
if (Path.HasExtension(profile) == false) | ||
profile += ".json"; | ||
if (! File.Exists(profile) && ! Path.IsPathRooted(profile) ) | ||
profile = Path.Combine(USConfig.BaseProfileFolder,profile); | ||
if (File.Exists(profile)) { | ||
await Task.Delay(1500); | ||
await persistantService.ImportSettings(profile); | ||
} | ||
|
||
|
||
} | ||
|
||
foreach (var itm in toAdd) { | ||
var procs = System.Diagnostics.Process.GetProcesses().Where(p => p.ProcessName.Equals(itm, StringComparison.OrdinalIgnoreCase)).ToList(); | ||
foreach (var proc in procs) { | ||
if (!proc.HasExited) | ||
AddTab( WindowEx.FromWindowHandle(proc.MainWindowHandle)); | ||
} | ||
} | ||
if (editLastAddedWindow && Tabs.Count > 0) | ||
Tabs.Last().TabDoubleTapped(this, new Microsoft.UI.Xaml.Input.DoubleTappedRoutedEventArgs()); | ||
|
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double }
at the end, causing compile error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I did selective staging to make the commits a bit cleaner, I caught this pretty fast but I think you had already cloned it by then.
… to save and restore UWP processes.
…elegant solution in OnNotNull. UWP apps working, delayed tab field setting.
Thanks for the hard work. I will try my best to resolve conflicts and see if this works |
There are some commits in this PR not directly related to the PR itself. I also did not squish the PR for this reason. They can be put in as separate PR but to make merging a bit easier did it as one PR with multiple commits (these are all the initial commits and are commented to their purpose).
Even though this PR is large I wouldn't make a release including it yet. It probably deserves some internal testing first and like a few more changes to make it a bit more user friendly. Still didn't want it to grow further in complexity for the initial review.
There is code commented out in a few select spots where I would have liked to do X but couldn't, normally not a big fan of leaving code that isn't used in but might prevent someone wasting time trying the same without knowing why it won't work, and if the WinUI limitations are fixed can be used at that point. Could also be hidden behind feature flags.
The main idea of profiles is to allow layered / stacked settings to exist. Rather than just one config used for all the settings (even if you let the user choose which config) this designs is so a config can contain only partial settings that get put on top of the current instance. The app starts with an internal "default config" if a default config exists that is loaded at startup and any entires in there override the internal defaults.
Further in the user requests an addition config be loaded (--profile cli) or from the settings menu this gets applied on top. Configs can store certain settings or can store tabs/cells to restore.
Right now when exporting the config it only exports everything, but you can manually remove things from the json to do partial profile loads. There is only one setting right now at export "All or just things different from defaults" but because of that I don't automatically close the window when you pick the filename in the browse dialog to save to (you have to actually hit save). In time I think this should grow to a tree dialog showing you all the settings it will export with checkboxes next to them to disable them.
The load dialog also doesn't automatically activate until you hit load, but here it may make sense to automatically close it and load when you select a profile (as no other settings). This could be easily done (as the code is already commented out) in EpxortImportInputViewModel around line #69 (with a check to make sure on Save NotLoad.
Almost all config file settings are nullable specifically to allow empty /false to be determined separately from null. A settings file with the titlebar override set to empty string "" is very different from one that does not set it at all. If it is set but empty that means the existing titlebar will be set to empty (if not already) if in the loading profile it is missing/null however then nothing happens.
Certain refactoring was done to allow for external cell/window creation.
This adds the import/export buttons to the settings menu
Adds new UI for ExportImportVM that prompts user for basic settings and the profile to use.
Removes FauxSettings and FeatureFlags and merged the remaining required items into the USConfig class.
Some of the new settings were added to the settings page, most are json config only.
When exporting you can select to export all settings rather than just those that differ from the defaults to see what others are the re.
Removed the setter background thread for Settings window we now properly dispatch where needed on setting changes.
PreservedHelpers.cs - These functions follow the idea of if this property is set in the new profile, override the current property as long as it is valid, otherwise throw an exception. Sadly all functions here are duplicated to support full value type/struct args. I couldn't figure out how to properly cast a struct arg call to call the class version, mostly because behind the scenes a Nullable value type is quite different from a Nullable reference type. A hope would have been something like:
public static SRC_TYPE DoOrThrow<SRC_TYPE>(SRC_TYPE? data, Action<SRC_TYPE> OnSuccess, Func<SRC_TYPE, bool> MustBeTrue, Func? OnFail = null) where SRC_TYPE : struct => DoOrThrow(data,OnSuccess,MustBeTrue,OnFail); This would not work for all situations though. This class also contains some color converter helpers as WinUI color class is quite lacking.
PropHelper.cs - Allows copying properties from one instance of a class to another, optionally recursively. The recursive flag is quite 'dangerous' as it tries to recurse into sub value/reference types and work on each property individually. This was needed to avoid a lot of boilerplate code for Clone() and dealing with config classes with sub classes etc. To try and prevent accidental failure it is scoped to only work on our specific classes, not on arrays, and not on primitives/enums. Was lazy here and didn't want to go through and fix all the nullable options so just disabled all warnings.
WinUI manual theme setting is basically broken, so stopped users from being able to pick a theme (Sad as live theme changes would be nice and easy to implement).
There is a USConfig class, this is very close to just being an alias for SavedInstanceData (the root class for saved profiles) but is an easier name to read while keeping SavedInstanceData in line with all the other profile data classes.
To keep some classes/code as close to existing as possible there are some oddities where properties are forwarded to another class. These could be changed.
Automatically close our settings pane when it is likely the user would not pick another action from it (more true flyout style).
Settings are only for the running instance unless the user selects SaveAsDefault which makes them the defaults.
I have one additional set of test cases for CellCreation I will put in a separate PR after this. They should not be merged into master but we should look to see if they warrant us making any changes to fix some of them.