natinusala / borealis

Hardware accelerated, controller and TV oriented UI library for PC and Nintendo Switch (libnx)
Apache License 2.0
260 stars 83 forks source link

TabFrame and Dialog modifications. #40

Closed jon-dez closed 3 years ago

jon-dez commented 4 years ago

TabFrame: rightPane now manages responsibilty for displaying the sidebar item's view. Dialog: set horizontalButtonsLayout's parent.

This is somewhat related to the Touch pr draft such that these modifications fix some problems with "touching" views. This is separate from the Touch pr draft, since these changes do not depend on the Touch pr, and could be merged without it.

natinusala commented 4 years ago

Thanks for the PR!

I'm sorry to say that now but we already intended to rewrite TabFrame using the LayerView @WerWolv made. This achieves roughly the same thing (LayerView would replace the container you wrote).

Do you think you could adapt your PR to use LayerView instead?

jon-dez commented 4 years ago

Adapting the PR to use LayerView seems more complex. My reasoning is the way I see it is that the Sidebar is already some sort of LayerView, since it contains SidebarItems's with associated views. I see the SidebarItems's as layers that ask the "rightPane" (ViewContainer) to bring it into view when they are clicked. So, in my opinion I just feel that asking a one layerview to ask another layerview to display its views a little redundant.

natinusala commented 4 years ago

How is the Sidebar a LayerView ? I would just replace the right pane with LayerView and have the sidebar call changeLayer when needed ?

jon-dez commented 4 years ago

I guess what I meant to say was that SideBarItems have an associated view with them, and we can use their click callbacks to change "rightPane" like so:

...
// Using ViewContainer
// 'item' is a SideBarItem
item->getFocusEvent()->subscribe([this](View* view) {
    if (SidebarItem* item = dynamic_cast<SidebarItem*>(view))
       this->rightPane->setView(item->getAssociatedView());
});
...

But using LayerView:

...
// Using TabFrame
// 'item' is a SideBarItem
int next_item_idx = this->rightPane->getLayerCount(); // Note: This member function does not exist
this->rightPane->addLayer(item->getAssociatedView());
item->getFocusEvent()->subscribe([this, next_item_idx](View* view) {
    if (SidebarItem* item = dynamic_cast<SidebarItem*>(view))
       this->rightPane->changeLayer(next_item_idx, true);
});
...

As you can see, there are less steps involved in the first example. Also, it eliminates having to keep track of the index of the sidebar item's associated view in the view layer and avoids having to keep a duplicate record of the views in a LayerView.

natinusala commented 4 years ago

Why don't you add the associated view id in the SidebarItem ? When adding a tab, add the view to the LayerView and store the id in the SidebarItem

natinusala commented 3 years ago

Closed as this PR is reimplemented or made irrelevant by the changes on the yoga branch.