feat: add option to show only visible items

[devprofile98]

I was adding a filter/search feature to your mqttui project, so I needed to be able to render only items that qualified the search query. To achieve this, I added an is_visible option to the TreeItem struct and a function to change the item's visibility. Additionally, these changes are backward-compatible.

[EdJoPaTo]

I am not sure about this change… visibility seems more like a state to me so it should be either on the TreeState or the invisible items should be filtered out entirely instead of filtering them later on?

Filtering out items before passing them to the Tree to render would require a different way of referencing the items in the TreeState (mqttui doesn't really use the current way either so it should be overhauled I guess…) see #24

[devprofile98]

I think the TreeState is something that holds the state of the Tree as a whole, each Item should hold its state for itself, like how objects holds their visible property in ui frameworks or game engines. I agree that the items that are visible should be filtered in one step, is the flatten() a good place for this? I also understand that your concern about performance, holding some kind of cache for visible items, like the TreeIdentifierVec that you mentioned may help us to save some CPU clocks.

[EdJoPaTo]

I think the TreeState is something that holds the state of the Tree as a whole, each Item should hold its state for itself, like how objects holds their visible property in ui frameworks or game engines.

With that logic the open / close state should also be on the individual item rather than the TreeState. That itself is probably easy to do. The more complicated part is the keyboard action as the user then needs to manage by themselves which item to mark open / closed 🤔

[devprofile98]

The more complicated part is the keyboard action as the user then needs to manage by themselves which item to mark open / closed 🤔

the goal is achiveable by only open/close or visible/invisible properties as i see, and i cant see where it is up to the user to handle them, If you are concerned about maintaining the overall condition of the tree at each moment (and i think we should ), then holding TreeIdentifierVec as cache is good

[EdJoPaTo]

the goal is achievable by only open/close or visible/invisible properties as i see, and i cant see where it is up to the user to handle them

mqttui generates the TreeItems on each render from the internal data and does not keep them over multiple runs. When a user presses a key the TreeState is changed and kept to the next render. This means everything on the TreeItem is gone until the next render while the TreeState is kept.
Maybe I understand the approach of tui wrong there but that's what I got from their way of doing things.

So when open/close or visible state is on the Item they must be known on the item generation which is the developer and not the crate.

Am I overlooking something?

[devprofile98]

I am not also familiar with the way of TUI's, but MQTTUI and this project are different projects, so I think we should do the right thing for this project first and the other one should follow this changes, or make them a single project.
re-evaluating TreeState is not efficient unless there is a change, and I believe there should be a way to cache the overall tree state and change it on actual changes, not every tick.
for this purpose, holding an index list of open/close Item could be sufficient, like the current way, same thing could been applied for visible/invisible items if I understand the problem correctly, also holding the list of pointers to TreeItems is another way ( like Vec<Rc> ).
what do you think?

[EdJoPaTo]

I am not also familiar with the way of TUI's, but MQTTUI and this project are different projects, so I think we should do the right thing for this project first and the other one should follow this changes, or make them a single project.

this library was built for mqttui as mqttui needed something like that and its probably useful for other people too. So my own goal with this library is to provide something useful to mqttui. When I can use it better for mqttui its probably easier to use for others too so I prefer to think about my stuff first to solve my problems. Solving them in a good way will result in better results for everyone. The hard part here is the requirement of breaking changes which is a bit annoying for everyone including me.

re-evaluating TreeState is not efficient unless there is a change, and I believe there should be a way to cache the overall tree state and change it on actual changes, not every tick. for this purpose, holding an index list of open/close Item could be sufficient, like the current way, same thing could been applied for visible/invisible items if I understand the problem correctly

In case of mqttui the data is there and the text of the items can be different so it would mean a lot of "which item to update" logic. Regenerating the whole items is just easier than updating most of them while needing to search for them first.

Keeping the open/close stuff somewhere else than the TreeState is probably a good idea but that should be done on #24 first (while keeping in mind that invisibility might follow in the future).

also holding the list of pointers to TreeItems is another way ( like Vec ).

Keeping the index or pointers doesn't change much of the logic I think. Pointers are probably more annoying the handle as indices.

[EdJoPaTo]

Seeing this being closed without any further comment… Whats your approach on visibility now?

With #27 in place it should be much easier to accomplish visibility with a working TreeState. Just dont create the invisible TreeItems on render. The TreeState isnt using the index anymore per default and will not result in a broken open / close situation as it did before.
At least thats what mqttui will likely do in the future. Or are there other approaches?

Personally I think its still strange to have the TreeState manage open/close…

[devprofile98]

Hello Ed,
When I created this pull request, my primary objective was to enhance the functionality of the mqttui project by adding a search feature, we needed that and we wanted to be able to search between publishers and topics, so I forked these two projects and made some changes, actually, I did add search in my mqttui fork, without search in mqttui, it was quite challenging to find an specific topic within even more than 30 topics. so thank you for Mqttui, I used it a lot and still utilizing its cli mode, I think of search functionality for mqttui as something that it must have, mine was so basic and was based on changes that I made in this project and unfortunately, I don't remember the project structure very well now, but if I find some spare time, I am eager to contribute and add these features.

[EdJoPaTo]

Topic search is definitely on my list of wanted features for mqttui and it should be easier to do now with the generic identifier. Thank you for you compliments (it is definitely motivating) and your interest in contributing!

[EdJoPaTo]

I started working on it! You might want to take a look into EdJoPaTo/mqttui#144 and join in with thoughts and ideas if you like. Would be nice to have other thought on it! :)