feature: SizedBox, Center components & rendering fix - #47
Conversation
implemented fix: StatefulComponentInstance.render was accessing child.bounds before they were set
|
@paramendula thanks for contributing! I'll get back to you tomorrow! |
| }) : _childInstance = component.child.createInstance(); | ||
|
|
||
| @override | ||
| Size measure(Size maxSize) { |
There was a problem hiding this comment.
The measure function is meant to report the component’s full size to the layout engine so it can position everything correctly. Right now the returned width and height don’t account for padding.vertical and padding.horizontal. If these values aren’t included, the layout engine may place the component incorrectly or produce visual glitches.
The same applies to any style properties that add visual size, such as border width or border height. These also contribute to the component’s actual layout size and should be added to the measured width and height.
Including padding and style in the measurement will prevent unexpected UI issues and keep the layout consistent.
| for (final child in childrenInstance) { | ||
| Logger.trace("StatefulComponent", "Item $child is being rendered"); | ||
| child.render(buffer, child.bounds); | ||
| child.render(buffer, bounds); |
There was a problem hiding this comment.
child.bounds is the rectangle assigned by LayoutEngine._layoutRecursiveCompute. That’s how every ParentComponentInstance (rows, columns, buttons, etc.) learns the exact slot it should draw into. Passing the parent’s bounds instead would tell every child to render over the entire parent area, so siblings would stack on top of one another and ignore their measured positions, padding, and absolute offsets. Hit testing and clipping would likewise break because children no longer know their true coordinates.
I noticed that _SizedBoxInstance extends ComponentInstance instead of ParentComponentInstance changing this will allow the layout tree calculation to reach the child nodes.
| /// Fills a rectangular [area] with a specified [backgroundColor]. | ||
| /// | ||
| /// All cells within [area] will have their background color set to [backgroundColor]. | ||
| void fill(Rect area, AnsiColorType backgroundColor) { |
There was a problem hiding this comment.
Great addition! absolutely needed
| } | ||
|
|
||
| @override | ||
| int fitHeight() => size.height + margin.vertical; |
primequantuM4
left a comment
primequantuM4
left a comment
There was a problem hiding this comment.
Thanks for putting this together! The addition is solid and it's clear care went into the implementation. There are just a couple of pieces to adjust so the component fits smoothly into the layout system.
-
The
measurefunction should report the full layout size, which includes padding (vertical and horizontal) and any style properties that affect the final dimensions, like borders. Adding these in will help avoid unexpected placement issues in the layout engine. -
SizedBoxInstanceis currently extendingComponentInstance, but its role matchesParentComponentInstancemore closely. Updating the base class will make it behave consistently with the other layout containers.
2 participants
New:
Center component: allows to center child widgets, improving layout flexibility (Documented)
SizedBox component: a box of fixed size that has one child (useful for fixed layouts, Documented)
CanvasBuffer.fill: implemented a fill method for CanvasBuffer to support background coloring (Documented)
SizedBox demo: incorporates SizedBox and Center components in a simple showcase app
Rendering bug fix:
Resolved an issue in StatefulComponentInstance.render where child component bounds were accessed before being set (my demo was throwing exceptions because of that)