From 1de142b4b16b8e048b0bb82d8a6cd10c2c18b8ea Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 23 Aug 2024 12:12:00 -0500 Subject: [PATCH] Dismiss the snippet preview when there is no suggestions (#17777) Pretty obvious in retrospect. If there's no results, then we need to preview _nothing_ to make sure that we clear out any old previews. Closes #17773 Additionally, while I was here: I realized why it seems like the selected item is so wacky when you first open the sxnui: * on launch we're not scrolling to the bottom item (which makes it awkward in bottom up mode) * when we filter the list, we're maintaining the selection _index_, not the selection _item_. Alas, that second part is... shockingly bodgy. --- .../TerminalApp/SuggestionsControl.cpp | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalApp/SuggestionsControl.cpp b/src/cascadia/TerminalApp/SuggestionsControl.cpp index e6f482ac19..80206ea5dd 100644 --- a/src/cascadia/TerminalApp/SuggestionsControl.cpp +++ b/src/cascadia/TerminalApp/SuggestionsControl.cpp @@ -269,6 +269,8 @@ namespace winrt::TerminalApp::implementation const auto selectedCommand = _filteredActionsView().SelectedItem(); const auto filteredCommand{ selectedCommand.try_as() }; + _filteredActionsView().ScrollIntoView(selectedCommand); + PropertyChanged.raise(*this, Windows::UI::Xaml::Data::PropertyChangedEventArgs{ L"SelectedItem" }); // Make sure to not send the preview if we're collapsed. This can @@ -795,14 +797,46 @@ namespace winrt::TerminalApp::implementation // here will ensure that we can check this case appropriately. _lastFilterTextWasEmpty = _searchBox().Text().empty(); - const auto lastSelectedIndex = _filteredActionsView().SelectedIndex(); + const auto lastSelectedIndex = std::max(0, _filteredActionsView().SelectedIndex()); // SelectedIndex will return -1 for "nothing" _updateFilteredActions(); - // In the command line mode we want the user to explicitly select the command - _filteredActionsView().SelectedIndex(std::min(lastSelectedIndex, _filteredActionsView().Items().Size() - 1)); + if (const auto newSelectedIndex = _filteredActionsView().SelectedIndex(); + newSelectedIndex == -1) + { + // Make sure something stays selected + _scrollToIndex(lastSelectedIndex); + } + else + { + // BODGY: Calling ScrollIntoView on a ListView doesn't always work + // immediately after a change to the items. See: + // https://stackoverflow.com/questions/16942580/why-doesnt-listview-scrollintoview-ever-work + // The SelectionChanged thing we do (in _selectedCommandChanged), + // but because we're also not changing the actual selected item when + // the size of the list grows (it _stays_ selected, so it never + // _changes_), we never get a SelectionChanged. + // + // To mitigate, only in the case of totally clearing out the filter + // (like hitting `esc`), we want to briefly select the 0th item, + // then immediately select the one we want to make visible. That + // will make sure we get a SelectionChanged when the ListView is + // ready, and we can use that to scroll to the right item. + // + // If we do this on _every_ change, then the preview text flickers + // between the 0th item and the correct one. + if (_lastFilterTextWasEmpty) + { + _filteredActionsView().SelectedIndex(0); + } + _scrollToIndex(newSelectedIndex); + } const auto currentNeedleHasResults{ _filteredActions.Size() > 0 }; + if (!currentNeedleHasResults) + { + PreviewAction.raise(*this, nullptr); + } _noMatchesText().Visibility(currentNeedleHasResults ? Visibility::Collapsed : Visibility::Visible); if (auto automationPeer{ Automation::Peers::FrameworkElementAutomationPeer::FromElement(_searchBox()) }) { @@ -1203,7 +1237,7 @@ namespace winrt::TerminalApp::implementation if (_direction == TerminalApp::SuggestionsDirection::BottomUp) { const auto last = _filteredActionsView().Items().Size() - 1; - _filteredActionsView().SelectedIndex(last); + _scrollToIndex(last); } // Move the cursor to the very last position, so it starts immediately // after the text. This is apparently done by starting a 0-wide