From 358f886fabe3486d52533d891aa2d0105e4845b2 Mon Sep 17 00:00:00 2001 From: Andrey Zimin Date: Thu, 6 Jan 2022 21:38:32 +0000 Subject: [PATCH] Clamp selected_ on list resize for Radiobox/Menu/Toggle (#298) fix: https://github.com/ArthurSonzogni/FTXUI/issues/296#issue-1092343846 When the list in Radiobox/Menu/Toggle is resized, clamp the |selected_| values so that it stays within bounds. Clamping is executed in Render() and in OnEvent() Co-authored-by: ArthurSonzogni --- cmake/ftxui_test.cmake | 1 + src/ftxui/component/menu.cpp | 53 +++++++++++++++------------ src/ftxui/component/menu_test.cpp | 48 ++++++++++++++++++++++++ src/ftxui/component/radiobox.cpp | 51 +++++++++++++++----------- src/ftxui/component/radiobox_test.cpp | 37 +++++++++++++++++-- src/ftxui/component/toggle.cpp | 35 +++++++++++------- src/ftxui/component/toggle_test.cpp | 29 +++++++++++++++ 7 files changed, 192 insertions(+), 62 deletions(-) create mode 100644 src/ftxui/component/menu_test.cpp diff --git a/cmake/ftxui_test.cmake b/cmake/ftxui_test.cmake index 57f32cc..66cafa2 100644 --- a/cmake/ftxui_test.cmake +++ b/cmake/ftxui_test.cmake @@ -14,6 +14,7 @@ add_executable(tests src/ftxui/component/component_test.cpp src/ftxui/component/container_test.cpp src/ftxui/component/input_test.cpp + src/ftxui/component/menu_test.cpp src/ftxui/component/radiobox_test.cpp src/ftxui/component/receiver_test.cpp src/ftxui/component/screen_interactive_test.cpp diff --git a/src/ftxui/component/menu.cpp b/src/ftxui/component/menu.cpp index bd26f0e..c7095ec 100644 --- a/src/ftxui/component/menu.cpp +++ b/src/ftxui/component/menu.cpp @@ -1,21 +1,20 @@ -#include // for size_t -#include // for max, min +#include // for clamp, max #include // for function #include // for shared_ptr, allocator_traits<>::value_type #include // for operator+, string #include // for move -#include // for vector, __alloc_traits<>::value_type +#include // for vector -#include "ftxui/component/captured_mouse.hpp" // for CapturedMouse -#include "ftxui/component/component.hpp" // for Make, Menu -#include "ftxui/component/component_base.hpp" // for ComponentBase -#include "ftxui/component/component_options.hpp" // for MenuOption -#include "ftxui/component/event.hpp" // for Event, Event::ArrowDown, Event::ArrowUp, Event::Return, Event::Tab, Event::TabReverse -#include "ftxui/component/mouse.hpp" // for Mouse, Mouse::Left, Mouse::Released +#include "ftxui/component/captured_mouse.hpp" // for CapturedMouse +#include "ftxui/component/component.hpp" // for Make, Menu, MenuEntry +#include "ftxui/component/component_base.hpp" // for ComponentBase +#include "ftxui/component/component_options.hpp" // for MenuOption, MenuEntryOption +#include "ftxui/component/event.hpp" // for Event, Event::ArrowDown, Event::ArrowUp, Event::End, Event::Home, Event::PageDown, Event::PageUp, Event::Return, Event::Tab, Event::TabReverse +#include "ftxui/component/mouse.hpp" // for Mouse, Mouse::Left, Mouse::Released, Mouse::WheelDown, Mouse::WheelUp, Mouse::None #include "ftxui/component/screen_interactive.hpp" // for Component -#include "ftxui/dom/elements.hpp" // for operator|, Element, reflect, text, vbox, Elements, focus, nothing, select +#include "ftxui/dom/elements.hpp" // for operator|, Element, reflect, text, nothing, select, vbox, Elements, focus #include "ftxui/screen/box.hpp" // for Box -#include "ftxui/util/ref.hpp" // for Ref +#include "ftxui/util/ref.hpp" // for Ref, ConstStringListRef, ConstStringRef namespace ftxui { @@ -27,12 +26,12 @@ class MenuBase : public ComponentBase { : entries_(entries), selected_(selected), option_(option) {} Element Render() override { + Clamp(); Elements elements; bool is_menu_focused = Focused(); - boxes_.resize(entries_.size()); - for (size_t i = 0; i < entries_.size(); ++i) { - bool is_focused = (focused_entry() == int(i)) && is_menu_focused; - bool is_selected = (*selected_ == int(i)); + for (int i = 0; i < size(); ++i) { + bool is_focused = (focused_entry() == i) && is_menu_focused; + bool is_selected = (*selected_ == i); auto style = is_selected ? (is_focused ? option_->style_selected_focused : option_->style_selected) @@ -49,6 +48,7 @@ class MenuBase : public ComponentBase { } bool OnEvent(Event event) override { + Clamp(); if (!CaptureMouse(event)) return false; @@ -68,13 +68,13 @@ class MenuBase : public ComponentBase { if (event == Event::Home) (*selected_) = 0; if (event == Event::End) - (*selected_) = entries_.size() - 1; - if (event == Event::Tab && entries_.size()) - *selected_ = (*selected_ + 1) % entries_.size(); - if (event == Event::TabReverse && entries_.size()) - *selected_ = (*selected_ + entries_.size() - 1) % entries_.size(); + (*selected_) = size() - 1; + if (event == Event::Tab && size()) + *selected_ = (*selected_ + 1) % size(); + if (event == Event::TabReverse && size()) + *selected_ = (*selected_ + size() - 1) % size(); - *selected_ = std::max(0, std::min(int(entries_.size()) - 1, *selected_)); + *selected_ = std::clamp(*selected_, 0, size() - 1); if (*selected_ != old_selected) { focused_entry() = *selected_; @@ -103,7 +103,7 @@ class MenuBase : public ComponentBase { } if (!CaptureMouse(event)) return false; - for (int i = 0; i < int(boxes_.size()); ++i) { + for (int i = 0; i < size(); ++i) { if (!boxes_[i].Contain(event.mouse().x, event.mouse().y)) continue; @@ -131,15 +131,22 @@ class MenuBase : public ComponentBase { if (event.mouse().button == Mouse::WheelDown) (*selected_)++; - *selected_ = std::max(0, std::min(int(entries_.size()) - 1, *selected_)); + *selected_ = std::clamp(*selected_, 0, size() - 1); if (*selected_ != old_selected) option_->on_change(); return true; } + void Clamp() { + boxes_.resize(size()); + *selected_ = std::clamp(*selected_, 0, size() - 1); + focused_entry() = std::clamp(focused_entry(), 0, size() - 1); + } + bool Focusable() const final { return entries_.size(); } int& focused_entry() { return option_->focused_entry(); } + int size() const { return entries_.size(); } protected: ConstStringListRef entries_; diff --git a/src/ftxui/component/menu_test.cpp b/src/ftxui/component/menu_test.cpp new file mode 100644 index 0000000..f7088db --- /dev/null +++ b/src/ftxui/component/menu_test.cpp @@ -0,0 +1,48 @@ +#include // for Message +#include // for TestPartResult, SuiteApiResolver, TestFactoryImpl +#include // for allocator, __shared_ptr_access, shared_ptr +#include // for string, basic_string +#include // for vector + +#include "ftxui/component/captured_mouse.hpp" // for ftxui +#include "ftxui/component/component.hpp" // for Menu +#include "ftxui/component/component_base.hpp" // for ComponentBase +#include "ftxui/component/component_options.hpp" // for MenuOption +#include "ftxui/component/event.hpp" // for Event, Event::ArrowDown, Event::Return +#include "ftxui/util/ref.hpp" // for Ref +#include "gtest/gtest_pred_impl.h" // for Test, EXPECT_EQ, TEST + +using namespace ftxui; + +TEST(MenuTest, RemoveEntries) { + int focused_entry = 0; + int selected = 0; + std::vector entries = {"1", "2", "3"}; + MenuOption option; + option.focused_entry = &focused_entry; + auto menu = Menu(&entries, &selected, option); + + EXPECT_EQ(selected, 0); + EXPECT_EQ(focused_entry, 0); + + menu->OnEvent(Event::ArrowDown); + menu->OnEvent(Event::ArrowDown); + menu->OnEvent(Event::Return); + + EXPECT_EQ(selected, 2); + EXPECT_EQ(focused_entry, 2); + + entries.resize(2); + + EXPECT_EQ(selected, 2); + EXPECT_EQ(focused_entry, 2); + + (void)menu->Render(); + + EXPECT_EQ(selected, 1); + EXPECT_EQ(focused_entry, 1); +} + +// Copyright 2022 Arthur Sonzogni. All rights reserved. +// Use of this source code is governed by the MIT license that can be found in +// the LICENSE file. diff --git a/src/ftxui/component/radiobox.cpp b/src/ftxui/component/radiobox.cpp index cf43823..618d301 100644 --- a/src/ftxui/component/radiobox.cpp +++ b/src/ftxui/component/radiobox.cpp @@ -1,5 +1,4 @@ -#include // for size_t -#include // for max, min +#include // for clamp, max #include // for function #include // for shared_ptr, allocator_traits<>::value_type #include // for string @@ -10,12 +9,12 @@ #include "ftxui/component/component.hpp" // for Make, Radiobox #include "ftxui/component/component_base.hpp" // for ComponentBase #include "ftxui/component/component_options.hpp" // for RadioboxOption -#include "ftxui/component/event.hpp" // for Event, Event::ArrowDown, Event::ArrowUp, Event::Return, Event::Tab, Event::TabReverse -#include "ftxui/component/mouse.hpp" // for Mouse, Mouse::Left, Mouse::Pressed +#include "ftxui/component/event.hpp" // for Event, Event::ArrowDown, Event::ArrowUp, Event::End, Event::Home, Event::PageDown, Event::PageUp, Event::Return, Event::Tab, Event::TabReverse +#include "ftxui/component/mouse.hpp" // for Mouse, Mouse::WheelDown, Mouse::WheelUp, Mouse::Left, Mouse::Released #include "ftxui/component/screen_interactive.hpp" // for Component -#include "ftxui/dom/elements.hpp" // for Element, operator|, text, hbox, reflect, vbox, focus, nothing, select -#include "ftxui/screen/box.hpp" // for Box -#include "ftxui/util/ref.hpp" // for Ref +#include "ftxui/dom/elements.hpp" // for operator|, reflect, text, Element, hbox, vbox, Elements, focus, nothing, select +#include "ftxui/screen/box.hpp" // for Box +#include "ftxui/util/ref.hpp" // for Ref, ConstStringListRef namespace ftxui { @@ -42,12 +41,12 @@ class RadioboxBase : public ComponentBase { private: Element Render() override { + Clamp(); Elements elements; bool is_menu_focused = Focused(); - boxes_.resize(entries_.size()); - for (size_t i = 0; i < entries_.size(); ++i) { - bool is_focused = (focused_entry() == int(i)) && is_menu_focused; - bool is_selected = (hovered_ == int(i)); + for (int i = 0; i < size(); ++i) { + bool is_focused = (focused_entry() == i) && is_menu_focused; + bool is_selected = (hovered_ == i); auto style = is_selected ? (is_focused ? option_->style_selected_focused : option_->style_selected) @@ -57,9 +56,8 @@ class RadioboxBase : public ComponentBase { : is_menu_focused ? focus : select; - const std::string& symbol = *selected_ == int(i) - ? option_->style_checked - : option_->style_unchecked; + const std::string& symbol = + *selected_ == i ? option_->style_checked : option_->style_unchecked; elements.push_back(hbox(text(symbol), text(entries_[i]) | style) | focus_management | reflect(boxes_[i])); } @@ -67,6 +65,7 @@ class RadioboxBase : public ComponentBase { } bool OnEvent(Event event) override { + Clamp(); if (!CaptureMouse(event)) return false; @@ -86,13 +85,13 @@ class RadioboxBase : public ComponentBase { if (event == Event::Home) (hovered_) = 0; if (event == Event::End) - (hovered_) = entries_.size() - 1; - if (event == Event::Tab && entries_.size()) - hovered_ = (hovered_ + 1) % entries_.size(); - if (event == Event::TabReverse && entries_.size()) - hovered_ = (hovered_ + entries_.size() - 1) % entries_.size(); + (hovered_) = size() - 1; + if (event == Event::Tab && size()) + hovered_ = (hovered_ + 1) % size(); + if (event == Event::TabReverse && size()) + hovered_ = (hovered_ + size() - 1) % size(); - hovered_ = std::max(0, std::min(int(entries_.size()) - 1, hovered_)); + hovered_ = std::clamp(hovered_, 0, size() - 1); if (hovered_ != old_hovered) { focused_entry() = hovered_; @@ -116,7 +115,7 @@ class RadioboxBase : public ComponentBase { return OnMouseWheel(event); } - for (int i = 0; i < int(boxes_.size()); ++i) { + for (int i = 0; i < size(); ++i) { if (!boxes_[i].Contain(event.mouse().x, event.mouse().y)) continue; @@ -146,7 +145,7 @@ class RadioboxBase : public ComponentBase { if (event.mouse().button == Mouse::WheelDown) (hovered_)++; - hovered_ = std::max(0, std::min(int(entries_.size()) - 1, hovered_)); + hovered_ = std::clamp(hovered_, 0, size() - 1); if (hovered_ != old_hovered) option_->on_change(); @@ -154,8 +153,16 @@ class RadioboxBase : public ComponentBase { return true; } + void Clamp() { + boxes_.resize(size()); + *selected_ = std::clamp(*selected_, 0, size() - 1); + focused_entry() = std::clamp(focused_entry(), 0, size() - 1); + hovered_ = std::clamp(hovered_, 0, size() - 1); + } + bool Focusable() const final { return entries_.size(); } int& focused_entry() { return option_->focused_entry(); } + int size() const { return entries_.size(); } ConstStringListRef entries_; int* selected_; diff --git a/src/ftxui/component/radiobox_test.cpp b/src/ftxui/component/radiobox_test.cpp index fa89da3..08a4550 100644 --- a/src/ftxui/component/radiobox_test.cpp +++ b/src/ftxui/component/radiobox_test.cpp @@ -4,10 +4,12 @@ #include // for string, basic_string #include // for vector -#include "ftxui/component/captured_mouse.hpp" // for ftxui -#include "ftxui/component/component.hpp" // for Radiobox -#include "ftxui/component/component_base.hpp" // for ComponentBase +#include "ftxui/component/captured_mouse.hpp" // for ftxui +#include "ftxui/component/component.hpp" // for Radiobox +#include "ftxui/component/component_base.hpp" // for ComponentBase +#include "ftxui/component/component_options.hpp" // for RadioboxOption #include "ftxui/component/event.hpp" // for Event, Event::Return, Event::ArrowDown, Event::ArrowUp, Event::Tab, Event::TabReverse +#include "ftxui/util/ref.hpp" // for Ref #include "gtest/gtest_pred_impl.h" // for EXPECT_EQ, Test, TEST using namespace ftxui; @@ -114,6 +116,35 @@ TEST(RadioboxTest, Navigation) { radiobox->OnEvent(Event::Return); } +TEST(RadioboxTest, RemoveEntries) { + int focused_entry = 0; + int selected = 0; + std::vector entries = {"1", "2", "3"}; + RadioboxOption option; + option.focused_entry = &focused_entry; + auto radiobox = Radiobox(&entries, &selected, option); + + EXPECT_EQ(selected, 0); + EXPECT_EQ(focused_entry, 0); + + radiobox->OnEvent(Event::ArrowDown); + radiobox->OnEvent(Event::ArrowDown); + radiobox->OnEvent(Event::Return); + + EXPECT_EQ(selected, 2); + EXPECT_EQ(focused_entry, 2); + + entries.resize(2); + + EXPECT_EQ(selected, 2); + EXPECT_EQ(focused_entry, 2); + + (void)radiobox->Render(); + + EXPECT_EQ(selected, 1); + EXPECT_EQ(focused_entry, 1); +} + // Copyright 2020 Arthur Sonzogni. All rights reserved. // Use of this source code is governed by the MIT license that can be found in // the LICENSE file. diff --git a/src/ftxui/component/toggle.cpp b/src/ftxui/component/toggle.cpp index eae1bf7..bc143e9 100644 --- a/src/ftxui/component/toggle.cpp +++ b/src/ftxui/component/toggle.cpp @@ -1,5 +1,4 @@ -#include // for size_t -#include // for max, min +#include // for clamp, max #include // for function #include // for shared_ptr, allocator_traits<>::value_type #include // for move @@ -13,7 +12,7 @@ #include "ftxui/component/mouse.hpp" // for Mouse, Mouse::Left, Mouse::Pressed #include "ftxui/dom/elements.hpp" // for operator|, Element, Elements, hbox, reflect, separator, text, focus, nothing, select #include "ftxui/screen/box.hpp" // for Box -#include "ftxui/util/ref.hpp" // for ConstStringListRef, Ref +#include "ftxui/util/ref.hpp" // for Ref, ConstStringListRef namespace ftxui { @@ -30,16 +29,16 @@ class ToggleBase : public ComponentBase { private: Element Render() override { + Clamp(); Elements children; bool is_toggle_focused = Focused(); - boxes_.resize(entries_.size()); - for (size_t i = 0; i < entries_.size(); ++i) { + for (int i = 0; i < size(); ++i) { // Separator. if (i != 0) children.push_back(separator()); - bool is_focused = (focused_entry() == int(i)) && is_toggle_focused; - bool is_selected = (*selected_ == int(i)); + bool is_focused = (focused_entry() == i) && is_toggle_focused; + bool is_selected = (*selected_ == i); auto style = is_selected ? (is_focused ? option_->style_selected_focused : option_->style_selected) @@ -55,6 +54,7 @@ class ToggleBase : public ComponentBase { } bool OnEvent(Event event) override { + Clamp(); if (event.is_mouse()) return OnMouseEvent(event); @@ -63,12 +63,12 @@ class ToggleBase : public ComponentBase { (*selected_)--; if (event == Event::ArrowRight || event == Event::Character('l')) (*selected_)++; - if (event == Event::Tab && entries_.size()) - *selected_ = (*selected_ + 1) % entries_.size(); - if (event == Event::TabReverse && entries_.size()) - *selected_ = (*selected_ + entries_.size() - 1) % entries_.size(); + if (event == Event::Tab && size()) + *selected_ = (*selected_ + 1) % size(); + if (event == Event::TabReverse && size()) + *selected_ = (*selected_ + size() - 1) % size(); - *selected_ = std::max(0, std::min(int(entries_.size()) - 1, *selected_)); + *selected_ = std::clamp(*selected_, 0, size() - 1); if (old_selected != *selected_) { focused_entry() = *selected_; @@ -87,7 +87,7 @@ class ToggleBase : public ComponentBase { bool OnMouseEvent(Event event) { if (!CaptureMouse(event)) return false; - for (int i = 0; i < int(boxes_.size()); ++i) { + for (int i = 0; i < size(); ++i) { if (!boxes_[i].Contain(event.mouse().x, event.mouse().y)) continue; @@ -106,8 +106,15 @@ class ToggleBase : public ComponentBase { return false; } - bool Focusable() const final { return entries_.size(); } + void Clamp() { + boxes_.resize(size()); + *selected_ = std::clamp(*selected_, 0, size() - 1); + focused_entry() = std::clamp(focused_entry(), 0, size() - 1); + } + + bool Focusable() const final { return size(); } int& focused_entry() { return option_->focused_entry(); } + int size() const { return entries_.size(); } ConstStringListRef entries_; int* selected_ = 0; diff --git a/src/ftxui/component/toggle_test.cpp b/src/ftxui/component/toggle_test.cpp index 871a8c4..126869e 100644 --- a/src/ftxui/component/toggle_test.cpp +++ b/src/ftxui/component/toggle_test.cpp @@ -10,6 +10,7 @@ #include "ftxui/component/component_base.hpp" // for ComponentBase #include "ftxui/component/component_options.hpp" // for ToggleOption #include "ftxui/component/event.hpp" // for Event, Event::ArrowLeft, Event::ArrowRight, Event::Return, Event::Tab, Event::TabReverse +#include "ftxui/util/ref.hpp" // for Ref #include "gtest/gtest_pred_impl.h" // for AssertionResult, EXPECT_EQ, Test, EXPECT_TRUE, EXPECT_FALSE, TEST using namespace ftxui; @@ -150,6 +151,34 @@ TEST(ToggleTest, OnEnter) { EXPECT_EQ(counter, 7); } +TEST(ToggleTest, RemoveEntries) { + int focused_entry = 0; + int selected = 0; + std::vector entries = {"1", "2", "3"}; + ToggleOption option; + option.focused_entry = &focused_entry; + auto toggle = Toggle(&entries, &selected, option); + + EXPECT_EQ(selected, 0); + EXPECT_EQ(focused_entry, 0); + + toggle->OnEvent(Event::ArrowRight); + toggle->OnEvent(Event::ArrowRight); + + EXPECT_EQ(selected, 2); + EXPECT_EQ(focused_entry, 2); + + entries.resize(2); + + EXPECT_EQ(selected, 2); + EXPECT_EQ(focused_entry, 2); + + (void)toggle->Render(); + + EXPECT_EQ(selected, 1); + EXPECT_EQ(focused_entry, 1); +} + // Copyright 2020 Arthur Sonzogni. All rights reserved. // Use of this source code is governed by the MIT license that can be found in // the LICENSE file.