From 744631a29344717feb26188e10b2c2a5ad515464 Mon Sep 17 00:00:00 2001 From: Don-Vito Date: Mon, 9 Nov 2020 23:55:11 +0200 Subject: [PATCH] Teach the command palette to clamp its indices on page up/down (#8190) This commit will teach CommandPalette to clamp the scroll page up and scroll page down navigation so as to not wrap. Closes #8189 (cherry picked from commit 624d07f2837712153964360bcf1fa3bd15a8b77e) --- src/cascadia/TerminalApp/CommandPalette.cpp | 101 ++++++++++++++------ src/cascadia/TerminalApp/CommandPalette.h | 10 +- 2 files changed, 78 insertions(+), 33 deletions(-) diff --git a/src/cascadia/TerminalApp/CommandPalette.cpp b/src/cascadia/TerminalApp/CommandPalette.cpp index 5c2748e0192..bb6658af0d5 100644 --- a/src/cascadia/TerminalApp/CommandPalette.cpp +++ b/src/cascadia/TerminalApp/CommandPalette.cpp @@ -120,48 +120,87 @@ namespace winrt::TerminalApp::implementation } // Method Description: - // - Scrolls the focus up or down the list of commands. + // - Scroll the command palette to the specified index // Arguments: - // - pageDown: if true, we're attempting to move to last visible item in the - // list. Otherwise, we're attempting to move to first visible item. + // - index within a list view of commands // Return Value: // - - void CommandPalette::ScrollDown(const bool pageDown) + void CommandPalette::_scrollToIndex(uint32_t index) + { + auto numItems = _filteredActionsView().Items().Size(); + + if (numItems == 0) + { + // if the list is empty no need to scroll + return; + } + + auto clampedIndex = std::clamp(index, 0, numItems - 1); + _filteredActionsView().SelectedIndex(clampedIndex); + _filteredActionsView().ScrollIntoView(_filteredActionsView().SelectedItem()); + } + + // Method Description: + // - Computes the number of visible commands + // Arguments: + // - + // Return Value: + // - the approximate number of items visible in the list (in other words the size of the page) + uint32_t CommandPalette::_getNumVisibleItems() { const auto container = _filteredActionsView().ContainerFromIndex(0); const auto item = container.try_as(); const auto itemHeight = ::base::saturated_cast(item.ActualHeight()); const auto listHeight = ::base::saturated_cast(_filteredActionsView().ActualHeight()); - const int numVisibleItems = listHeight / itemHeight; + return listHeight / itemHeight; + } + // Method Description: + // - Scrolls the focus one page up the list of commands. + // Arguments: + // - + // Return Value: + // - + void CommandPalette::ScrollPageUp() + { auto selected = _filteredActionsView().SelectedIndex(); - const int numItems = ::base::saturated_cast(_filteredActionsView().Items().Size()); + auto numVisibleItems = _getNumVisibleItems(); + _scrollToIndex(selected - numVisibleItems); + } - const auto newIndex = ((numItems + selected + (pageDown ? numVisibleItems : -numVisibleItems)) % numItems); - _filteredActionsView().SelectedIndex(newIndex); - _filteredActionsView().ScrollIntoView(_filteredActionsView().SelectedItem()); + // Method Description: + // - Scrolls the focus one page down the list of commands. + // Arguments: + // - + // Return Value: + // - + void CommandPalette::ScrollPageDown() + { + auto selected = _filteredActionsView().SelectedIndex(); + auto numVisibleItems = _getNumVisibleItems(); + _scrollToIndex(selected + numVisibleItems); } // Method Description: - // - Moves the focus either to top item or end item in the list of commands. + // - Moves the focus to the top item in the list of commands. // Arguments: - // - end: if true, we're attempting to move to last item in the - // list. Otherwise, we're attempting to move to first item. - // Depends on the pageUpDown argument. + // - // Return Value: // - - void CommandPalette::GoEnd(const bool end) + void CommandPalette::ScrollToTop() { - const auto lastIndex = ::base::saturated_cast(_filteredActionsView().Items().Size() - 1); - if (end) - { - _filteredActionsView().SelectedIndex(lastIndex); - } - else - { - _filteredActionsView().SelectedIndex(0); - } - _filteredActionsView().ScrollIntoView(_filteredActionsView().SelectedItem()); + _scrollToIndex(0); + } + + // Method Description: + // - Moves the focus to the bottom item in the list of commands. + // Arguments: + // - + // Return Value: + // - + void CommandPalette::ScrollToBottom() + { + _scrollToIndex(_filteredActionsView().Items().Size() - 1); } // Method Description: @@ -242,24 +281,26 @@ namespace winrt::TerminalApp::implementation } else if (key == VirtualKey::PageUp) { - // Action Mode: Move focus to the previous item in the list. - ScrollDown(false); + // Action Mode: Move focus to the first visible item in the list. + ScrollPageUp(); e.Handled(true); } else if (key == VirtualKey::PageDown) { - // Action Mode: Move focus to the previous item in the list. - ScrollDown(true); + // Action Mode: Move focus to the last visible item in the list. + ScrollPageDown(); e.Handled(true); } else if (key == VirtualKey::Home) { - GoEnd(false); + // Action Mode: Move focus to the first item in the list. + ScrollToTop(); e.Handled(true); } else if (key == VirtualKey::End) { - GoEnd(true); + // Action Mode: Move focus to the last item in the list. + ScrollToBottom(); e.Handled(true); } else if (key == VirtualKey::Enter) diff --git a/src/cascadia/TerminalApp/CommandPalette.h b/src/cascadia/TerminalApp/CommandPalette.h index 94f52034ecf..34f025e6f36 100644 --- a/src/cascadia/TerminalApp/CommandPalette.h +++ b/src/cascadia/TerminalApp/CommandPalette.h @@ -34,9 +34,10 @@ namespace winrt::TerminalApp::implementation void SelectNextItem(const bool moveDown); - void ScrollDown(const bool pageDown); - - void GoEnd(const bool end); + void ScrollPageUp(); + void ScrollPageDown(); + void ScrollToTop(); + void ScrollToBottom(); // Tab Switcher void EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx); @@ -103,6 +104,9 @@ namespace winrt::TerminalApp::implementation void _dispatchCommand(const TerminalApp::Command& command); void _dispatchCommandline(); void _dismissPalette(); + + void _scrollToIndex(uint32_t index); + uint32_t _getNumVisibleItems(); }; }