Skip to content

Propagate layout constraints from non-inlined component children#11177

Open
dfaure-kdab wants to merge 1 commit intoslint-ui:masterfrom
dfaure-kdab:wip/dfaure/propagate_layout_constraints
Open

Propagate layout constraints from non-inlined component children#11177
dfaure-kdab wants to merge 1 commit intoslint-ui:masterfrom
dfaure-kdab:wip/dfaure/propagate_layout_constraints

Conversation

@dfaure-kdab
Copy link
Copy Markdown
Collaborator

When a component with internal layout constraints (e.g., a VerticalLayout
with min-width) was used without being inlined, its constraints were not
propagated to the parent layout. This caused layout behavior to differ
depending on whether a component was inlined or not.

The fix adds a fallback in gen_layout_info_prop: when a child is a
non-inlined component reference whose base component's root element has
layout_info_prop, use implicit_layout_info_call to propagate the
constraints across the component boundary.

Fixes: #7700


Claude came up with this, I don't know this code enough to validate whether this is the right approach.
I asked
"non-inlined non-repeated", does this mean the bug still exists for inlined and/or repeated?

and the answer was:
No — the three cases are:
1. Inlined: The component's internal elements become direct children of the parent, so gen_layout_info_prop sees the VerticalLayout (with its layout_info_prop) as a direct child. Works already.
2. Repeated: Excluded at line 187-189 (the existing if c.borrow().repeated.is_some() { return None; } with the FIXME comment about #407). This is a pre-existing limitation for all layout types, not specific to this bug.
3. Non-inlined, non-repeated: This was the broken case, now fixed.

So "non-inlined non-repeated" is the only case that needed fixing. The repeated.is_none() check in my code isn't hiding a bug — it avoids the implicit_layout_info_call on repeated elements which would panic (as I discovered), and repeated elements are already handled (or rather, explicitly not handled) by the existing code above.

@dfaure-kdab dfaure-kdab force-pushed the wip/dfaure/propagate_layout_constraints branch from 8347fd7 to 5c0c6af Compare March 29, 2026 16:51
@dfaure-kdab
Copy link
Copy Markdown
Collaborator Author

This also required fixing a binding loop in the weather demo exposed by this change:
DayForecastGraph used a HorizontalLayout for children that already had
explicit widths (width: root.day-width), creating a circular dependency
between the layout info and the container width. Replace with a Rectangle
and explicit x positioning.

I guess the alternative would be to get rid of day-width and let the HorizontalLayout take care of the items width.

@dfaure-kdab dfaure-kdab enabled auto-merge (rebase) March 29, 2026 17:06
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the code should be put here.
If there is no explicit constraint, we should merge the implicit_layout_info_call with the base constraints.

In fact, i'd be hoping that the implicit_layout_info_call do the right thing, but perhaps the use_implicit_size is not correct.
The test that check for the default_size_binding should probably be moved into implicit_layout_info_call instead of being done here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the tip, it works!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, actually it doesn't. It breaks elements_component_container_init, unlike my earlier version.
Looks like this is over my head.

When a component with internal layout constraints (e.g., a VerticalLayout
with min-width) was used without being inlined, its constraints were not
propagated to the parent layout. This caused layout behavior to differ
depending on whether a component was inlined or not.

The fix: always call implicit_layout_info_call in gen_layout_info_prop
when a child has no explicit constraints, instead of only doing so for
builtins with ImplicitSize. implicit_layout_info_call already handles
components correctly (propagates layout_info_prop from the base
component's root element).

Also fix a binding loop in the weather demo exposed by this change:
DayForecastGraph used a HorizontalLayout for children that already had
explicit widths (width: root.day-width), creating a circular dependency.
Replace with a Rectangle and explicit x positioning.

Fixes: slint-ui#7700
@dfaure-kdab dfaure-kdab force-pushed the wip/dfaure/propagate_layout_constraints branch from 5c0c6af to 0c9762b Compare March 29, 2026 19:38
@ogoffart
Copy link
Copy Markdown
Member

Hmm, actually it doesn't. It breaks elements_component_container_init, unlike my earlier version.

For some reason (not sure why actually, maybe that's another oversight) TextInput's default_size_binding is expands_to_parent_geometry (so not DefaultSizeBinding::ImplicitSize).
So that means that we call implicit_layout_info_call now while before we were not calling it. So that's why it breaks now.

Maybe we should pass a flag to implicit_layout_info_call so that it wouldn't get the implicit size of builtin element whose default_size_binding != DefaultSizeBinding::ImplicitSize
(that's also an optimization, as we don't want to generate code for it)

But it also expose another bug: the explicit preferred size should in principle still take over (as reported in #6535)

@dfaure-kdab
Copy link
Copy Markdown
Collaborator Author

Should I stash this for now and look at fixing #6535 first then, to do one thing at a time?

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.

Layout constraints are only taken if the element is inlined

2 participants