Polish OpenSettings action for Settings UI and Profile page navigation on refresh (#8670)

Performs a number of minor bugfixes related to the Settings UI:
- b5370a1 Dropdown bug:
  - the dropdown would display the keybinding for the first
    `openSettings` found. So it would accidentally present and bind the
    one for the Settings UI.
- 91eb49e autogenerated name for opening Settings UI:
  - the Settings UI keybinding would display "open settings file". This
    was updated to say "Open Settings UI".
- 1cadbf4 Profile Page navigation crash:
  - the selected item off of a MUX navigation view returns a MUX
    NavViewItem (as opposed to WUX)
-  dd2f3e5 Hookup delete for Profile page navigation:
   - missed a spot where we were manually navigating to the Profile
     page. So it wasn't hooked up properly
- 9fea6de Properly cast NavViewItem tags
  - When we update the NavigationView's menu items, we were casting the
    tags to `Model::Profile` instead of `Editor::ProfileViewModel`.

## References
#6800 - Settings UI epic

Fixes the following bug:
> - [ ] JSON change --> crash
>   - open SUI --> open JSON --> edit retro effects in JSON --> save file --> cry because the app crashed

## Additional comments
This was a part of some manual testing I performed on the Settings UI.
More intricate bugs are being reported on #6800 and will be fixed in
their own PR.
This commit is contained in:
Carlos Zamora 2021-01-04 16:14:51 -06:00 committed by GitHub
parent 08646e5ca3
commit 990e06b445
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 17 additions and 6 deletions

View file

@ -602,7 +602,9 @@ namespace winrt::TerminalApp::implementation
settingsItem.Click({ this, &TerminalPage::_SettingsButtonOnClick });
newTabFlyout.Items().Append(settingsItem);
auto settingsKeyChord = keyBindings.GetKeyBindingForAction(ShortcutAction::OpenSettings);
Microsoft::Terminal::Settings::Model::OpenSettingsArgs args{ SettingsTarget::SettingsFile };
Microsoft::Terminal::Settings::Model::ActionAndArgs settingsAction{ ShortcutAction::OpenSettings, args };
const auto settingsKeyChord{ keyBindings.GetKeyBindingForActionWithArgs(settingsAction) };
if (settingsKeyChord)
{
_SetAcceleratorForMenuItem(settingsItem, settingsKeyChord);

View file

@ -76,7 +76,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
if (const auto tag{ navViewItem.Tag() })
{
if (tag.try_as<Model::Profile>())
if (tag.try_as<Editor::ProfileViewModel>())
{
// hide NavViewItem pointing to a Profile
navViewItem.Visibility(Visibility::Collapsed);
@ -104,13 +104,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
if (const auto tag{ selectedItem.as<MUX::Controls::NavigationViewItem>().Tag() })
{
if (const auto profile{ tag.try_as<Model::Profile>() })
if (const auto oldProfile{ tag.try_as<Editor::ProfileViewModel>() })
{
// check if the profile still exists
if (_settingsClone.FindProfile(profile.Guid()))
if (const auto profile{ _settingsClone.FindProfile(oldProfile.Guid()) })
{
// Navigate to the page with the given profile
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), winrt::make<ProfilePageNavigationState>(_viewModelForProfile(profile), _settingsClone.GlobalSettings().ColorSchemes(), *this));
_Navigate(_viewModelForProfile(profile));
return;
}
}
@ -126,7 +126,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
// could not find the page we were on, fallback to first menu item
const auto firstItem{ navigationMenu.MenuItems().GetAt(0) };
navigationMenu.SelectedItem(firstItem);
if (const auto tag{ navigationMenu.SelectedItem().as<NavigationViewItem>().Tag() })
if (const auto tag{ navigationMenu.SelectedItem().as<MUX::Controls::NavigationViewItem>().Tag() })
{
_Navigate(unbox_value<hstring>(tag));
}

View file

@ -292,6 +292,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
return RS_(L"OpenDefaultSettingsCommandKey");
case SettingsTarget::AllFiles:
return RS_(L"OpenBothSettingsFilesCommandKey");
case SettingsTarget::SettingsUI:
return RS_(L"OpenSettingsUICommandKey");
default:
return RS_(L"OpenSettingsCommandKey");
}

View file

@ -441,6 +441,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
struct OpenSettingsArgs : public OpenSettingsArgsT<OpenSettingsArgs>
{
OpenSettingsArgs() = default;
OpenSettingsArgs(const SettingsTarget& target) :
_Target{ target } {}
GETSET_PROPERTY(SettingsTarget, Target, SettingsTarget::SettingsFile);
static constexpr std::string_view TargetKey{ "target" };
@ -850,4 +852,5 @@ namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
BASIC_FACTORY(CloseOtherTabsArgs);
BASIC_FACTORY(CloseTabsAfterArgs);
BASIC_FACTORY(MoveTabArgs);
BASIC_FACTORY(OpenSettingsArgs);
}

View file

@ -147,6 +147,7 @@ namespace Microsoft.Terminal.Settings.Model
[default_interface] runtimeclass OpenSettingsArgs : IActionArgs
{
OpenSettingsArgs(SettingsTarget target);
SettingsTarget Target { get; };
};

View file

@ -363,4 +363,7 @@
<data name="BreakIntoDebuggerCommandKey" xml:space="preserve">
<value>Break into the debugger</value>
</data>
<data name="OpenSettingsUICommandKey" xml:space="preserve">
<value>Open Settings...</value>
</data>
</root>