feat: Prevent some global widgets from being attached to a panel

[sir4ur0n]

I went for the "have a constant list of forbidden widget types at the panel level, and check in BaseWidget" approach.

An alternative would be "copy-paste the check (or create a tiny function and call it) in each specific constructor". However it felt a bit clunky to me.

Let me know if you would rather another approach.

Closes https://github.com/statgen/locuszoom/issues/271

[abought]

I like the effort to provide code reuse, but I'm a bit worried about the maintainability of this approach. Because it is based on registry names (layout options) rather than code, anyone extending one of these widget classes would bypass the mechanism entirely. And because the behavior is defined partly outside the class, it might not be immediately obvious where this is happening.

That seems a little unintuitive for object-oriented code. (normally you'd expect child classes to inherit the same behavior as the parent) Additionally, any new custom widget would not be able to extend this list of forbidden types, so it would be hard to use with extensions that provide new features.

From an OOP standpoint, it makes more sense for the behavior to be checked in the constructor for each child class that is restricted. There are very few buttons, so de-duplication doesn't save much (but perhaps a helper functions, if we're worried about the redundant text).

[abought]

Anyway, sorry for the brief reply- I'm out this week on PTO. I love seeing new PRs, so I'll try to poke my head in unofficially every day or two while out.

I hope the notes give a quick outline of current thinking. Tl;dr: general restrictions useful, maybe move some logic back into constructors for reasons noted?

[sir4ur0n]

No worries, it was on me for not wanting to follow the OOP style of the code base. I dislike inheritance because it easily becomes difficult to maintain with more than 1 layer, but I should have nonetheless followed the style of the code base.

I force-pushed changes to put the check in each constructor.

I also took the liberty to slightly rephrase each error message to make it more specific to the widget, let me know if that's ok (or feel free to update before/after merging).

[abought]

Thanks! After merge, I'll go back and remove the set_state widget restriction. (because a panel could do something creative) Overall looks nice!

[abought]

No worries, it was on me for not wanting to follow the OOP style of the code base. I dislike inheritance because it easily becomes difficult to maintain with more than 1 layer, but I should have nonetheless followed the style of the code base.

I have mixed feelings here. When building UI, there are a lot of reusable boilerplate steps- things like the template pattern are well suited to an inheritance world.

Some of the early LZ code was in 3-5 coding styles and seems to have been inspired by the darkest pits of Java- it's a hard balance to get OOP code right, and I think a lot of people end up avoiding it altogether after bad experiences. Mixing styles made stuff super confusing, so we've tried to standardize so that all behavior is defined in a predictable way.

Some brief thoughts, just to nerd out about technology and how we've evolved over time: I've been progressively changing the internals to make individual methods far more functional (same inputs -> same result), and less dependent on side effects/global state. You might also notice that the LZ docs increasingly emphasize public/private methods to help reduce the maintenance burden. ("this stuff is safe to change and we will usually respect the maintainers") Template pattern for structure and extensibility, but less global state.

Other libraries (like vue or react) are doing similar things when they use classes nowadays, via their hook and composition APIs.