RobustToolbox icon indicating copy to clipboard operation
RobustToolbox copied to clipboard

Update child controls style when a control is restyled

Open eoineoineoin opened this issue 2 months ago • 5 comments

Mildly-controversial PR incoming.

I was looking to fix https://github.com/space-wizards/space-station-14/issues/40923 - in that issue, the problematic element is a CollapsibleHeading. The CollapsibleHeading is a ContainerButton with a child TextureRect, which is what is used to display the triangles.

So, the following style rules should do the trick. Use collapsedTexture normally, but expandedTexture when the heading has been pressed:

            E<CollapsibleHeading>()
                .ParentOf(E<BoxContainer>())
                .ParentOf(E<TextureRect>().Class(OptionButton.StyleClassOptionTriangle))
                .Prop(TextureRect.StylePropertyTexture, collapsedTexture),

            E<CollapsibleHeading>().PseudoPressed()
                .ParentOf(E<BoxContainer>())
                .ParentOf(E<TextureRect>().Class(OptionButton.StyleClassOptionTriangle))
                .Prop(TextureRect.StylePropertyTexture, expandedTexture),

However, this only works once, and can't be changed once the CollapsibleHeading is visible. Pressing the CollapsibleHeading will toggle it's pressed pseudo state, which causes the CollapsibleHeading to be restyled. However, the style rules are targetting the child TextureRect, which never gets queued to refresh it's style.

This PR enqueues a style update for any child Controls whenever a parent Control's style is updated. Potentially, this is a bunch more style re-evalulations, but not sure there's a cleverer way to do this, since Selector.Matches could be doing anything, so no real way to know if child styles are affected.

eoineoineoin avatar Oct 22 '25 17:10 eoineoineoin

This should really check if the control is possibly involved in a child selector, and only do the recalculation in that case.

PJB3005 avatar Oct 23 '25 18:10 PJB3005

Button.StylePropertiesChanged() contains a workaround for this; it manually calls Restyle() on one of its children. It still doesn't check whether the calculation is actually needed, but it's applied only to a case where it is often needed.

Also, while CollapsibleHeading could use this workaround, controls in content can't, as Restyle() is not available to them.

I admit I'm pointing this out due to self-interest, as it affects switch buttons. But I do think having children update automatically is likely to be more generally useful. And the comment on the workaround in Button makes clear that the workaround was only meant to be temporary until a fix such as this is available.

Absotively avatar Oct 25 '25 19:10 Absotively

This should really check if the control is possibly involved in a child selector, and only do the recalculation in that case.

I agree with the sentiment - in 2e1c831, I added this; doesn't feel great, but would also worry that doing something more sophisticated would add a lot of complexity.

Button.StylePropertiesChanged() contains a workaround for this; it manually calls Restyle() on one of its children. It still doesn't check whether the calculation is actually needed, but it's applied only to a case where it is often needed.

Nice spot! I've removed the workaround since whatever solution we land on should obsolete that. The only other thing in Content (right now, at least) which uses parent selectors is hud tooltips, which wouldn't ever have their styles changed after construction.

eoineoineoin avatar Oct 26 '25 09:10 eoineoineoin

I think I have worked out how to work around the lack of this for switch buttons; I can apply relevant pseudoclasses to the buttons' children and change the style rules accordingly. I'm going to put off actually implementing that for the moment, though, in hopes that this gets merged and saves me from having to.

Absotively avatar Nov 02 '25 16:11 Absotively

I think I have worked out how to work around the lack of this for switch buttons; I can apply relevant pseudoclasses to the buttons' children and change the style rules accordingly. I'm going to put off actually implementing that for the moment, though, in hopes that this gets merged and saves me from having to.

I have implemented a workaround in switch buttons. It ended up being a bit messier than I'd hoped, so I'm definitely still hoping this change will some day remove the need for workarounds.

Absotively avatar Nov 28 '25 22:11 Absotively