Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dynamic list childs events (click etc) #282

Open
johannphilippe opened this issue Dec 24, 2021 · 18 comments
Open

dynamic list childs events (click etc) #282

johannphilippe opened this issue Dec 24, 2021 · 18 comments
Assignees

Comments

@johannphilippe
Copy link
Contributor

The dynamic list does not seem to allow its childs to respond to user interaction (key, click ...) .

class basic_row : public htile_composite
{
public:
    basic_row(size_t index) : htile_composite(),
        _index(index)
    {
        for(size_t i = 0; i < 4; i++)
        {
            auto input = input_box(std::to_string(i));
            input.second->on_text = [&](std::string_view s ){
                std::cout << "on text " << std::endl;
            };
            push_back(share(input.first));
        }
    }


    size_t _index;
};

int main(int argc, char* argv[])
{
    auto && make_row = [](size_t index)
    {
      return share(basic_row(index));
    };

   app _app(argc, argv, "Empty Starter", "com.johannphilippe.empty-starter");
   window _win(_app.name());
   _win.on_close = [&_app]() { _app.stop(); };

   view view_(_win);

   auto my_composer = basic_cell_composer(1000, make_row);
   auto content = share(dynamic_list(my_composer));

   view_.content(
               margin({10, 10, 10, 10},
                      vscroller(hold(content))
                      ),
               background
               );


   _app.run();
   return 0;
}
@johannphilippe
Copy link
Contributor Author

I wrote a class copying dynamic_list and adapting a few actions from composite_base (click and key). It works with a special cell_composer whose compose method returns an already created element. I also templated all those guys, to allow a get_composer to access the real rows elements, not just element_ptr, which is really useful in active elements context.
I have a few bugs to track before I have something to show or PR, but it's on the way !

@johannphilippe
Copy link
Contributor Author

This class would also need to manage all actions (scroll, drag, cursor...). So, there is still some work to be done here :)

@djowel djowel self-assigned this Dec 25, 2021
@djowel
Copy link
Member

djowel commented Dec 25, 2021

This class would also need to manage all actions (scroll, drag, cursor...). So, there is still some work to be done here :)

Yes there is :) Thank you very much for doing this. I suggest upgrading dynamic_list instead of makeing a new one. After all, it's what's intended.

@djowel
Copy link
Member

djowel commented Dec 25, 2021

It works with a special cell_composer whose compose method returns an already created element.

I don't think this is necessary. cell_composer as-is can already do that. The user sill just have to save the returned element ptr somewhere so it won't be garbage collected.

@johannphilippe
Copy link
Contributor Author

johannphilippe commented Dec 25, 2021

It works with a special cell_composer whose compose method returns an already created element.

I don't think this is necessary. cell_composer as-is can already do that. The user sill just have to save the returned element ptr somewhere so it won't be garbage collected.

You're right, I think I was going tired. The make lambda can be used to store elsewhere.
Are you ok to insert a resize() method to the dynamic list ? It would just resize the composer and update.

@johannphilippe
Copy link
Contributor Author

I have everything working quite fine here. Just the scroll method does not react in dynamic list.

@johannphilippe
Copy link
Contributor Author

johannphilippe commented Dec 25, 2021

https://github.com/johannphilippe/elements/tree/active_dynamic_list/
Hope I didn't make a mistake on my fork this time :)
Click, key, drag, cursor are implemented.
For scroll, I'm not sure if element base class is supposed to receive it ?
There is an example called active_dynamic_list in the example folder to play with (two possible make functions, to check particular cases with a text box and a canvas)

@johannphilippe
Copy link
Contributor Author

A few thoughts about it :

  • Add a template parameter or an argument to specify whether it is a vertical or horizontal list ?
  • This last idea could lead to the creation of fast table list element (2D list)
    That's it.

@djowel
Copy link
Member

djowel commented Dec 26, 2021

I had a brief look, and it's looking good! I'll probably have some comments/edits when you file a PR.

  • For consistency, instead of a template for vertical or horizontal list, I suggest a base dynamic_list, hdynamic_list and vdynamic_list.
  • resize makes sense.
  • scroll not working is probably because of the scrollable view it is already in, but not necessarily dynamic_list. Try making a nested scroll views. I bet the parent scrollable view intercepts the scroll events and does not let it pass through the child. This can be 'fixed, as a separate issue.
  • hgrid and vgrid should be performant too. Although, it can probably take advantage of some optimizations, as done in dynamic_list. Have you tested it? The advantage of grids is that it is possible to have different cell sizes.

@johannphilippe
Copy link
Contributor Author

johannphilippe commented Dec 26, 2021

I had a brief look, and it's looking good! I'll probably have some comments/edits when you file a PR.

Of course I'm sure there are a lot of things to improve ! I didn't run a lot of tests yet. I think there are some ways to improve it in the use of ̀_composer.compose that I used a lot to replace _rows[index].elem_ptr to avoid crashes. In an "active" context (list with user actions) it's quite good, since the user keeps pointers to elements. But in a static context, it may slow down the list.

@johannphilippe
Copy link
Contributor Author

johannphilippe commented Dec 26, 2021

I didn't try grids yet. I might try to replace htiles by hgrids in my tables if there is a performance difference ?

@johannphilippe
Copy link
Contributor Author

  • scroll not working is probably because of the scrollable view it is already in, but not necessarily dynamic_list. Try making a nested scroll views. I bet the parent scrollable view intercepts the scroll events and does not let it pass through the child. This can be 'fixed, as a separate issue.

I think I ran a test with a small dynamic list that is not in a scroller with same results : no scroll action. I need to re-test that.

@djowel
Copy link
Member

djowel commented Dec 26, 2021

I think I ran a test with a small dynamic list that is not in a scroller with same results : no scroll action. I need to re-test that.

I think some member functions are not yet implemented, for example, wants_control()

@djowel
Copy link
Member

djowel commented Dec 26, 2021

I didn't try grids yet. I might try to replace htiles by hgrids in my tables if there is a performance difference ?

Yes. The main advantage of htiles and vtiles is that they have "fluid" layouts and automatically adjusts to window resizing, etc. But that can be expensive. Tiles, in general, should be used at the topmost only.

@johannphilippe
Copy link
Contributor Author

I think some member functions are not yet implemented, for example, wants_control()

It's true. I don't know what wants_control is used for yet.

@johannphilippe
Copy link
Contributor Author

johannphilippe commented Dec 26, 2021

For consistency, instead of a template for vertical or horizontal list, I suggest a base dynamic_list, hdynamic_list and vdynamic_list.

That implies to modify slightly the cell_composer.
I suggest something like

float main_axis_size;
limits secondary_axis_limits;

instead of

float line_height;
limits width_limits;

Where main_axis_size will represent line_height in vdynamic_list, or width in hdynamic_list.

@johannphilippe
Copy link
Contributor Author

I now have hdynamic_list and vdynamic_list working, with a new example "two_dimensional_list".
Also improved performance.
I had to mess a bit with cell_composers though, and copied the fixed_derived_limits_cell_composer for vertical_fixed_derived_limits_cell_composer and horizontal_fixed_derived_limits_cell_composer.
Did the same for basic_cell_composer which now has a version for vertical and horizontal. Probably not the best way to do, I don't know.
Do I submit PR from the active_dynamic_list branch I made ?

@johannphilippe
Copy link
Contributor Author

johannphilippe commented Dec 27, 2021

The scroll event works fine if the list is not inside a scroller !
Thought, I don't know if this is relevant or not, but I'm kind of interested in having the scroll event to propagate to childs of scroller.
Let's say :

  • It would not propagate if the child has no scroll event (scroll event returns false for example) so the scroller scrolls
  • The scroller doesn't scroll and the childs receives scroll if it has a scroll event (and returns true)
    Does it seem good ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants