Revert "Maintain current Pivot selection... (#8803)" (#8838)

This reverts commit a7d7362b95 introduced by #8803.

Reverting this commit fixes #8836 at the expense of the profile page
remembering the last Pivot selection. The 

## References
#6800 - Settings UI Epic

#8803 maintained a `ProfilePageNavigationState` in `MainPage` to remember
the pivot position. However, the two-way binding on the TextBoxes
now seem to happen too late (after the navigation occurs),
resulting in the text being applied to the wrong profile.  In other
words, the sequence of events probably looks something like this:

1. user types text (_state.profile = old profile)
2. user moves to new profile
3. navigation completes (_state.profile = new profile)
4. textbox two-way binding fires, setting _state.profile.WHATEVER = value

## Validation Steps Performed
Performed repro sets from #8836. Bug no longer occurs. 

Reopens #8769
Closes #8836
This commit is contained in:
Carlos Zamora 2021-01-21 02:50:49 -08:00 committed by GitHub
parent 9c4950cb32
commit b6f5f2a323
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 10 additions and 66 deletions

View file

@ -51,18 +51,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_InitializeProfilesList();
_colorSchemesNavState = winrt::make<ColorSchemesPageNavigationState>(_settingsClone.GlobalSettings());
// We have to provide _some_ profile in the profile nav state, so just
// hook it up with the base for now. It'll get updated when we actually
// navigate to a profile.
auto profileVM{ _viewModelForProfile(_settingsClone.ProfileDefaults()) };
profileVM.IsBaseLayer(true);
_profilesNavState = winrt::make<ProfilePageNavigationState>(profileVM,
_settingsClone.GlobalSettings().ColorSchemes(),
*this);
// Add an event handler for when the user wants to delete a profile.
_profilesNavState.DeleteProfile({ this, &MainPage::_DeleteProfile });
}
// Method Description:
@ -123,8 +111,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_InitializeProfilesList();
// Update the Nav State with the new version of the settings
_colorSchemesNavState.Globals(_settingsClone.GlobalSettings());
_profilesNavState.Schemes(_settingsClone.GlobalSettings().ColorSchemes());
// We'll update the profile in the _profilesNavState whenever we actually navigate to one
// now that the menuItems are repopulated,
// refresh the current page using the SelectedItem data we collected before the refresh
@ -269,11 +255,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
auto profileVM{ _viewModelForProfile(_settingsClone.ProfileDefaults()) };
profileVM.IsBaseLayer(true);
// Update the profiles navigation state
_profilesNavState.Profile(profileVM);
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), _profilesNavState);
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), winrt::make<ProfilePageNavigationState>(profileVM, _settingsClone.GlobalSettings().ColorSchemes(), *this));
}
else if (clickedItemTag == colorSchemesTag)
{
@ -287,10 +269,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void MainPage::_Navigate(const Editor::ProfileViewModel& profile)
{
// Update the profiles navigation state
_profilesNavState.Profile(profile);
auto state{ winrt::make<ProfilePageNavigationState>(profile, _settingsClone.GlobalSettings().ColorSchemes(), *this) };
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), _profilesNavState);
// Add an event handler for when the user wants to delete a profile.
state.DeleteProfile({ this, &MainPage::_DeleteProfile });
contentFrame().Navigate(xaml_typename<Editor::Profiles>(), state);
}
void MainPage::OpenJsonTapped(IInspectable const& /*sender*/, Windows::UI::Xaml::Input::TappedRoutedEventArgs const& /*args*/)

View file

@ -42,7 +42,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void _Navigate(const Editor::ProfileViewModel& profile);
ColorSchemesPageNavigationState _colorSchemesNavState{ nullptr };
ProfilePageNavigationState _profilesNavState{ nullptr };
};
}

View file

@ -185,9 +185,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
{
StartingDirectoryUseParentCheckbox().IsChecked(true);
}
// Navigate to the pivot in the provided navigation state
ProfilesPivot().SelectedIndex(static_cast<int>(_State.LastActivePivot()));
}
ColorScheme Profiles::CurrentColorScheme()
@ -369,10 +366,4 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
return _State.Profile().CursorShape() == TerminalControl::CursorStyle::Vintage;
}
void Profiles::Pivot_SelectionChanged(Windows::Foundation::IInspectable const& /*sender*/,
Windows::UI::Xaml::RoutedEventArgs const& /*e*/)
{
_State.LastActivePivot(static_cast<Editor::ProfilesPivots>(ProfilesPivot().SelectedIndex()));
}
}

View file

@ -85,9 +85,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
struct ProfilePageNavigationState : ProfilePageNavigationStateT<ProfilePageNavigationState>
{
public:
ProfilePageNavigationState(const Editor::ProfileViewModel& viewModel,
const Windows::Foundation::Collections::IMapView<hstring, Model::ColorScheme>& schemes,
const IHostedInWindow& windowRoot) :
ProfilePageNavigationState(const Editor::ProfileViewModel& viewModel, const Windows::Foundation::Collections::IMapView<hstring, Model::ColorScheme>& schemes, const IHostedInWindow& windowRoot) :
_Profile{ viewModel },
_Schemes{ schemes },
_WindowRoot{ windowRoot }
@ -101,27 +99,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
TYPED_EVENT(DeleteProfile, Editor::ProfilePageNavigationState, Editor::DeleteProfileEventArgs);
GETSET_PROPERTY(IHostedInWindow, WindowRoot, nullptr);
GETSET_PROPERTY(Editor::ProfilesPivots, LastActivePivot, Editor::ProfilesPivots::General);
public:
// Manually define Profile(), so we can overload the setter
Editor::ProfileViewModel Profile() const noexcept { return _Profile; }
void Profile(const Editor::ProfileViewModel& value) noexcept
{
// If the profile has a different guid than the new one, then reset
// the selected pivot to the "General" tab.
const auto& oldGuid = _Profile.Guid();
const auto& newGuid = value.Guid();
if (oldGuid != newGuid)
{
_LastActivePivot = Editor::ProfilesPivots::General;
}
_Profile = value;
}
GETSET_PROPERTY(Editor::ProfileViewModel, Profile, nullptr);
private:
Editor::ProfileViewModel _Profile{ nullptr };
Windows::Foundation::Collections::IMapView<hstring, Model::ColorScheme> _Schemes;
};
@ -143,7 +123,6 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
void DeleteConfirmation_Click(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::RoutedEventArgs const& e);
void UseParentProcessDirectory_Check(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::RoutedEventArgs const& e);
void UseParentProcessDirectory_Uncheck(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::RoutedEventArgs const& e);
void Pivot_SelectionChanged(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::RoutedEventArgs const& e);
// CursorShape visibility logic
void CursorShape_Changed(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::RoutedEventArgs const& e);

View file

@ -60,19 +60,12 @@ namespace Microsoft.Terminal.Settings.Editor
Guid ProfileGuid { get; };
}
enum ProfilesPivots {
General = 0,
Appearance = 1,
Advanced = 2
};
runtimeclass ProfilePageNavigationState
{
Windows.Foundation.Collections.IMapView<String, Microsoft.Terminal.Settings.Model.ColorScheme> Schemes;
IHostedInWindow WindowRoot; // necessary to send the right HWND into the file picker dialogs.
ProfileViewModel Profile;
ProfilesPivots LastActivePivot;
ProfileViewModel Profile { get; };
event Windows.Foundation.TypedEventHandler<ProfilePageNavigationState, DeleteProfileEventArgs> DeleteProfile;
};

View file

@ -49,10 +49,8 @@ the MIT License. See LICENSE in the project root for license information. -->
Style="{StaticResource DisclaimerStyle}"
Visibility="{x:Bind State.Profile.IsBaseLayer}"/>
<Pivot x:Name="ProfilesPivot"
HorizontalAlignment="Left"
<Pivot HorizontalAlignment="Left"
Grid.Row="1"
SelectionChanged="Pivot_SelectionChanged"
Margin="1,0,0,0">
<!-- General Tab -->
<PivotItem x:Uid="Profile_General">