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 <sonzogniarthur@gmail.com>

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

src/ftxui/component/menu.cpp

@@ -1,21 +1,20 @@
-#include <stddef.h>    // for size_t
-#include <algorithm>   // for max, min
+#include <algorithm>   // for clamp, max
 #include <functional>  // for function
 #include <memory>      // for shared_ptr, allocator_traits<>::value_type
 #include <string>      // for operator+, string
 #include <utility>     // for move
-#include <vector>      // for vector, __alloc_traits<>::value_type
+#include <vector>      // 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_;

src/ftxui/component/menu_test.cpp

@@ -0,0 +1,48 @@
+#include <gtest/gtest-message.h>  // for Message
+#include <gtest/gtest-test-part.h>  // for TestPartResult, SuiteApiResolver, TestFactoryImpl
+#include <memory>  // for allocator, __shared_ptr_access, shared_ptr
+#include <string>  // for string, basic_string
+#include <vector>  // 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<std::string> 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.

src/ftxui/component/radiobox.cpp

@@ -1,5 +1,4 @@
-#include <stddef.h>    // for size_t
-#include <algorithm>   // for max, min
+#include <algorithm>   // for clamp, max
 #include <functional>  // for function
 #include <memory>      // for shared_ptr, allocator_traits<>::value_type
 #include <string>      // 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_;

src/ftxui/component/radiobox_test.cpp

@@ -4,10 +4,12 @@
 #include <string>  // for string, basic_string
 #include <vector>  // 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<std::string> 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.

src/ftxui/component/toggle.cpp

@@ -1,5 +1,4 @@
-#include <stddef.h>    // for size_t
-#include <algorithm>   // for max, min
+#include <algorithm>   // for clamp, max
 #include <functional>  // for function
 #include <memory>      // for shared_ptr, allocator_traits<>::value_type
 #include <utility>     // 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;

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<std::string> 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.