Conflation of containment and visibility in TabbedComponent


#1

Hi,

Working with TabbedComponent, I noticed that tabs lose their containment (parent node) when they become invisible. Looking at the code I found that -indeed- the TabbedComponent calls removeChildComponent() when a tab is hidden.

Since containment and visibility are entirely separate concepts, seeing them conflated like this makes my code-smell alarm go off rather loudly. I thus have to assume there is a deep technical reason for why an inactive tab cannot have a parent. But I can’t think of one. Please advise. Thanks.


#2

My nose is acutely sensitive to code-smells, but I don’t really smell a problem with that… It has a list of components to use as tabs, and adds/removes them so that only one at a time is a child component… I can’t think of any advantage in keeping a load of invisible comps hanging around as children when they’re not doing anything?


#3

Thanks for your reply.

It’s just that in my dialogs they are not at all “doing nothing”. For example, when the dialog’s Apply button is pressed all tabs have to save their state, not just the active one. Likewise when Close is pressed all tabs need to cancel unapplied settings, not just the active one. Etc.

In fact, whether a tab is visible or not is irrelevant to my code. I care about components not leaving the containment hierarchy I put them in (behind my back!) and not at all about which of the tabs the user might currently be looking at. That’s a detail nobody (well, besides that user) is interested in.

I also have to insist, with prejudice, that containment and visibility are entirely and utterly unrelated concepts, with fully independent life-cycles. :wink:

[i]


#4

I understood, and agree with that. But it’s not an argument against removing the background tabs from their parent (?)

TBH the original version of the tabbedcomp actually deleted its child comps when they weren’t active, so you’re getting off lightly! But really, I don’t understand your issue - if your child component can’t survive without a parent, then that’s a code-smell itself, surely? A child comp shouldn’t be so tightly-coupled to its parent, particularly if the parent is just a generic component like the tabbedcomp, which may change its behaviour in future without warning.

Don’t worry, you don’t come across like that at all! And I like to be challenged on things like this, it’s the only way to improve… :slight_smile:


#5

I would expect them to stay in the hierarchy, in fact these classes depend on it:

vf::componentNotifyParent
vf::componentBroadCast

In the OP’s case I would handle pressing Apply this way:

void MyTabbedDialog::onApply ()
{
  // tell all tabs that Apply was pressed
  vf::componentBroadcast (this, &PreferencesTab::onApply);
}

So, you said you can’t think of an advantage but I can think of one: it participates in the hierarchy of Components leading up to the top level window.

Also, what happens if the LookAndFeel changes while one of the tabs is invisible? Will it miss the notification?


#6

Sorry guys, but the tabbedcomponent has just never worked that way! The original one used to create/delete the components on demand when they were shown and hidden. Then I changed it to allow them to persist, but didn’t see any reason to clutter the hierarchy with inactive tabs… Sure, you can come up with lots of examples where your particular class might find it vaguely convenient to have access to its parent, but I’m not sold on the idea that it’s somehow essential, or even particularly important, to guarantee that these things don’t get removed. When a comp is off-screen, it should be inactive… if it’s still busy when hidden, then perhaps you’re putting functionality into your GUI that should really belong in your data model?


#7

OIC. Well, I come to you from the Interwebs, and there we think of a tabbed component as one large dialog, shuffled into tabs simply to save screen real estate.

Also, there is still no reason why a tab has to lose its parent when hidden, is there? :wink:

So I have this suggestion to make:
https://github.com/stefanholek/JUCE/commit/342c89385cb31986f77c051b657df8d4642f1317

It basically turns panelComponent into an index and keeps tabs located at all times. The TabbedComponent behavior is not altered in any way, so I think it’s low risk to apply. I can make a pull request if you like.

Cheers


#8

It’s how MFC, the Windows Tab CommonControl, and the Mac OS Tab control works. JUCE behaves differently from what UI programmers have gotten used to over the last 20 years.

By your logic, why even have Component::setVisible when you can just remove the Component completely from the hierarchy?

And you never did answer the question, what happens to inactive tabs when the look and feel changes, do they get lookAndFeelChanged called?


#9

No, it won’t.

…no particular reason, except to allow them to respond to the visibility change.

I’m a bit mystified by the strong feelings on this… Personally, I couldn’t give a hoot whether tabs get removed, hidden, deleted, or all/none of the above! If a component is robustly-written, it should deal with any of those situations without any special handling.


#10

I realize I haven’t mentioned this yet, but my intention is to use findParentComponentOfType all over the place. At the moment this appears to work everywhere, except for hidden tabs.

So I was surprised. The method is even called addTab, not addTabButNotWhenHidden. :slight_smile:


#11

In my own coding nowadays I’m very wary of using that method… It can be a code smell, depending on the reason why you’re calling it.

Good reasons for using it are when your component may be placed inside different types of layout and needs to find out about its current context. E.g. The function autoScrollForMouseEvent() uses it to see whether or not something is inside a Viewport, and if so, scrolls that viewport. The drag-and-drop functionality uses it to see whether a component is inside a DragAndDropContainer, etc.

Bad reasons for using it are as a lazy way to get hold of some kind of shared resource that your component needs. If your comp needs a permanent reference to some other object, then give it a reference to that object! And preferably make it a non-GUI reference! If you have code like MyFoobarManager& m = findParentComponentOfClass()->getFoobarManager(), then that’s bad practice IMHO.

Anyway, that’s my last word on the subject, I’ve wasted too much time discussing it already!