Opened 10 years ago

Closed 10 years ago

#3161 closed bug (fixed)

[PATCH] BTabView layout fix

Reported by: aljen Owned by: axeld
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

When BTabView is embedded in layouted parent and BView::BView(name, flags, layout) constructor is used for creating tabs, only selected tab (usually first one) is relayouted, rest tabs views aren't (and are not visible, because they've got frame(0,0,0,0))

Attachments (4)

view.diff (462 bytes) - added by aljen 10 years ago.
second part of this fix
tabview.diff (2.7 KB) - added by aljen 10 years ago.
tabview_layouted.diff (5.3 KB) - added by aljen 10 years ago.
tabview_layouted.2.diff (5.0 KB) - added by aljen 10 years ago.

Download all attachments as: .zip

Change History (9)

Changed 10 years ago by aljen

Attachment: view.diff added

second part of this fix

Changed 10 years ago by aljen

Attachment: tabview.diff added

comment:1 Changed 10 years ago by stippi

The patch is not taking the right approach. You are trying to embed an old-style (ie using "follow modes") view hierarchy into a new-style layout managed hierarchy. This is why you also need the "view.diff" patch, which is wrong. Follow modes are not supposed to work in the layout management.

What needs to happen is that BTabView actually uses a BCardLayout to layout it's children views. The *Size() methods would be implemented accordingly by using the return values from the respective functions of layout. The user may supply a custom layout though, so use SetLayout() in the constructor and then access only via Layout()! What will make the implementation more involved is that the BTabView should still behave like it did before when in old-style "follow mode". Maybe it's possible to implement the old mode via the new mode though. But extra care needs to be taken, and a lot of testing with existing apps.

One important thing to keep in mind: The layout mode should not detach the views like the follow mode version does. I think this was broken even there, as can be witnessed in many apps, for example Appearance. If you resize the window and then switch to another tab, the view size did not adopt, even if follow modes are set correctly. Implementations would need to take extra care and layout themselves when attached again by a tab change. I don't want this to happen at least for the layouted mode. There, the BCardLayout will take care of making children visible and invisible according to the active tab.

All this being said, thanks a lot for your work! But I hope you understand that I can't apply the patch as is. If you have the time and motivation, it would be great if you give it another shot taking these things into account! Thanks a lot!

Changed 10 years ago by aljen

Attachment: tabview_layouted.diff added

comment:2 Changed 10 years ago by aljen

Thanks for explanation :) I followed your tips and implemented layout mode using BCardLayout. Normal behaviour isn't changed (tested with existings apps), changes are visible only when using new layout friendly constructor

comment:3 Changed 10 years ago by stippi

Ok, this looks much better! The *Size() methods are all fine. There are to remaining issues:

1) You cannot add a new virtual Deselect() version to BTab, since that will break binary compatibility. But you can change the code in the existing Deselect() version to check the parent view's layout. Ie

bool removeView = false;
BView* container = View()->Parent();
if (container != NULL) {
    removeView = dynamic_cast<BCardLayout*>(container->GetLayout()) == NULL;
}
if (removeView)
   View()->RemoveSelf();

... this will also simplify the code in a few other places.

2) In line 707, you dynamic_cast the Layout() to BCardLayout*, but don't check for success. If the user did used another type of layout, then he is out of luck I'd say.

Thanks again for your work!

comment:4 Changed 10 years ago by aljen

Ahhh, right I forgot about keeping binary compability :)[br] Deselect now is working like you said and I added error-checking for dynamic_cast in code[br] Thanks ! :)

Changed 10 years ago by aljen

Attachment: tabview_layouted.2.diff added

comment:5 Changed 10 years ago by stippi

Resolution: fixed
Status: newclosed

Applied your patch in hrev28701. Thanks a lot for your work!

Note: See TracTickets for help on using tickets.