Implement the Settings UI #8048

leonMSFT · 2020-10-26T18:35:39Z

Summary of the Pull Request

This introduces the Settings UI to Windows Terminal. This allows users to customize their global settings, color schemes, and profiles without having to open the settings.json. Actions are not currently included in the Settings UI.

To open the Settings UI, add the following keybinding to your settings.json:

{ "name": "OPEN SETTINGS UI", "command": {"action": "openSettings", "target": "settingsUI"}, "keys": "ctrl+shift+," }

Closes #1564 - Settings UI
Spec: #6720, #8269

Demo

miniksa

Only minor comments. Let's do this!

src/cascadia/TerminalSettingsEditor/Interaction.xaml

src/cascadia/TerminalSettingsEditor/KeyBindingsPage.xaml

src/cascadia/TerminalSettingsEditor/Keybindings.cpp

src/cascadia/TerminalSettingsEditor/MainPage.cpp

src/cascadia/TerminalSettingsEditor/Profiles.cpp

src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw

zadjii-msft · 2020-12-09T03:10:44Z

Random thought while reading: we might want to actually make an updated version of AddASetting.md for the Terminal now with all this. For ex, if you add any sort of enum setting, you need to add it to a bunch of different places so it can be exposed in the SUI. Maybe a checklist would be helpful to have?
(47/90)

zadjii-msft

okay I'm at 69/90 but I think he's finally asleep so I'm going to bed

zadjii-msft · 2020-12-09T02:51:31Z

src/cascadia/TerminalApp/SettingsTab.cpp

+            TabViewItem().IconSource(IconPathConverter::IconSourceMUX(glyph));
+
+            // Update SwitchToTab command's icon
+            SwitchToTabCommand().Icon(glyph);


Oh this will likely conflict with #8427, but git might not be the thing that complains about it. @Don-Vito as an FYI

@zadjii-msft - not sure about the git but the compiler will complain for sure :) In any case the solution will be straightforwad - removing this line since the palette items are "bound" to TabBase::Icon.

src/cascadia/TerminalSettingsModel/Profile.h

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

zadjii-msft · 2020-12-09T03:06:14Z

src/cascadia/TerminalSettingsEditor/ColorToBrushConverter.cpp

+                                                            Foundation::IInspectable const& /* parameter */,
+                                                            hstring const& /* language */)
+    {
+        return winrt::box_value(Windows::UI::Xaml::Media::SolidColorBrush(winrt::unbox_value<Windows::UI::Color>(value)));


do we have to worry about this unbox_value throwing an exception if value isn't a Color

Technically, yes. But considering that this is a Color-to-Brush converter, I feel like this is fine? (also, idk how to check the type of something without unboxing it, would try_as work?)

yea, try_as returns null if it couldn't do the conversion.

src/cascadia/TerminalSettingsModel/EnumMappings.cpp

src/cascadia/TerminalSettingsEditor/Utils.cpp

src/cascadia/TerminalSettingsEditor/GlobalSettings.idl

src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.filters

zadjii-msft

Okay, I'm through 90/90. I've got a pile of nits and questions so I'll hold for answers, but there's nothing dramatic here

src/cascadia/TerminalSettingsEditor/ColorSchemes.cpp

zadjii-msft · 2020-12-09T16:41:17Z

src/cascadia/TerminalSettingsEditor/Profiles.cpp

+        _State.WindowRoot().TryPropagateHostingWindow(picker); // if we don't do this, there's no HWND for it to attach to
+        picker.ViewMode(PickerViewMode::Thumbnail);
+        picker.SuggestedStartLocation(PickerLocationId::ComputerFolder);
+        picker.FileTypeFilter().ReplaceAll({ L".bat", L".exe", L".cmd" });


wait, we definitely can't put .bat or .cmd's in here directly -

holy shit what, you can CreateProcess(foo.bat) and it works? That's insane.

Either way, if you do select a .bat file, then apply, it seems to roundtrip it as cmd.exe. Not sure if that counts as a follow-up though

Wait that might just be it's not persisting at all. That's probably something else, nevermind me.

zadjii-msft · 2020-12-09T16:59:40Z

src/cascadia/TerminalSettingsEditor/Profiles.cpp

+
+    fire_and_forget Profiles::StartingDirectory_Click(IInspectable const&, RoutedEventArgs const&)
+    {
+        auto lifetime = get_strong();


Should these be weak refs that we check on the other side of the co_await?

I'll be honest, I straight up took this from the FilePicker Sample. We don't even use lifetime. I'll have to play around a bit here to see if that works. I'll get back to you.

so, it really doesn't matter. lifetime anchors the page so that it stays alive while the button click handler is being run.

That's a good thing.

We've gone crazy with the weak references in Terminal. We make them weak, check if they're alive, all the dang time

src/cascadia/TerminalSettingsEditor/Profiles.cpp

src/cascadia/TerminalSettingsEditor/MainPage.cpp

zadjii-msft · 2020-12-09T18:28:46Z

src/cascadia/TerminalApp/TerminalPage.cpp

+            _tabs.Append(*newTabImpl);
+            _mruTabActions.Append(newTabImpl->SwitchToTabCommand());
+
+            newTabImpl->SetDispatch(*_actionDispatch);
+
+            // Give the tab its index in the _tabs vector so it can manage its own SwitchToTab command.
+            _UpdateTabIndices();
+
+            // Don't capture a strong ref to the tab. If the tab is removed as this
+            // is called, we don't really care anymore about handling the event.
+            auto weakTab = make_weak(newTabImpl);
+
+            auto tabViewItem = newTabImpl->TabViewItem();
+            _tabView.TabItems().Append(tabViewItem);


A lot of this seems similar to the setup we do when we create a terminal tab, should we make a card to unify it?

This was some of the code that Leon wrote up. I'm a bit worried that I may break some tab switching functionality in the process of moving stuff around. For now, I've lumped it into #8452 since that seems to be touching some similar code. I'll do it as a follow-up to this PR since this is already a pretty big one.

src/cascadia/TerminalApp/TerminalPage.cpp

src/cascadia/TerminalSettingsEditor/MainPage.cpp

zadjii-msft

okay. lets do this. We can polish in post

src/cascadia/TerminalSettingsEditor/Profiles.cpp

src/cascadia/TerminalSettingsEditor/MainPage.cpp

zadjii-msft · 2020-12-11T13:31:15Z

src/cascadia/TerminalSettingsEditor/MainPage.xaml

+                <Grid.ColumnDefinitions>
+                    <ColumnDefinition Width="*"></ColumnDefinition>
+                </Grid.ColumnDefinitions>
+                <TextBlock x:Uid="Settings_UnsavedSettingsWarning"


This currently isnt used, right?

Correct. We need a way to detect unsaved changes to properly show/hide it. #8552 starts getting us there, so I'll keep it in there for now. But I'll definitely add a comment explaining that (and filing a follow-up)

DHowett · 2020-12-11T21:01:47Z

src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h

@@ -168,7 +167,7 @@ struct ::Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<::winr
        }

        ::winrt::Windows::UI::Text::FontWeight weight{
-            static_cast<uint16_t>(std::clamp(value, 100u, 990u))
+            static_cast<uint16_t>(std::clamp(value, 1u, 999u))


I'm sorry, what? 1 is not a valid font weight

I'm sorry, what? 1 is not a valid font weight

Typically goes from 100 to 990 I think - but I believe the font sets its minimum and maximum values?

https://docs.microsoft.com/en-us/typography/opentype/spec/fvar

Example This example is for a hypothetical font with family name “SelawikV” that has two axes of variation, for weight and width. This table summarizes the description of the axes for the font:

Axis tag Minimum value Default value Maximum value Axis name ID

'wght' 300 400 700 256

'wdth' 62.5 100 150 257

But the documentation for FontWeight says that this is the supported range though. Do we clamp it somewhere else?

I backed this out and clamped it to 100 on deserialization. On a round trip it will be "fixed".

This commit is an amalgamation of some of the TSM changes in PR #8048. It: * Introduces CascadiaSettings.CreateNewProfile to add a new profile * Introduces CascadiaSettings.ProfileDefaults, which returns the "defaults" object as a profile * Fixes the font weight deserializer to work on uint16_ts * Fixes a property getter in ColorScheme to not be a property getter * Fixes a reserialization error with default profiles * Sets a default icon for all profiles (to the C:\ Segoe MDL2 icon)

This commit introduces the terminal settings editor (to wit: the Settings UI) as a standalone project. This project, and this commit, is the result of two and a half months of work. TSE started as a hackathon project in the Microsoft 2020 Hackathon, and from there it's grown to be a bona-fide graphical settings editor. There is a lot of xaml data binding in here, a number of views and a number of view models, and a bunch of paradigms that we've been reviewing and testing out and designing and refining. Specified in #6720, #8269 Follow-up work in #6800 Closes #1564 Closes #8048 (PR) Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com> Co-authored-by: Kayla Cinnamon <cinnamon@microsoft.com> Co-authored-by: Alberto Medina Gutierrez <almedina@microsoft.com> Co-authored-by: John Grandle <jograndl@microsoft.com> Co-authored-by: xerootg <xerootg@users.noreply.github.com> Co-authored-by: Scott <sarmiger1@gmail.com> Co-authored-by: Vineeth Thomas Alex <vineeththomasalex@gmail.com> Co-authored-by: Leon Liang <lelian@microsoft.com> Co-authored-by: Dustin L. Howett <duhowett@microsoft.com> Signed-off-by: Dustin L. Howett <duhowett@microsoft.com>

This commit iontroduces another `target` to the `openSettings` binding: `settingsUI`. It opens the settings UI introduced in the previous commit. Closes #1564 Closes #8048 (PR) Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com> Co-authored-by: Leon Liang <lelian@microsoft.com>

This commit introduces the terminal settings editor (to wit: the Settings UI) as a standalone project. This project, and this commit, is the result of two and a half months of work. TSE started as a hackathon project in the Microsoft 2020 Hackathon, and from there it's grown to be a bona-fide graphical settings editor. There is a lot of xaml data binding in here, a number of views and a number of view models, and a bunch of paradigms that we've been reviewing and testing out and designing and refining. Specified in #6720, #8269 Follow-up work in #6800 Closes #1564 Closes #8048 (PR) Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com> Co-authored-by: Kayla Cinnamon <cinnamon@microsoft.com> Co-authored-by: Alberto Medina Gutierrez <almedina@microsoft.com> Co-authored-by: John Grandle <jograndl@microsoft.com> Co-authored-by: xerootg <xerootg@users.noreply.github.com> Co-authored-by: Scott <sarmiger1@gmail.com> Co-authored-by: Vineeth Thomas Alex <vineeththomasalex@gmail.com> Co-authored-by: Leon Liang <lelian@microsoft.com> Co-authored-by: Dustin L. Howett <duhowett@microsoft.com> Signed-off-by: Dustin L. Howett <duhowett@microsoft.com>

This commit is an amalgamation of some of the TSM changes in PR microsoft#8048. It: * Introduces CascadiaSettings.CreateNewProfile to add a new profile * Introduces CascadiaSettings.ProfileDefaults, which returns the "defaults" object as a profile * Fixes the font weight deserializer to work on uint16_ts * Fixes a property getter in ColorScheme to not be a property getter * Fixes a reserialization error with default profiles * Sets a default icon for all profiles (to the C:\ Segoe MDL2 icon)

This commit introduces the terminal settings editor (to wit: the Settings UI) as a standalone project. This project, and this commit, is the result of two and a half months of work. TSE started as a hackathon project in the Microsoft 2020 Hackathon, and from there it's grown to be a bona-fide graphical settings editor. There is a lot of xaml data binding in here, a number of views and a number of view models, and a bunch of paradigms that we've been reviewing and testing out and designing and refining. Specified in microsoft#6720, microsoft#8269 Follow-up work in microsoft#6800 Closes microsoft#1564 Closes microsoft#8048 (PR) Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com> Co-authored-by: Kayla Cinnamon <cinnamon@microsoft.com> Co-authored-by: Alberto Medina Gutierrez <almedina@microsoft.com> Co-authored-by: John Grandle <jograndl@microsoft.com> Co-authored-by: xerootg <xerootg@users.noreply.github.com> Co-authored-by: Scott <sarmiger1@gmail.com> Co-authored-by: Vineeth Thomas Alex <vineeththomasalex@gmail.com> Co-authored-by: Leon Liang <lelian@microsoft.com> Co-authored-by: Dustin L. Howett <duhowett@microsoft.com> Signed-off-by: Dustin L. Howett <duhowett@microsoft.com>

…t#8048) This commit iontroduces another `target` to the `openSettings` binding: `settingsUI`. It opens the settings UI introduced in the previous commit. Closes microsoft#1564 Closes microsoft#8048 (PR) Co-authored-by: Carlos Zamora <carlos.zamora@microsoft.com> Co-authored-by: Leon Liang <lelian@microsoft.com>

zadjii-msft assigned zadjii-msft and unassigned zadjii-msft Oct 28, 2020

carlos-zamora changed the title ~~Merge the SUI feature branch into main~~ Implement the Settings UI Dec 3, 2020

ghost added Area-Settings Issues related to settings and customizability, for console or terminal Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Scenario Product-Terminal The new Windows Terminal. labels Dec 3, 2020

zadjii-msft self-assigned this Dec 7, 2020

miniksa approved these changes Dec 7, 2020

View reviewed changes

carlos-zamora self-assigned this Dec 8, 2020

zadjii-msft requested changes Dec 9, 2020

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Dec 9, 2020

zadjii-msft requested changes Dec 9, 2020

View reviewed changes

DHowett reviewed Dec 10, 2020

View reviewed changes

src/cascadia/TerminalSettingsEditor/MainPage.cpp Outdated Show resolved Hide resolved

zadjii-msft approved these changes Dec 11, 2020

View reviewed changes

zadjii-msft removed their assignment Dec 11, 2020

carlos-zamora mentioned this pull request Dec 11, 2020

Epic: Settings UI Completion #6800

Open

25 tasks

DHowett reviewed Dec 11, 2020

View reviewed changes

DHowett and others added 2 commits December 11, 2020 13:35

DHowett force-pushed the feature/settings-ui branch from 81a50de to 0cab86f Compare December 11, 2020 21:45

DHowett merged commit d02812d into main Dec 11, 2020

cinnamon-msft mentioned this pull request Jan 22, 2021

Add remaining 1.6 release features MicrosoftDocs/terminal#223

Closed

15 tasks

zadjii-msft mentioned this pull request Jan 29, 2021

Going to the settings option launches Altova XMLSpy 2020 ! #8940

Closed

opticyclic mentioned this pull request Feb 15, 2021

Settings UI Is Not Visible #9134

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the Settings UI #8048

Implement the Settings UI #8048

leonMSFT commented Oct 26, 2020 •

edited by miniksa

Loading

miniksa left a comment

zadjii-msft commented Dec 9, 2020 •

edited

Loading

zadjii-msft left a comment

zadjii-msft Dec 9, 2020

Don-Vito Dec 9, 2020

zadjii-msft Dec 9, 2020

carlos-zamora Dec 9, 2020

zadjii-msft Dec 9, 2020

zadjii-msft left a comment

zadjii-msft Dec 9, 2020

zadjii-msft Dec 9, 2020

zadjii-msft Dec 9, 2020

zadjii-msft Dec 9, 2020

carlos-zamora Dec 9, 2020

DHowett Dec 11, 2020

zadjii-msft Dec 9, 2020

carlos-zamora Dec 9, 2020

zadjii-msft left a comment

zadjii-msft Dec 11, 2020

carlos-zamora Dec 11, 2020

DHowett Dec 11, 2020

mdtauk Dec 11, 2020

carlos-zamora Dec 11, 2020

DHowett Dec 11, 2020

Implement the Settings UI #8048

Implement the Settings UI #8048

Conversation

leonMSFT commented Oct 26, 2020 • edited by miniksa Loading

Summary of the Pull Request

Demo

miniksa left a comment

Choose a reason for hiding this comment

zadjii-msft commented Dec 9, 2020 • edited Loading

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonMSFT commented Oct 26, 2020 •

edited by miniksa

Loading

zadjii-msft commented Dec 9, 2020 •

edited

Loading