Repository navigation
Port SettingsCard/Expander from WCT and update SUI to use it - #20232
Carlos Zamora (carlos-zamora) merged 26 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
|
UI changes I noticed while poking around:
I hit a point where I could go in and make everything look the same as we have it now, but I felt that that mildly contradicted the idea behind trying to make the UI "consistent" by using these controls. Niels Laute (@niels9001) I'll go through and do a second pass fixing the things above. Can you also go through it and point out any inconsistencies with the UI that should be addressed? The main files that are probably of interest are:
Once we figure that out and this gets reviewed/merged, we can probably use |
The issue is that we are now mixing-and-matching How big is the gap between |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
834a37e to
f7eb2fb
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
Dustin L. Howett (DHowett)
left a comment
There was a problem hiding this comment.
44/69 wait this does get rid of the "inherit" buttons still doesn't it
blocking to finish review, not because any of these changes are particularly objectionable except for the one (you know the one)
| @@ -5,6 +5,7 @@ | |||
|
|
|||
| #include "pch.h" | |||
| #include "Utils.h" | |||
| #include <winrt/Microsoft.Terminal.Settings.Editor.h> | |||
There was a problem hiding this comment.
surprised this isn't in our pch already
Leonard Hecker (lhecker)
left a comment
There was a problem hiding this comment.
6000 lines, yep looks good. 👍
| controlToFocus.StartBringIntoView(); | ||
| controlToFocus.Focus(winrt::Windows::UI::Xaml::FocusState::Programmatic); | ||
| // will end up doing nothing. | ||
| winrt::Microsoft::Terminal::Settings::ExpandAncestorsAndBringIntoView(page.template as<winrt::Windows::UI::Xaml::FrameworkElement>(), controlToFocus); |
There was a problem hiding this comment.
This is the first time I see a legit use of template foo in years. I'm reminded once again that C++ is not a real language. This is so ugly. lol
This comment has been minimized.
This comment has been minimized.
## Summary of the Pull Request Updates the settings UI to introduce a new Profiles page. This page includes: - a "Profile Defaults" navigator - a "Color Schemes" navigator (removed top-level item in favor of this) - an "add profile" split button (removed AddProfile page in favor of this) - a navigator for each profile - tooltips were added for the "hidden"/"orphaned" info badge ## Validation Steps Performed Scenarios: - [x] settings search - [X] defaults - [X] color schemes - [X] setting in defaults - [X] breadcrumbs - [X] navigate to individual profiles (via search and not) - [X] update settings for a profile - [X] accessibility testing ## PR Checklist Related to #17000 Stacked on #20232
Summary of the Pull Request
Ports the
SettingsCardandSettingsExpanderfrom Windows Community Toolkit to the TerminalSettingsEditor project:SettingsCardandSettingsExpanderwere ported over and reuse our macros to reduce boilerplate.Descriptionis applied asAutomationProperties.FullDescriptionfor accessibility. This is for accessibility compliance. Description text should be read out by the screen reader (scan mode isn't enough). Prior to this change, we would expose it asHelpText. This change exposes it asFullDescriptionwhich is more fitting as it's generally an opt-in behavior for screen reader users to hear. This prevents the user from being overwhelmed with information.SettingsControlsHelpersis used to make this much easier to review. Any converters, triggers, or other classes that are used for the migration were added there.Additionally, this PR performs the migration to use these new controls and removes
SettingContainerentirely. Associated changes:.HelpText-->.DescriptionSettingContainerStyle.xamland moving anything that's still used toCommonResources.xamlBackgroundImageSettingsVisible-->BackgroundImageSettingsEnabledto make use of the disabled stylingOther notable changes as a part of using the new controls:
SettingsCardSwitching to use the WCT port directly allows us to iterate faster on the settings UI and be more consistent with the Windows ecosystem. However, this does mean that the "reset button" had to be removed from the individual settings. This was already a weird pattern, so removing it shouldn't be too much of an issue. However, there was value to being able to reset settings. To provide this kind of functionality, a "reset" button is being added to the profile pages. This clears all settings in a profile so that they can apply their own customizations on top. It sacrifices granularity for simplicity and is a better understood concept.
Validation Steps Performed
PR Checklist
Related to #17000