From f8eebb9237e216b5149b63df197de47560720eac Mon Sep 17 00:00:00 2001 From: Amadeusz Wieczorek Date: Thu, 27 Jun 2024 11:47:38 -0700 Subject: [PATCH] UI improvements for the Copilot rename feature (#74149) * simplify the rename UI * minor changes to textbox * restore the ComboBox * simplify the ComboBox * alignment, double click * partially implement the delay * implement the delay * fix layout, button highlight * fix strings * button highlight now indicates whether suggestions are obtained automatically * remove unused field * revert * add comments * revert unnecessary change * remove ICommand and use explicit, documented methods for fetching suggestions and toggling the behavior * simplify logic * indent * PR feedback * comments * fix cancellation token, add isDisposed flag * remove unused property * revert diagnostic values * address AccInsights issue * update accessible name to address a warning * fix warning * undo diagnostic change --- .../UI/Adornment/RenameFlyout.xaml.cs | 13 +- .../SmartRename/SmartRenameStatusControl.xaml | 4 +- .../SmartRenameUserInputComboBox.xaml | 100 +++---------- .../SmartRenameUserInputComboBox.xaml.cs | 91 ++---------- .../UI/SmartRename/SmartRenameViewModel.cs | 133 +++++++++++------- .../Lightup/ISmartRenameSessionWrapper.cs | 4 + 6 files changed, 121 insertions(+), 224 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/Adornment/RenameFlyout.xaml.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/Adornment/RenameFlyout.xaml.cs index 57033d4f56af2..4b35e3e3832f6 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/Adornment/RenameFlyout.xaml.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/Adornment/RenameFlyout.xaml.cs @@ -201,16 +201,11 @@ private void Adornment_KeyDown(object sender, KeyEventArgs e) case Key.Space: if (Keyboard.Modifiers == ModifierKeys.Control) { - e.Handled = true; - // If smart rename is available, trigger it. - if (_viewModel.SmartRenameViewModel is not null && _viewModel.SmartRenameViewModel.GetSuggestionsCommand.CanExecute(null)) + // If smart rename is available, trigger or toggle it. + if (_viewModel.SmartRenameViewModel is not null) { - // If smart rename can use automatic behavior, enable it - if (!_viewModel.SmartRenameViewModel.IsUsingDropdown) - { - _viewModel.SmartRenameViewModel.IsSuggestionsPanelCollapsed = !_viewModel.SmartRenameViewModel.IsSuggestionsPanelCollapsed; - } - _viewModel.SmartRenameViewModel.GetSuggestionsCommand.Execute(null); + _viewModel.SmartRenameViewModel.ToggleOrTriggerSuggestions(); + e.Handled = true; } } break; diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameStatusControl.xaml b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameStatusControl.xaml index 18180e168879e..5af646c9f22aa 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameStatusControl.xaml +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameStatusControl.xaml @@ -5,13 +5,15 @@ xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:vsfx="clr-namespace:Microsoft.VisualStudio.Shell;assembly=Microsoft.VisualStudio.Shell.15.0" + xmlns:vsui="clr-namespace:Microsoft.VisualStudio.PlatformUI;assembly=Microsoft.VisualStudio.Shell.15.0" xmlns:platformUI="clr-namespace:Microsoft.VisualStudio.PlatformUI;assembly=Microsoft.VisualStudio.Shell.15.0" mc:Ignorable="d" Margin="0 0 0 5"> - + @@ -68,9 +67,9 @@ - diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameUserInputComboBox.xaml.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameUserInputComboBox.xaml.cs index b1ce11fbb270d..e387f28946df5 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameUserInputComboBox.xaml.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameUserInputComboBox.xaml.cs @@ -16,15 +16,13 @@ namespace Microsoft.CodeAnalysis.InlineRename.UI.SmartRename; /// Interaction logic for SmartRenameUserInputComboBox.xaml /// [TemplatePart(Name = InnerTextBox, Type = typeof(TextBox))] -[TemplatePart(Name = DropDownPopup, Type = typeof(Popup))] internal sealed partial class SmartRenameUserInputComboBox : ComboBox, IRenameUserInput { private const string InnerTextBox = "PART_EditableTextBox"; - private const string DropDownPopup = "PART_Popup"; private readonly SmartRenameViewModel _smartRenameViewModel; + private readonly RenameFlyoutViewModel _baseViewModel; private readonly Lazy _innerTextBox; - private Popup? _dropDownPopup; internal SmartRenameUserInputComboBox(RenameFlyoutViewModel viewModel) { @@ -33,7 +31,7 @@ internal SmartRenameUserInputComboBox(RenameFlyoutViewModel viewModel) InitializeComponent(); DataContext = viewModel.SmartRenameViewModel; - + _baseViewModel = viewModel; _smartRenameViewModel = viewModel.SmartRenameViewModel; _innerTextBox = new Lazy(() => { @@ -95,27 +93,20 @@ void IRenameUserInput.Focus() this.Focus(); } - public override void OnApplyTemplate() + private void ComboBox_GotKeyboardFocus(object sender, KeyboardFocusChangedEventArgs e) { - base.OnApplyTemplate(); + // Handle the event to avoid stack overflow through passing execution back RenameFlyout.Adornment_GotKeyboardFocus + e.Handled = true; + } - _dropDownPopup = (Popup)GetTemplateChild(DropDownPopup)!; + private void SuggestionsPanelScrollViewer_MouseDoubleClick(object sender, MouseEventArgs e) + { + _baseViewModel.Submit(); } private void GetSuggestionsButtonClick(object sender, RoutedEventArgs e) { - if (_smartRenameViewModel.IsUsingResultPanel) - { - _smartRenameViewModel.IsSuggestionsPanelCollapsed = !_smartRenameViewModel.IsSuggestionsPanelCollapsed; - if (_smartRenameViewModel.IsSuggestionsPanelExpanded) - { - _smartRenameViewModel.GetSuggestionsCommand.Execute(null); - } - } - else - { - _smartRenameViewModel.GetSuggestionsCommand.Execute(null); - } + _smartRenameViewModel.ToggleOrTriggerSuggestions(); } private void ComboBox_Unloaded(object sender, RoutedEventArgs e) @@ -123,30 +114,6 @@ private void ComboBox_Unloaded(object sender, RoutedEventArgs e) _smartRenameViewModel.SuggestedNames.CollectionChanged -= SuggestedNames_CollectionChanged; } - private void ComboBox_GotKeyboardFocus(object sender, KeyboardFocusChangedEventArgs e) - { - e.Handled = true; - } - - private void ComboBox_LostKeyboardFocus(object sender, KeyboardFocusChangedEventArgs e) - { - Assumes.NotNull(_dropDownPopup); - _dropDownPopup.IsOpen = false; - } - - private void ComboBox_PreviewKeyUp(object sender, KeyEventArgs e) - { - if (!_smartRenameViewModel.IsUsingDropdown) - { - return; - } - if ((e.Key is Key.Up or Key.Down) && Items.Count > 0) - { - Assumes.NotNull(_dropDownPopup); - _dropDownPopup.IsOpen = true; - } - } - private void ComboBox_SelectionChanged(object sender, SelectionChangedEventArgs e) { if (e.AddedItems.Count > 0) @@ -156,44 +123,6 @@ private void ComboBox_SelectionChanged(object sender, SelectionChangedEventArgs } } - private void ItemsPresenter_PreviewMouseUp(object sender, MouseButtonEventArgs e) - { - Assumes.NotNull(_dropDownPopup); - _dropDownPopup.IsOpen = false; - } - - private void InnerTextBox_GotFocus(object sender, RoutedEventArgs e) - { - e.Handled = true; // Prevent selecting all the text - if (!_smartRenameViewModel.IsUsingDropdown) - { - return; - } - if (Items.Count > 0) - { - Assumes.NotNull(_dropDownPopup); - _dropDownPopup.IsOpen = true; - } - } - - private void InnerTextBox_LostFocus(object sender, RoutedEventArgs e) - { - Assumes.NotNull(_dropDownPopup); - _dropDownPopup.IsOpen = false; - } - - private void InnerTextBox_PreviewKeyDown(object sender, KeyEventArgs e) - { - Assumes.NotNull(_dropDownPopup); - if ((e.Key is Key.Escape or Key.Space or Key.Enter) - && _dropDownPopup.IsOpen) // Handle these keystrokes when dropdown is present - { - _dropDownPopup.IsOpen = false; - SelectAllText(); - e.Handled = true; - } - } - private void SuggestedNames_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { Focus(); diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs index ab077794afeca..b78afe146a442 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/UI/SmartRename/SmartRenameViewModel.cs @@ -5,6 +5,7 @@ using System; using System.Collections.ObjectModel; using System.ComponentModel; +using System.Linq; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -30,7 +31,8 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi private readonly IThreadingContext _threadingContext; private readonly IAsynchronousOperationListenerProvider _listenerProvider; private CancellationTokenSource? _cancellationTokenSource; - + private bool _isDisposed; + private TimeSpan AutomaticFetchDelay => _smartRenameSession.AutomaticFetchDelay; private Task _getSuggestionsTask = Task.CompletedTask; public event PropertyChangedEventHandler? PropertyChanged; @@ -48,8 +50,20 @@ internal sealed partial class SmartRenameViewModel : INotifyPropertyChanged, IDi public string StatusMessage => _smartRenameSession.StatusMessage; public bool StatusMessageVisibility => _smartRenameSession.StatusMessageVisibility; - public bool IsUsingResultPanel { get; set; } - public bool IsUsingDropdown { get; set; } + + /// + /// Determines whether smart rename is in automatic mode (if true) or explicit mode (if false). + /// The mode is assigned based on feature flag / options. + /// + public bool SupportsAutomaticSuggestions { get; } + + /// + /// When smart rename is in automatic mode and is set, + /// developer gets to control whether the requests are made automatically on initialization. + /// Developer can toggle this option using the keyboard shortcut or button click, + /// both of which are handled in ."/> + /// + public bool IsAutomaticSuggestionsEnabled { get; private set; } private string? _selectedSuggestedName; @@ -70,39 +84,20 @@ public string? SelectedSuggestedName } } - public bool IsSuggestionsPanelCollapsed - { - get => IsUsingDropdown || _globalOptionService.GetOption(InlineRenameUIOptionsStorage.CollapseSuggestionsPanel); - set - { - if (value != IsSuggestionsPanelCollapsed) - { - _globalOptionService.SetGlobalOption(InlineRenameUIOptionsStorage.CollapseSuggestionsPanel, value); - NotifyPropertyChanged(nameof(IsSuggestionsPanelCollapsed)); - NotifyPropertyChanged(nameof(IsSuggestionsPanelExpanded)); - } - } - } - - public bool IsSuggestionsPanelExpanded - { - get => IsUsingResultPanel && !IsSuggestionsPanelCollapsed; - } + public bool IsSuggestionsPanelExpanded => HasSuggestions; public string GetSuggestionsTooltip - => IsUsingDropdown - ? EditorFeaturesWpfResources.Get_AI_suggestions - : EditorFeaturesWpfResources.Toggle_AI_suggestions; + => SupportsAutomaticSuggestions + ? EditorFeaturesWpfResources.Toggle_AI_suggestions + : EditorFeaturesWpfResources.Get_AI_suggestions; public string SubmitTextOverride - => IsUsingDropdown - ? EditorFeaturesWpfResources.Enter_to_rename_shift_enter_to_preview_ctrl_space_for_ai_suggestion - : EditorFeaturesWpfResources.Enter_to_rename_shift_enter_to_preview; + => SupportsAutomaticSuggestions + ? EditorFeaturesWpfResources.Enter_to_rename_shift_enter_to_preview + : EditorFeaturesWpfResources.Enter_to_rename_shift_enter_to_preview_ctrl_space_for_ai_suggestion; public static string GeneratingSuggestionsLabel => EditorFeaturesWpfResources.Generating_suggestions; - public ICommand GetSuggestionsCommand { get; } - public SmartRenameViewModel( IGlobalOptionService globalOptionService, IThreadingContext threadingContext, @@ -122,40 +117,53 @@ public SmartRenameViewModel( BaseViewModel.PropertyChanged += IdentifierTextPropertyChanged; this.BaseViewModel.IdentifierText = baseViewModel.IdentifierText; - GetSuggestionsCommand = new DelegateCommand(OnGetSuggestionsCommandExecute, null, threadingContext.JoinableTaskFactory); - - var getSuggestionsAutomatically = _globalOptionService.GetOption(InlineRenameUIOptionsStorage.GetSuggestionsAutomatically); - IsUsingResultPanel = getSuggestionsAutomatically; - IsUsingDropdown = !IsUsingResultPanel; SetupTelemetry(); - if (IsUsingResultPanel && IsSuggestionsPanelExpanded) + + this.SupportsAutomaticSuggestions = _globalOptionService.GetOption(InlineRenameUIOptionsStorage.GetSuggestionsAutomatically); + // Use existing "CollapseSuggestionsPanel" option (true if user does not wish to get suggestions automatically) to honor user's choice. + this.IsAutomaticSuggestionsEnabled = this.SupportsAutomaticSuggestions && !_globalOptionService.GetOption(InlineRenameUIOptionsStorage.CollapseSuggestionsPanel); + if (this.IsAutomaticSuggestionsEnabled) { - OnGetSuggestionsCommandExecute(); + this.FetchSuggestions(isAutomaticOnInitialization: true); } } - private void OnGetSuggestionsCommandExecute() + private void FetchSuggestions(bool isAutomaticOnInitialization) { _threadingContext.ThrowIfNotOnUIThread(); - if (IsUsingResultPanel && SuggestedNames.Count > 0) + if (this.SuggestedNames.Count > 0 || _isDisposed) { - // Don't get suggestions again in the automatic scenario + // Don't get suggestions again return; } + if (_getSuggestionsTask.Status is TaskStatus.RanToCompletion or TaskStatus.Faulted or TaskStatus.Canceled) { var listener = _listenerProvider.GetListener(FeatureAttribute.SmartRename); var listenerToken = listener.BeginAsyncOperation(nameof(_smartRenameSession.GetSuggestionsAsync)); - if (IsUsingDropdown && _suggestionsDropdownTelemetry is not null) - { - _suggestionsDropdownTelemetry.DropdownButtonClickTimes += 1; - } _cancellationTokenSource?.Dispose(); _cancellationTokenSource = new CancellationTokenSource(); - _getSuggestionsTask = _smartRenameSession.GetSuggestionsAsync(_cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); + _getSuggestionsTask = GetSuggestionsTaskAsync(isAutomaticOnInitialization, _cancellationTokenSource.Token).CompletesAsyncOperation(listenerToken); } } + private async Task GetSuggestionsTaskAsync(bool isAutomaticOnInitialization, CancellationToken cancellationToken) + { + if (isAutomaticOnInitialization) + { + // ConfigureAwait(true) to stay on the UI thread; + // WPF view is bound to _smartRenameSession properties and so they must be updated on the UI thread. + await Task.Delay(_smartRenameSession.AutomaticFetchDelay, cancellationToken).ConfigureAwait(true); + } + + if (cancellationToken.IsCancellationRequested || _isDisposed) + { + return; + } + _ = await _smartRenameSession.GetSuggestionsAsync(cancellationToken).ConfigureAwait(true); + return; + } + private void SessionPropertyChanged(object sender, PropertyChangedEventArgs e) { _threadingContext.ThrowIfNotOnUIThread(); @@ -165,20 +173,16 @@ private void SessionPropertyChanged(object sender, PropertyChangedEventArgs e) var textInputBackup = BaseViewModel.IdentifierText; SuggestedNames.Clear(); - var count = 0; - foreach (var name in _smartRenameSession.SuggestedNames) + // Set limit of 3 results + foreach (var name in _smartRenameSession.SuggestedNames.Take(3)) { - if (++count > 3 && IsUsingResultPanel) - { - // Set limit of 3 results when using the result panel - break; - } SuggestedNames.Add(name); } // Changing the list may have changed the text in the text box. We need to restore it. BaseViewModel.IdentifierText = textInputBackup; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(IsSuggestionsPanelExpanded))); return; } @@ -220,12 +224,39 @@ public void Commit(string finalIdentifierName) public void Dispose() { + _isDisposed = true; _smartRenameSession.PropertyChanged -= SessionPropertyChanged; BaseViewModel.PropertyChanged -= IdentifierTextPropertyChanged; _smartRenameSession.Dispose(); + _cancellationTokenSource?.Cancel(); _cancellationTokenSource?.Dispose(); } + /// + /// When smart rename operates in explicit mode, this method gets the suggestions. + /// When smart rename operates in automatic mode, this method toggles the automatic suggestions, + /// and gets the suggestions if it was just enabled. + /// + public void ToggleOrTriggerSuggestions() + { + if (this.SupportsAutomaticSuggestions) + { + this.IsAutomaticSuggestionsEnabled = !this.IsAutomaticSuggestionsEnabled; + if (this.IsAutomaticSuggestionsEnabled) + { + this.FetchSuggestions(isAutomaticOnInitialization: false); + } + + NotifyPropertyChanged(nameof(IsAutomaticSuggestionsEnabled)); + // Use existing "CollapseSuggestionsPanel" option (true if user does not wish to get suggestions automatically) to honor user's choice. + _globalOptionService.SetGlobalOption(InlineRenameUIOptionsStorage.CollapseSuggestionsPanel, !IsAutomaticSuggestionsEnabled); + } + else + { + this.FetchSuggestions(isAutomaticOnInitialization: false); + } + } + private void NotifyPropertyChanged([CallerMemberName] string? name = null) => PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(name)); diff --git a/src/EditorFeatures/Core.Wpf/Lightup/ISmartRenameSessionWrapper.cs b/src/EditorFeatures/Core.Wpf/Lightup/ISmartRenameSessionWrapper.cs index 78e2e0dda84c8..50975ec4c9aff 100644 --- a/src/EditorFeatures/Core.Wpf/Lightup/ISmartRenameSessionWrapper.cs +++ b/src/EditorFeatures/Core.Wpf/Lightup/ISmartRenameSessionWrapper.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.VisualStudio.Text.Editor; +using Microsoft.VisualStudio.Text.Editor.SmartRename; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.EditorFeatures.Lightup; @@ -19,6 +20,7 @@ namespace Microsoft.CodeAnalysis.EditorFeatures.Lightup; internal const string WrappedTypeName = "Microsoft.VisualStudio.Text.Editor.SmartRename.ISmartRenameSession"; private static readonly Type s_wrappedType; + private static readonly Func s_automaticFetchDelayAccessor; private static readonly Func s_isAvailableAccessor; private static readonly Func s_hasSuggestionsAccessor; private static readonly Func s_isInProgressAccessor; @@ -36,6 +38,7 @@ static ISmartRenameSessionWrapper() { s_wrappedType = typeof(AggregateFocusInterceptor).Assembly.GetType(WrappedTypeName, throwOnError: false, ignoreCase: false); + s_automaticFetchDelayAccessor = LightupHelpers.CreatePropertyAccessor(s_wrappedType, nameof(AutomaticFetchDelay), TimeSpan.Zero); s_isAvailableAccessor = LightupHelpers.CreatePropertyAccessor(s_wrappedType, nameof(IsAvailable), false); s_hasSuggestionsAccessor = LightupHelpers.CreatePropertyAccessor(s_wrappedType, nameof(HasSuggestions), false); s_isInProgressAccessor = LightupHelpers.CreatePropertyAccessor(s_wrappedType, nameof(IsInProgress), false); @@ -53,6 +56,7 @@ private ISmartRenameSessionWrapper(object instance) this._instance = instance; } + public TimeSpan AutomaticFetchDelay => s_automaticFetchDelayAccessor(_instance); public bool IsAvailable => s_isAvailableAccessor(_instance); public bool HasSuggestions => s_hasSuggestionsAccessor(_instance); public bool IsInProgress => s_isInProgressAccessor(_instance);