ArthurSonzogni / FTXUI

:computer: C++ Functional Terminal User Interface. :heart:
MIT License
6.73k stars 401 forks source link

Question : Rendering different UI states : colored buttons on click, #161

Closed daminetreg closed 2 years ago

daminetreg commented 3 years ago

Hi @ArthurSonzogni,

Here is more of a question than any kind of bug report while working with your library with @Lambourl for it's integration at the CLI UI for tipi.build.

What is supposed to be the way that you've foreseen to have for instance colored components that are updated on events (custom or click) ?

We went for something where we delete from the Component children_ list and from the Element list before rerendering.

I feel we are missing the big picture, how it supposed to be done. I was wondering, why aren't all Components also Elements ?

Is it a design choice or a lack of time ? Because in the solution we have found it seems to require some manual synchronization between Elements and Components vectors.

My use case is : I want to change the color state of the button when I click on it.

The follow works but I don't think it's the way it is supposed to be done ? Or is it ?

image
#include <memory>  // for allocator, __shared_ptr_access, shared_ptr
#include <string>  // for wstring, basic_string
#include <vector>  // for vector

#include "ftxui/component/captured_mouse.hpp"  // for ftxui
#include "ftxui/component/component.hpp"  // for Radiobox, Renderer, Tab, Toggle, Vertical
#include "ftxui/component/component_base.hpp"      // for ComponentBase
#include "ftxui/component/screen_interactive.hpp"  // for ScreenInteractive
#include "ftxui/dom/elements.hpp"  // for Element, separator, operator|, vbox, border

using namespace ftxui;

int main(int argc, const char* argv[]) {

  struct AppMainWindow : public ComponentBase {

      int tab_selected = 0;
      std::vector<std::wstring> tab_1_entries{
          L"Forest",
          L"Water",
          L"I don't know",
      };
      int tab_1_selected = 0;

      std::vector<std::wstring> tab_2_entries{
          L"Hello",
          L"Hi",
          L"Hay",
      };
      int tab_2_selected = 0;

      std::vector<std::wstring> tab_3_entries{
          L"Table",
          L"Nothing",
          L"Is",
          L"Empty",
      };
      int tab_3_selected = 0;

      Component tab_container = Container::Tab(
      {
          Radiobox(&tab_1_entries, &tab_1_selected),
          Radiobox(&tab_2_entries, &tab_2_selected),
          Radiobox(&tab_3_entries, &tab_3_selected),
      },
      &tab_selected);

      Components buttons;
      Elements button_line;

      void updateButtonModel() {
          button_line.clear();
          for (auto& button : buttons) {
              auto found = std::find(children_.begin(), children_.end(), button);
              if (found != children_.end()) {
                  children_.erase(found);
              }

          }
          buttons.clear();

          for (int i = 0; i < 3; ++i) {
            auto button = Button( std::wstring(L" [ ⚈ Hey ") + std::to_wstring(i) + std::wstring(L"] "), 
            [this, i] () mutable { 
                tab_selected = i;
                updateButtonModel();
             }, false);
            this->Add(button);
            button_line.push_back(hbox(button->Render()) | color(((tab_selected == i) ? Color::Red1 : Color::Green1)));
            button_line.push_back(separator());
          }
      }

      AppMainWindow() : ComponentBase() {

          this->Add(tab_container);            
          this->updateButtonModel();
      }

      virtual Element Render() override {
        return 
        vbox({
                hbox(
                    button_line
                ),
                hbox({
                   tab_container->Render(),
                    }),
        }) ;
      }

  };

  auto screen = ScreenInteractive::TerminalOutput();
  screen.Loop(std::make_shared<AppMainWindow>());
}
ArthurSonzogni commented 3 years ago

Hi @daminetreg. Yes, I don't expect folks to add/remove component from the tree.

Note: What you showed on the example looks like the Toggle component.


The general idea is:

So a Component produces Element(s), and Elements produces Pixel to the Screen.

So, I believe in your case, you need to implement your own component doing what you would like to see.

I believe there are those possibilities:

  1. Implement your own component from scratch.
  2. If overriding only the Render() function is enough. You could wrap your component with:
    Component wrapper = Renderer(wrapped, [&state] {
    Element inner = wrapped->Render();
    return XXX(inner);
    })
  3. Ask me to add a few options to Button, similar to ToggleOption, MenuOption, RadioboxOption to specify a style when focused.

However note that Button executes its action when the mouse is pressed, not released. So this is already a bit different from what you want. Maybe I should change this and add a "pressed" style.

daminetreg commented 3 years ago

Thanks for the explanation. What made me wonder was why we need to keep a tree of components + a tree of Elements.

But now I understand that Components are meant to stay, while Elements are temporary that I can regenerate on each "Draw" event (gamemaker ^^).

I've seen most of the Components have private members, wouldn't it be a good thing to have them protected instead so that they are meant to be extended ?

Because actually in the case of the Button I couldn't just override Render() as I wanted to be able to dynamically changed the label and they were private. Because of this I add to copy/paste the ButtonBase.

class ColoredButton : public ComponentBase {
    public:
    ColoredButton(const std::wstring& label,  std::function<void(ColoredButton&)> on_click) : 
        label(label),
        on_click_(on_click)
    {}

    Element Render() override {
      return text(label) | color(color) | reflect(box_);; 
        //return ButtonBase::Render() | color(color); 
    }

    bool OnEvent(Event event) override {
      if (event.is_mouse() && box_.Contain(event.mouse().x, event.mouse().y)) {
        if (!CaptureMouse(event))
          return false;

        TakeFocus();

        if (event.mouse().button == Mouse::Left &&
            event.mouse().motion == Mouse::Pressed) {
          on_click_(*this);
          return true;
        }

        return false;
      }

      if (event == Event::Return) {
        on_click_(*this);
        return true;
      }
      return false;
    }

    std::wstring label;
    Color color;

    private:
    Box box_;
    std::function<void(ColoredButton&)> on_click_;
};

We will contribute back some component then. Happy for your review ^^ As per the great old times of Le CBNA.

daminetreg commented 3 years ago

Perhaps a last point, that I think makes it difficult to understand, is that on most example there is a mix of direct Elements and of component where Render is called. I don't know if it would be clearer if Render would be named something like : RenderToElement.

daminetreg commented 3 years ago

Just my 2cents now that I understand a bit more, If I put back my chose into the one of an user of FTXUI:

For instance if I take the example to tab_horizontal, there is separator() that is an Element and for me as newcomer I see : tab_menu->Render() which for me is a Component in middle of the Renderer. This makes it complex to understand that actually here should be only Elements there.

  auto container = Container::Horizontal({ 
      tab_menu,
      tab_container,
  });

// To me newbie user it's also hard to understand why a Container::Horizontal is a component, that I don't even render directly in the Renderer but somehow needs to be there.

  auto renderer = Renderer(container, [&] {
    return hbox({
               tab_menu->Render(), // Component
               separator(),                 // Element
               tab_container->Render(), // Component
           }) |
           border; // Element
  });

Would it be a PR that you would welcome to have a ComponentsRenderer that would see if the given type is Component vs Element, and when it is a Component it Add() it as child + Renders it on Event but when it is an Element it just keeps it and forward it on Event ?

So that we don't need to maintain one Component tree + a Renderer initializer_list we could end with something like this on the caller side :

  auto renderer = ComponentRenderer(
    hbox(
      tab_menu,
      separator(),
      tab_container
    ) | border;
ArthurSonzogni commented 3 years ago

Sorry for this late reply. I am on vacation this week.

I've seen most of the Components have private members, wouldn't it be a good thing to have them protected instead so that they are meant to be extended ?

Well, actually, I am trying to hide the implementations of every components in the latest FTXUI version. The only remaining API surface is the functions used to construct them.

I guess this doesn't really help people wanting to extend them. At least this allow me to evolve the implementation without breaking the API all the time.


Because actually in the case of the Button I couldn't just override Render() as I wanted to be able to dynamically changed the label and they were private. Because of this I add to copy/paste the ButtonBase.

You can already change the button label by providing a pointer for the first argument:

std::wstring button_label = L"button label";

auto button = Button(&button_label, [&] {
  // Change button label on click.
  button_label = L"New label".
});

We will contribute back some component then. Happy for your review ^^ As per the great old times of Le CBNA.

;-)

I would be happy to help you as well.


Would it be a PR that you would welcome to have a ComponentsRenderer that would see if the given type is Component vs Element, and when it is a Component it Add() it as child + Renders it on Event but when it is an Element it just keeps it and forward it on Event ?

But then, we would have to make every functions taking Elements to also support Components and return Components?

For instance:

Element bold(Element)

could be also used as:

Component bold(Component)

One thing to note is that Container::XXX() expects keyboard navigation to be able to focus its children one by one. So when using:

hbox({
  component,
  separator(),
  component,
})

Then, you would have 3 children, but only the to component draw something differently when focused.

Maybe I should add a way for components "not to be focusable" and make the Container::XXX() able to skip them.

ArthurSonzogni commented 2 years ago

@daminetreg I believe have addressed your comment during this time:

  1. Component implementations are now private. We only expose function to construct them.
  2. We can use the Renderer function to generate a component that will use the argument function to render:
    auto component = Renderer([] {
    return text("Lorel ipsum");
    });

    They can also be used to customize the Render() function of some other component:

    auto decorated = Renderer(nested, [child] {
    return nested->Render() | border | bold;
    });
  3. Component are now aware if they are "focusable" or not. It means when adding 3 components in a vertical container, keyboard navigation will skip the one not focusable.

However for now.

  1. This doesn't really solve the original question about colored button on click.
  2. This doesn't really provide "extension" of function taking "Element" toward functions taking "Component". It would have been nice to find a way to get it automatically get it for free.
daminetreg commented 2 years ago

Ok that's interesting the Renderer that allows nested->Render() to be overriden. We didn't release yet the FTXUI based UI but it will be revived and we are going to release it soon. 👍

daminetreg commented 2 years ago

Thanks :wink:

ArthurSonzogni commented 2 years ago

Ok that's interesting the Renderer that allows nested->Render() to be overriden. We didn't release yet the FTXUI based UI but it will be revived and we are going to release it soon. +1

I would be curious/happy to take a look if you want.

daminetreg commented 2 years ago

I'll brush up stuffs and come back to you when it's ready :-)