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.
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?
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.
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…
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?
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.
…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.
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!