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

FillConstraint ignores paddings by SiblingConstrains #127

Open
SashaSemenishchev opened this issue Oct 11, 2023 · 3 comments
Open

FillConstraint ignores paddings by SiblingConstrains #127

SashaSemenishchev opened this issue Oct 11, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@SashaSemenishchev
Copy link

SashaSemenishchev commented Oct 11, 2023

Describe the bug
When you make a stacked component and for example use Y, and every time you use SiblingConstraint() with paddings, the space in the parent component reduces, but FillComponent ignores that and uses full size and exceeds the space given by the parent component.

To Reproduce
Steps to reproduce the behavior:

val block = UIBlock().constrain {
                y = CramSiblingConstraint(3f)
                x = CramSiblingConstraint(3f)
                width = 100.pixels
                height = 110.pixels
                color = ConstantColorConstraint(VigilancePalette.getDividerDark())
            } effect ScissorEffect() childOf scroller
            UIImage.ofResource(module.logo).constrain {
                x = CenterConstraint()
                y = SiblingConstraint(3f) + 5f.pixels
                width = 64.pixels
                height = 64.pixels
            } childOf block
            UIText(module.spacedName).constrain {
                x = CenterConstraint()
                y = SiblingConstraint(10f)
            } childOf block
            val statusBlock = UIBlock(ConstantColorConstraint(BooleanDefinedColorState(module::state))).constrain {
                y = SiblingConstraint()
                x = CenterConstraint()
                width = 100.percent
                height = FillConstraint()
            } childOf block

            UIText("Enabled").constrain {
                x = CenterConstraint()
                y = CenterConstraint()
            } childOf statusBlock

Expected behavior
Used hard coded values

Снимок экрана 2023-10-11 в 20 26 17

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
No paddings:
No paddings
With paddings:
Paddings

@SashaSemenishchev SashaSemenishchev added the bug Something isn't working label Oct 11, 2023
@Sychic
Copy link
Member

Sychic commented Oct 11, 2023

Should be fixed by #75

@SashaSemenishchev
Copy link
Author

Should be fixed by #75

I wish it’d get merged, it’s more than a year since 😭

@Johni0702
Copy link
Contributor

I've closed the referenced PR as it has several shortcomings that make it unfit for general use.
If it works in your case, you could just create a local copy of it for your use.

Internally we've come to do something similar but with different tradeoffs than that PR, namely that it accounts for all SiblingConstraint-induced padding (or more generally any PaddingConstraint-induced padding) but not any offset induced in any other way. In particular your y = SiblingConstraint(3f) + 5f.pixels (which, given the aren't any preceding siblings (?) is just 5.pixels) wouldn't work. We've come to instead use dummy spacer components of appropriate size whenever we need extra padding at the very start/end.

Here's the relevant part of FillConstraint (at least the width; height works analogous) which we've modified:

        return if (useSiblings) {
            target.getWidth() - target.children.filter { it != component }.sumOf {
                it.getWidth().toDouble() + ((it.constraints.x as? PaddingConstraint)?.getHorizontalPadding(it) ?: 0f).toDouble()
            }.toFloat()
        } else target.getRight() - component.getLeft() + ((target.constraints.x as? PaddingConstraint)?.getHorizontalPadding(target) ?: 0f)

Beware that we only ever use it with useSiblings = true, so that else case is completely untested (and looking at it, idk what that's supposed to do; doesn't look right to me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants