Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow defining inheriting Styleboxes using a parent Stylebox with overrides #8094

Open
lostminds opened this issue Oct 13, 2023 · 5 comments · May be fixed by godotengine/godot#86779
Open

Comments

@lostminds
Copy link

lostminds commented Oct 13, 2023

Describe the project you are working on

Editor app with many buttons and panels

Describe the problem or limitation you are having in your project

Configuring controls and themes often require setting StyleBoxes for multiple states of the same control. For example, normal, hover, active, disabled. In most cases these styleboxes will be identical except for one or two parameters, like a different background or border.

But currently all properties need to be set for all styleboxes, which leads to a lot of data duplication and places where you need to change the same information if you want to change it. For example if you have your four Button state styleboxes set up and you decide to change the corner radius or border width, you'll need to set four values on four Stylebox resources. So 16 edits across four resources to change what I'd consider one property of the style.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add a simple inheritance mechanism in StyleBox resources where you can reference a parent StyleBox of the same type (StyleBoxFlat etc). This will then provide a default value for all parameters, and you can set only the parameters you want to override and be different in this StyleBox. Similar to how cascading style classes in CSS and other design style systems work.

With this in place I could just set a base ButtonStyleBox with all properties set, then add a HoverButtonStyleBox that references ButtonStyleBox and just overrides BGColor for example. If I then change border or corner properties in ButtonStyleBox this will automatically be inherited by HoverButtonStyleBox.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Add a new parentStylebox property on base StyleBox that can be used in all StyleBoxes (maybe not StyleBoxEmpty).
If parentStylebox is set, this resource will be used for default values for each property where the Resource doesn't have an override value set.

Currently it looks like the StyleBox editor interfaces already considers set parameters that are different from the base defaults overrides, that can be reset to the default. This means the interface, and possibly override flags for all properties are already in place at least in the editor. It's just that currently the stylebox doesn't inherit and override another stylebox, just the default values.

If this enhancement will not be used often, can it be worked around with a few lines of script?

You could I guess create your own stylebox override code that generates copies of a base stylebox resources with some overrides. But it will be inconvenient and hard to make as useful and convenient as integrating it in the base styleboxes.

Is there a reason why this should be core and not an add-on in the asset library?

StyleBoxes are part of the core UI components

@Mickeon
Copy link

Mickeon commented Oct 13, 2023

This could be better off generalised as a way for Resources to support some form of inheritance. StyleBox isn't the only class that suffers from this issue, and it won't be the last.

@lostminds
Copy link
Author

This could be better off generalised as a way for Resources to support some form of inheritance

That could be, but that sounds like a much bigger project. Starting with StyleBoxes could be a test case with a bit smaller scope where it will provide utility quickly.

It could also be implemented more generally from the start as part of the base Resource, but only be enabled for some Resource types using some allows_inheritance flag to limit the initial testing scope, but making it easy to expand to other resource types eventually.

@YuriSizov
Copy link
Contributor

That could be, but that sounds like a much bigger project. Starting with StyleBoxes could be a test case with a bit smaller scope where it will provide utility quickly.

You can't start with StyleBoxes. It needs to be done to the Resource class itself, somehow. This cannot be solved without a bigger change.

@ryanabx
Copy link

ryanabx commented Oct 19, 2023

I pointed this out in a similar proposal, but in addition to simple overriding of defaults, there should also be the ability to define relative-to-defaults, for example a font deriving from another font could be 2x the size of the original font, or 20 + x, or something similar.

@awardell
Copy link

awardell commented Nov 6, 2023

You can't start with StyleBoxes. It needs to be done to the Resource class itself, somehow. This cannot be solved without a bigger change.

This particular proposal could and should be implemented as a plugin specifically for StyleBoxes. That's a smaller project, and also not one that needs to go into the engine. I'm actually thinking of looking into how hard that would be to do myself. If I do, I'll obviously open-source it and put it on the Asset Library. It wouldn't make sense to add something like this to the core engine when it's just a stopgap and will be (hopefully) obsoleted.

I think general "resource-inheritence" is a great proposal, and there already is a proposal for that: #4089 Some of what was mentioned in this proposal might be worth mentioning over there.

I pointed this out in a similar proposal, but in addition to simple overriding of defaults, there should also be the ability to define relative-to-defaults, for example a font deriving from another font could be 2x the size of the original font, or 20 + x, or something similar.

I think this is a great idea and could also be worth mentioning over there. (I don't think that was the other proposal you were talking about)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants