Merge baa5973128 into 1d40687a40

CHANGELOG.md

@@ -39,8 +39,6 @@ current (development)
   component in its parent. See #932
 - Feature: Add `EntryState::index`. This allows to get the index of a menu entry.
   See #932
-- Feature: Add `SliderOption::on_change`. This allows to set a callback when the
-  slider value changes. See #938.
 
 ### Dom
 - Feature: Add `hscroll_indicator`. It display an horizontal indicator

README.md

@@ -336,7 +336,6 @@ Feel free to add your projects here:
 - [Step-Writer](https://github.com/BrianAnakPintar/step-writer)
 - [XJ music](https://github.com/xjmusic/xjmusic)
 - [UDP chat](https://github.com/Sergeydigl3/udp-chat-tui)
-- [2048-cpp](https://github.com/Chessom/2048-cpp)
 
 ### [cpp-best-practices/game_jam](https://github.com/cpp-best-practices/game_jam)
 

include/ftxui/component/component_options.hpp

@@ -228,7 +228,6 @@ struct SliderOption {
   Direction direction = Direction::Right;
   Color color_active = Color::White;
   Color color_inactive = Color::GrayDark;
-  std::function<void()> on_change;  ///> Called when `value` is updated.
 };
 
 // Parameter pack used by `WindowOptions::render`.

src/ftxui/component/slider.cpp

@@ -35,26 +35,31 @@ Decorator flexDirection(Direction direction) {
 }
 
 template <class T>
-class SliderBase : public SliderOption<T>, public ComponentBase {
+class SliderBase : public ComponentBase {
  public:
-  explicit SliderBase(SliderOption<T> options) : SliderOption<T>(options) {}
+  explicit SliderBase(SliderOption<T> options)
+      : value_(options.value),
+        min_(options.min),
+        max_(options.max),
+        increment_(options.increment),
+        options_(options) {}
 
   Element Render() override {
-    auto gauge_color =
+    auto gauge_color = Focused() ? color(options_.color_active)
-        Focused() ? color(this->color_active) : color(this->color_inactive);
+                                 : color(options_.color_inactive);
-    const float percent =
+    const float percent = float(value_() - min_()) / float(max_() - min_());
-        float(this->value() - this->min()) / float(this->max() - this->min());
+    return gaugeDirection(percent, options_.direction) |
-    return gaugeDirection(percent, this->direction) |
+           flexDirection(options_.direction) | reflect(gauge_box_) |
-           flexDirection(this->direction) | reflect(gauge_box_) | gauge_color;
+           gauge_color;
   }
 
   void OnLeft() {
-    switch (this->direction) {
+    switch (options_.direction) {
       case Direction::Right:
-        this->value() -= this->increment();
+        value_() -= increment_();
         break;
       case Direction::Left:
-        this->value() += this->increment();
+        value_() += increment_();
         break;
       case Direction::Up:
       case Direction::Down:
@@ -63,12 +68,12 @@ class SliderBase : public SliderOption<T>, public ComponentBase {
   }
 
   void OnRight() {
-    switch (this->direction) {
+    switch (options_.direction) {
       case Direction::Right:
-        this->value() += this->increment();
+        value_() += increment_();
         break;
       case Direction::Left:
-        this->value() -= this->increment();
+        value_() -= increment_();
         break;
       case Direction::Up:
       case Direction::Down:
@@ -77,12 +82,12 @@ class SliderBase : public SliderOption<T>, public ComponentBase {
   }
 
   void OnUp() {
-    switch (this->direction) {
+    switch (options_.direction) {
       case Direction::Up:
-        this->value() -= this->increment();
+        value_() -= increment_();
         break;
       case Direction::Down:
-        this->value() += this->increment();
+        value_() += increment_();
         break;
       case Direction::Left:
       case Direction::Right:
@@ -91,12 +96,12 @@ class SliderBase : public SliderOption<T>, public ComponentBase {
   }
 
   void OnDown() {
-    switch (this->direction) {
+    switch (options_.direction) {
       case Direction::Down:
-        this->value() += this->increment();
+        value_() -= increment_();
         break;
       case Direction::Up:
-        this->value() -= this->increment();
+        value_() += increment_();
         break;
       case Direction::Left:
       case Direction::Right:
@@ -109,7 +114,7 @@ class SliderBase : public SliderOption<T>, public ComponentBase {
       return OnMouseEvent(event);
     }
 
-    T old_value = this->value();
+    T old_value = value_();
     if (event == Event::ArrowLeft || event == Event::Character('h')) {
       OnLeft();
     }
@@ -123,11 +128,8 @@ class SliderBase : public SliderOption<T>, public ComponentBase {
       OnUp();
     }
 
-    this->value() = std::max(this->min(), std::min(this->max(), this->value()));
+    value_() = util::clamp(value_(), min_(), max_());
-    if (old_value != this->value()) {
+    if (old_value != value_()) {
-      if (this->on_change) {
-        this->on_change();
-      }
       return true;
     }
 
@@ -141,45 +143,33 @@ class SliderBase : public SliderOption<T>, public ComponentBase {
         return true;
       }
 
-      T old_value = this->value();
+      switch (options_.direction) {
-      switch (this->direction) {
         case Direction::Right: {
-          this->value() =
+          value_() = min_() + (event.mouse().x - gauge_box_.x_min) *
-              this->min() + (event.mouse().x - gauge_box_.x_min) *
+                                  (max_() - min_()) /
-                                (this->max() - this->min()) /
                                   (gauge_box_.x_max - gauge_box_.x_min);
-
           break;
         }
         case Direction::Left: {
-          this->value() =
+          value_() = max_() - (event.mouse().x - gauge_box_.x_min) *
-              this->max() - (event.mouse().x - gauge_box_.x_min) *
+                                  (max_() - min_()) /
-                                (this->max() - this->min()) /
                                   (gauge_box_.x_max - gauge_box_.x_min);
           break;
         }
         case Direction::Down: {
-          this->value() =
+          value_() = min_() + (event.mouse().y - gauge_box_.y_min) *
-              this->min() + (event.mouse().y - gauge_box_.y_min) *
+                                  (max_() - min_()) /
-                                (this->max() - this->min()) /
                                   (gauge_box_.y_max - gauge_box_.y_min);
           break;
         }
         case Direction::Up: {
-          this->value() =
+          value_() = max_() - (event.mouse().y - gauge_box_.y_min) *
-              this->max() - (event.mouse().y - gauge_box_.y_min) *
+                                  (max_() - min_()) /
-                                (this->max() - this->min()) /
                                   (gauge_box_.y_max - gauge_box_.y_min);
           break;
         }
       }
-
+      value_() = std::max(min_(), std::min(max_(), value_()));
-      this->value() =
-          std::max(this->min(), std::min(this->max(), this->value()));
-
-      if (old_value != this->value() && this->on_change) {
-        this->on_change();
-      }
       return true;
     }
 
@@ -207,6 +197,11 @@ class SliderBase : public SliderOption<T>, public ComponentBase {
   bool Focusable() const final { return true; }
 
  private:
+  Ref<T> value_;
+  ConstRef<T> min_;
+  ConstRef<T> max_;
+  ConstRef<T> increment_;
+  SliderOption<T> options_;
   Box gauge_box_;
   CapturedMouse captured_mouse_;
 };
@@ -261,7 +256,6 @@ class SliderWithLabel : public ComponentBase {
   Box box_;
   bool mouse_hover_ = false;
 };
-
 }  // namespace
 
 /// @brief An horizontal slider.
@@ -346,7 +340,6 @@ template <typename T>
 Component Slider(SliderOption<T> options) {
   return Make<SliderBase<T>>(options);
 }
-
 template Component Slider(SliderOption<int8_t>);
 template Component Slider(SliderOption<int16_t>);
 template Component Slider(SliderOption<int32_t>);

src/ftxui/component/slider_test.cpp

@@ -45,7 +45,6 @@ Event MouseReleased(int x, int y) {
 }  // namespace
 
 TEST(SliderTest, Right) {
-  int updated = 0;
   int value = 50;
   auto slider = Slider<int>({
       .value = &value,
@@ -53,31 +52,23 @@ TEST(SliderTest, Right) {
       .max = 100,
       .increment = 10,
       .direction = Direction::Right,
-      .on_change = [&]() { updated++; },
   });
   Screen screen(11, 1);
   Render(screen, slider->Render());
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(3, 0)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(9, 0)));
   EXPECT_EQ(value, 90);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(9, 2)));
   EXPECT_EQ(value, 90);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(5, 2)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 2);
   EXPECT_TRUE(slider->OnEvent(MouseReleased(5, 2)));
   EXPECT_FALSE(slider->OnEvent(MousePressed(5, 2)));
-  EXPECT_EQ(value, 50);
 }
 
 TEST(SliderTest, Left) {
-  int updated = 0;
   int value = 50;
   auto slider = Slider<int>({
       .value = &value,
@@ -85,31 +76,23 @@ TEST(SliderTest, Left) {
       .max = 100,
       .increment = 10,
       .direction = Direction::Left,
-      .on_change = [&]() { updated++; },
   });
   Screen screen(11, 1);
   Render(screen, slider->Render());
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(3, 0)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(9, 0)));
   EXPECT_EQ(value, 10);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(9, 2)));
   EXPECT_EQ(value, 10);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(5, 2)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 2);
   EXPECT_TRUE(slider->OnEvent(MouseReleased(5, 2)));
   EXPECT_FALSE(slider->OnEvent(MousePressed(5, 2)));
-  EXPECT_EQ(value, 50);
 }
 
 TEST(SliderTest, Down) {
-  int updated = 0;
   int value = 50;
   auto slider = Slider<int>({
       .value = &value,
@@ -117,32 +100,23 @@ TEST(SliderTest, Down) {
       .max = 100,
       .increment = 10,
       .direction = Direction::Down,
-      .on_change = [&]() { updated++; },
   });
   Screen screen(1, 11);
   Render(screen, slider->Render());
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(0, 3)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(0, 9)));
   EXPECT_EQ(value, 90);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(2, 9)));
   EXPECT_EQ(value, 90);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(2, 5)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 2);
   EXPECT_TRUE(slider->OnEvent(MouseReleased(2, 5)));
   EXPECT_FALSE(slider->OnEvent(MousePressed(2, 5)));
-  EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 2);
 }
 
 TEST(SliderTest, Up) {
-  int updated = 0;
   int value = 50;
   auto slider = Slider<int>({
       .value = &value,
@@ -150,27 +124,20 @@ TEST(SliderTest, Up) {
       .max = 100,
       .increment = 10,
       .direction = Direction::Up,
-      .on_change = [&]() { updated++; },
   });
   Screen screen(1, 11);
   Render(screen, slider->Render());
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(0, 3)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 0);
   EXPECT_TRUE(slider->OnEvent(MousePressed(0, 9)));
   EXPECT_EQ(value, 10);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(2, 9)));
   EXPECT_EQ(value, 10);
-  EXPECT_EQ(updated, 1);
   EXPECT_TRUE(slider->OnEvent(MousePressed(2, 5)));
   EXPECT_EQ(value, 50);
-  EXPECT_EQ(updated, 2);
   EXPECT_TRUE(slider->OnEvent(MouseReleased(2, 5)));
   EXPECT_FALSE(slider->OnEvent(MousePressed(2, 5)));
-  EXPECT_EQ(value, 50);
 }
 
 TEST(SliderTest, Focus) {