Skip to content

Conversation

@eoineoineoin
Copy link
Member

Mildly-controversial PR incoming.

I was looking to fix space-wizards/space-station-14#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.

@PJB3005
Copy link
Member

PJB3005 commented Oct 23, 2025

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

@Absotively
Copy link

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.

@eoineoineoin
Copy link
Member Author

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.

@Absotively
Copy link

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
Copy link

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.

AddChild(Label);
}

protected override void StylePropertiesChanged()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this overload.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent guidebook collapsible UI

3 participants